* process.c (check_exec_redirect_fd): prohibit duplex IO.

(check_exec_fds): record maxhint even if close_others is not
  specified.
  (rb_exec_arg_fixup): renamed from rb_exec_arg_fix.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@16235 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,10 @@
+Wed Apr 30 12:32:39 2008  Tanaka Akira  <akr@fsij.org>
+
+	* process.c (check_exec_redirect_fd): prohibit duplex IO.
+	  (check_exec_fds): record maxhint even if close_others is not
+	  specified.
+	  (rb_exec_arg_fixup): renamed from rb_exec_arg_fix.
+
 Mon Apr 28 20:24:27 2008  Tadayoshi Funaba  <tadf@dotrb.org>
 
 	* rational.c (nurat_marshal_load): checks the given

include/ruby/intern.h

@@ -455,7 +455,7 @@ int rb_proc_exec_n(int, VALUE*, const char*);
 int rb_proc_exec(const char*);
 VALUE rb_exec_arg_init(int argc, VALUE *argv, int accept_shell, struct rb_exec_arg *e);
 int rb_exec_arg_addopt(struct rb_exec_arg *e, VALUE key, VALUE val);
-void rb_exec_arg_fix(struct rb_exec_arg *e);
+void rb_exec_arg_fixup(struct rb_exec_arg *e);
 int rb_exec(const struct rb_exec_arg*);
 rb_pid_t rb_fork(int*, int (*)(void*), void*, VALUE);
 VALUE rb_f_exec(int,VALUE*);

io.c

@@ -3761,7 +3761,7 @@ pipe_open(struct rb_exec_arg *eargp, VALUE prog, const char *mode)
         rb_sys_fail(cmd);
     }
     if (eargp) {
-        rb_exec_arg_fix(arg.execp);
+        rb_exec_arg_fixup(arg.execp);
 	pid = rb_fork(&status, popen_exec, &arg, arg.execp->redirect_fds);
     }
     else {

process.c

@@ -1250,6 +1250,8 @@ check_exec_redirect_fd(VALUE v)
     else if (!NIL_P(tmp = rb_check_convert_type(v, T_FILE, "IO", "to_io"))) {
         rb_io_t *fptr;
         GetOpenFile(tmp, fptr);
+        if (fptr->tied_io_for_writing)
+            rb_raise(rb_eArgError, "duplex IO redirection");
         fd = fptr->fd;
     }
     else {
@@ -1510,7 +1512,7 @@ check_exec_fds(VALUE options)
             }
         }
     }
-    if (RTEST(rb_ary_entry(options, EXEC_OPTION_CLOSE_OTHERS))) {
+    if (rb_ary_entry(options, EXEC_OPTION_CLOSE_OTHERS) != Qfalse) {
         rb_ary_store(options, EXEC_OPTION_CLOSE_OTHERS, INT2FIX(maxhint));
     }
     return h;
@@ -1652,7 +1654,7 @@ rb_exec_arg_init(int argc, VALUE *argv, int accept_shell, struct rb_exec_arg *e)
 }
 
 void
-rb_exec_arg_fix(struct rb_exec_arg *e)
+rb_exec_arg_fixup(struct rb_exec_arg *e)
 {
     e->redirect_fds = check_exec_fds(e->options);
 }
@@ -1697,7 +1699,7 @@ rb_f_exec(int argc, VALUE *argv)
     rb_exec_arg_init(argc, argv, Qtrue, &earg);
     if (NIL_P(rb_ary_entry(earg.options, EXEC_OPTION_CLOSE_OTHERS)))
         rb_exec_arg_addopt(&earg, ID2SYM(rb_intern("close_others")), Qfalse);
-    rb_exec_arg_fix(&earg);
+    rb_exec_arg_fixup(&earg);
 
     rb_exec(&earg);
     rb_sys_fail(earg.prog);
@@ -2047,7 +2049,7 @@ run_exec_options(const struct rb_exec_arg *e)
 #ifdef HAVE_FORK
     obj = rb_ary_entry(options, EXEC_OPTION_CLOSE_OTHERS);
     if (obj != Qfalse) {
-        rb_close_before_exec(3, FIXNUM_P(obj) ? FIX2LONG(obj) : 0, e->redirect_fds);
+        rb_close_before_exec(3, FIX2LONG(obj), e->redirect_fds);
     }
 #endif
 
@@ -2547,7 +2549,7 @@ rb_spawn_internal(int argc, VALUE *argv, int default_close_others)
         VALUE v = default_close_others ? Qtrue : Qfalse;
         rb_exec_arg_addopt(&earg, ID2SYM(rb_intern("close_others")), v);
     }
-    rb_exec_arg_fix(&earg);
+    rb_exec_arg_fixup(&earg);
 
 #if defined HAVE_FORK
     status = rb_fork(&status, rb_exec_atfork, &earg, earg.redirect_fds);
@@ -2746,6 +2748,8 @@ rb_f_system(int argc, VALUE *argv)
  *  spawn closes all non-standard unspecified descriptors by default.
  *  The "standard" descriptors are 0, 1 and 2.
  *  This behavior is specified by :close_others option.
+ *  :close_others doesn't affect the standard descriptors which are
+ *  closed only if :close is specified explicitly.
  *
  *    pid = spawn(command, :close_others=>true)  # close 3,4,5,... (default)
  *    pid = spawn(command, :close_others=>false) # don't close 3,4,5,...

test/ruby/test_process.rb

@@ -523,6 +523,24 @@ class TestProcess < Test::Unit::TestCase
     }
   end
 
+  def test_execopts_redirect_self
+    with_pipe {|r, w|
+      w << "haha\n"
+      w.close
+      r.close_on_exec = true
+      IO.popen([RUBY, "-e", "print IO.new(#{r.fileno}).read", r.fileno=>r.fileno, :close_others=>false]) {|io|
+        assert_equal("haha\n", io.read)
+      }
+    }
+  end
+
+  def test_execopts_duplex_io
+    IO.popen("#{RUBY} -e ''", "r+") {|duplex|
+      assert_raise(ArgumentError) { system("#{RUBY} -e ''", duplex=>STDOUT) }
+      assert_raise(ArgumentError) { system("#{RUBY} -e ''", STDOUT=>duplex) }
+    }
+  end
+
   def test_execopts_modification
     h = {}
     Process.wait spawn(*TRUECOMMAND, h)