https://github.com/python/cpython/commit/2c9872428e65d32e5896cdf4d633027e88680491 commit: 2c9872428e65d32e5896cdf4d633027e88680491 branch: 3.11 author: Serhiy Storchaka <[email protected]> committer: serhiy-storchaka <[email protected]> date: 2024-01-18T11:32:57Z summary:
[3.11] gh-104522: Fix OSError raised when run a subprocess (GH-114195) (GH-114243) Only set filename to cwd if it was caused by failed chdir(cwd). _fork_exec() now returns "noexec:chdir" for failed chdir(cwd). (cherry picked from commit e2c097ebdee447ded1109f99a235e65aa3533bf8) Co-authored-by: Robert O'Shea <[email protected]> files: A Misc/NEWS.d/next/Library/2024-01-17-18-53-51.gh-issue-104522.3NyDf4.rst M Lib/subprocess.py M Lib/test/test_subprocess.py M Modules/_posixsubprocess.c diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 6df5dd551ea67e..3264d9afc7ebf3 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1938,16 +1938,21 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, SubprocessError) if issubclass(child_exception_type, OSError) and hex_errno: errno_num = int(hex_errno, 16) - child_exec_never_called = (err_msg == "noexec") - if child_exec_never_called: + if err_msg == "noexec:chdir": err_msg = "" # The error must be from chdir(cwd). err_filename = cwd + elif err_msg == "noexec": + err_msg = "" + err_filename = None else: err_filename = orig_executable if errno_num != 0: err_msg = os.strerror(errno_num) - raise child_exception_type(errno_num, err_msg, err_filename) + if err_filename is not None: + raise child_exception_type(errno_num, err_msg, err_filename) + else: + raise child_exception_type(errno_num, err_msg) raise child_exception_type(err_msg) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index f71f953fbd1797..b4206c7bea897d 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2030,11 +2030,12 @@ def test_user(self): "import os; print(os.getuid())"], user=user, close_fds=close_fds) - except PermissionError: # (EACCES, EPERM) - pass + except PermissionError as e: # (EACCES, EPERM) + self.assertIsNone(e.filename) except OSError as e: if e.errno not in (errno.EACCES, errno.EPERM): raise + self.assertIsNone(e.filename) else: if isinstance(user, str): user_uid = pwd.getpwnam(user).pw_uid @@ -2078,8 +2079,8 @@ def test_group(self): "import os; print(os.getgid())"], group=group, close_fds=close_fds) - except PermissionError: # (EACCES, EPERM) - pass + except PermissionError as e: # (EACCES, EPERM) + self.assertIsNone(e.filename) else: if isinstance(group, str): group_gid = grp.getgrnam(group).gr_gid @@ -2124,6 +2125,7 @@ def test_extra_groups(self): except OSError as ex: if ex.errno != errno.EPERM: raise + self.assertIsNone(ex.filename) perm_error = True else: diff --git a/Misc/NEWS.d/next/Library/2024-01-17-18-53-51.gh-issue-104522.3NyDf4.rst b/Misc/NEWS.d/next/Library/2024-01-17-18-53-51.gh-issue-104522.3NyDf4.rst new file mode 100644 index 00000000000000..ca980945ea12d3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-17-18-53-51.gh-issue-104522.3NyDf4.rst @@ -0,0 +1,3 @@ +:exc:`OSError` raised when run a subprocess now only has *filename* +attribute set to *cwd* if the error was caused by a failed attempt to change +the current directory. diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 072519c91a085d..d91bf214b46f68 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -566,9 +566,10 @@ child_exec(char *const exec_array[], PyObject *preexec_fn, PyObject *preexec_fn_args_tuple) { - int i, saved_errno, reached_preexec = 0; + int i, saved_errno; PyObject *result; - const char* err_msg = ""; + /* Indicate to the parent that the error happened before exec(). */ + const char *err_msg = "noexec"; /* Buffer large enough to hold a hex integer. We can't malloc. */ char hex_errno[sizeof(saved_errno)*2+1]; @@ -628,8 +629,12 @@ child_exec(char *const exec_array[], /* 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)); + if (cwd) { + if (chdir(cwd) == -1) { + err_msg = "noexec:chdir"; + goto error; + } + } if (child_umask >= 0) umask(child_umask); /* umask() always succeeds. */ @@ -672,7 +677,7 @@ child_exec(char *const exec_array[], #endif /* HAVE_SETREUID */ - reached_preexec = 1; + err_msg = ""; if (preexec_fn != Py_None && preexec_fn_args_tuple) { /* This is where the user has asked us to deadlock their program. */ result = PyObject_Call(preexec_fn, preexec_fn_args_tuple, NULL); @@ -730,16 +735,12 @@ child_exec(char *const exec_array[], } _Py_write_noraise(errpipe_write, cur, hex_errno + sizeof(hex_errno) - cur); _Py_write_noraise(errpipe_write, ":", 1); - if (!reached_preexec) { - /* Indicate to the parent that the error happened before exec(). */ - _Py_write_noraise(errpipe_write, "noexec", 6); - } /* We can't call strerror(saved_errno). It is not async signal safe. * The parent process will look the error message up. */ } else { _Py_write_noraise(errpipe_write, "SubprocessError:0:", 18); - _Py_write_noraise(errpipe_write, err_msg, strlen(err_msg)); } + _Py_write_noraise(errpipe_write, err_msg, strlen(err_msg)); } _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3/lists/python-checkins.python.org/ Member address: [email protected]
