diff options
| -rw-r--r-- | Lib/test/test_subprocess.py | 30 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Library/2018-09-10-14-15-53.bpo-32270.wSJjuD.rst | 2 | ||||
| -rw-r--r-- | Modules/_posixsubprocess.c | 24 |
3 files changed, 46 insertions, 10 deletions
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 4719773..8419061 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2529,6 +2529,36 @@ class POSIXProcessTestCase(BaseTestCase): self.assertEqual(os.get_inheritable(inheritable), True) self.assertEqual(os.get_inheritable(non_inheritable), False) + + # bpo-32270: Ensure that descriptors specified in pass_fds + # are inherited even if they are used in redirections. + # Contributed by @izbyshev. + def test_pass_fds_redirected(self): + """Regression test for https://bugs.python.org/issue32270.""" + fd_status = support.findfile("fd_status.py", subdir="subprocessdata") + pass_fds = [] + for _ in range(2): + fd = os.open(os.devnull, os.O_RDWR) + self.addCleanup(os.close, fd) + pass_fds.append(fd) + + stdout_r, stdout_w = os.pipe() + self.addCleanup(os.close, stdout_r) + self.addCleanup(os.close, stdout_w) + pass_fds.insert(1, stdout_w) + + with subprocess.Popen([sys.executable, fd_status], + stdin=pass_fds[0], + stdout=pass_fds[1], + stderr=pass_fds[2], + close_fds=True, + pass_fds=pass_fds): + output = os.read(stdout_r, 1024) + fds = {int(num) for num in output.split(b',')} + + self.assertEqual(fds, {0, 1, 2} | frozenset(pass_fds), f"output={output!a}") + + def test_stdout_stdin_are_single_inout_fd(self): with io.open(os.devnull, "r+") as inout: p = subprocess.Popen([sys.executable, "-c", "import sys; sys.exit(0)"], diff --git a/Misc/NEWS.d/next/Library/2018-09-10-14-15-53.bpo-32270.wSJjuD.rst b/Misc/NEWS.d/next/Library/2018-09-10-14-15-53.bpo-32270.wSJjuD.rst new file mode 100644 index 0000000..83f6862 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-09-10-14-15-53.bpo-32270.wSJjuD.rst @@ -0,0 +1,2 @@ +The subprocess module no longer mistakenly closes redirected fds even when +they were in pass_fds when outside of the default {0, 1, 2} set. diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 0150fcb..aeb10f9 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -422,10 +422,20 @@ child_exec(char *const exec_array[], /* 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) + if (c2pwrite == 0) { POSIX_CALL(c2pwrite = dup(c2pwrite)); - while (errwrite == 0 || errwrite == 1) + /* issue32270 */ + if (_Py_set_inheritable_async_safe(c2pwrite, 0, NULL) < 0) { + goto error; + } + } + while (errwrite == 0 || errwrite == 1) { POSIX_CALL(errwrite = dup(errwrite)); + /* issue32270 */ + if (_Py_set_inheritable_async_safe(errwrite, 0, NULL) < 0) { + goto error; + } + } /* Dup fds for child. dup2() removes the CLOEXEC flag but we must do it ourselves if dup2() @@ -451,14 +461,8 @@ child_exec(char *const exec_array[], else if (errwrite != -1) POSIX_CALL(dup2(errwrite, 2)); /* stderr */ - /* Close pipe fds. Make sure we don't close the same fd more than */ - /* once, or standard fds. */ - if (p2cread > 2) - POSIX_CALL(close(p2cread)); - if (c2pwrite > 2 && c2pwrite != p2cread) - POSIX_CALL(close(c2pwrite)); - if (errwrite != c2pwrite && errwrite != p2cread && errwrite > 2) - POSIX_CALL(close(errwrite)); + /* We no longer manually close p2cread, c2pwrite, and errwrite here as + * _close_open_fds takes care when it is not already non-inheritable. */ if (cwd) POSIX_CALL(chdir(cwd)); |