From: Ross Lagerwall <rosslagerwall@gmail.com>
Date: Wed, 27 Jul 2011 05:16:31 +0000 (+0200)
Subject: Issue #12607: In subprocess, fix issue where if stdin, stdout or stderr is
X-Git-Tag: v3.2.2rc1~63
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d98646e43096053ee13f5a9a6af38f8c723b1aaf;p=thirdparty%2FPython%2Fcpython.git

Issue #12607: In subprocess, fix issue where if stdin, stdout or stderr is
given as a low fd, it gets overwritten.
---

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index ddbca6ab1be5..2d85b507103f 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1219,6 +1219,14 @@ class Popen(object):
                                     os.close(errread)
                                 os.close(errpipe_read)
 
+                                # When duping fds, if there arises a situation
+                                # where one of the fds is either 0, 1 or 2, it
+                                # is possible that it is overwritten (#12607).
+                                if c2pwrite == 0:
+                                    c2pwrite = os.dup(c2pwrite)
+                                if errwrite == 0 or errwrite == 1:
+                                    errwrite = os.dup(errwrite)
+
                                 # Dup fds for child
                                 def _dup2(a, b):
                                     # dup2() removes the CLOEXEC flag but
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 3440b4548534..8382c720d161 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1106,6 +1106,64 @@ class POSIXProcessTestCase(BaseTestCase):
             for fd in temp_fds:
                 os.close(fd)
 
+    def check_swap_fds(self, stdin_no, stdout_no, stderr_no):
+        # open up some temporary files
+        temps = [mkstemp() for i in range(3)]
+        temp_fds = [fd for fd, fname in temps]
+        try:
+            # unlink the files -- we won't need to reopen them
+            for fd, fname in temps:
+                os.unlink(fname)
+
+            # save a copy of the standard file descriptors
+            saved_fds = [os.dup(fd) for fd in range(3)]
+            try:
+                # duplicate the temp files over the standard fd's 0, 1, 2
+                for fd, temp_fd in enumerate(temp_fds):
+                    os.dup2(temp_fd, fd)
+
+                # write some data to what will become stdin, and rewind
+                os.write(stdin_no, b"STDIN")
+                os.lseek(stdin_no, 0, 0)
+
+                # now use those files in the given order, so that subprocess
+                # has to rearrange them in the child
+                p = subprocess.Popen([sys.executable, "-c",
+                    'import sys; got = sys.stdin.read();'
+                    'sys.stdout.write("got %s"%got); sys.stderr.write("err")'],
+                    stdin=stdin_no,
+                    stdout=stdout_no,
+                    stderr=stderr_no)
+                p.wait()
+
+                for fd in temp_fds:
+                    os.lseek(fd, 0, 0)
+
+                out = os.read(stdout_no, 1024)
+                err = support.strip_python_stderr(os.read(stderr_no, 1024))
+            finally:
+                for std, saved in enumerate(saved_fds):
+                    os.dup2(saved, std)
+                    os.close(saved)
+
+            self.assertEqual(out, b"got STDIN")
+            self.assertEqual(err, b"err")
+
+        finally:
+            for fd in temp_fds:
+                os.close(fd)
+
+    # When duping fds, if there arises a situation where one of the fds is
+    # either 0, 1 or 2, it is possible that it is overwritten (#12607).
+    # This tests all combinations of this.
+    def test_swap_fds(self):
+        self.check_swap_fds(0, 1, 2)
+        self.check_swap_fds(0, 2, 1)
+        self.check_swap_fds(1, 0, 2)
+        self.check_swap_fds(1, 2, 0)
+        self.check_swap_fds(2, 0, 1)
+        self.check_swap_fds(2, 1, 0)
+
     def test_surrogates_error_message(self):
         def prepare():
             raise ValueError("surrogate:\uDCff")
diff --git a/Misc/NEWS b/Misc/NEWS
index ed8511dea923..7da5eaab0281 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -37,6 +37,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #12607: In subprocess, fix issue where if stdin, stdout or stderr is
+  given as a low fd, it gets overwritten.
+
 - Issue #12590: IDLE editor window now always displays the first line
   when opening a long file.  With Tk 8.5, the first line was hidden.
 
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index bf10cbb43e13..11b24a031f50 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -69,6 +69,13 @@ static void child_exec(char *const exec_array[],
     }
     POSIX_CALL(close(errpipe_read));
 
+    /* When duping fds, if there arises a situation where one of the fds is
+       either 0, 1 or 2, it is possible that it is overwritten (#12607). */
+    if (c2pwrite == 0)
+        POSIX_CALL(c2pwrite = dup(c2pwrite));
+    if (errwrite == 0 || errwrite == 1)
+        POSIX_CALL(errwrite = dup(errwrite));
+
     /* Dup fds for child.
        dup2() removes the CLOEXEC flag but we must do it ourselves if dup2()
        would be a no-op (issue #10806). */