gh-104522: Fix OSError raised when run a subprocess (#114195)

Only set filename to cwd if it was caused by failed chdir(cwd).

_fork_exec() now returns "noexec:chdir" for failed chdir(cwd).

Co-authored-by: Robert O'Shea <PurityLake@users.noreply.github.com>

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index d5bd9a9e31aa04deb00e5082bc000aa888118092..20db7747d5db13c323c0136b49735589a4a5d4f1 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1944,16 +1944,21 @@ class Popen:
                         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 944a7de4210bc91d9f1e3841a5f507a15e1791ef..12b88294a2d370f0cbc45742c98d68fef87553d8 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -2017,11 +2017,12 @@ class POSIXProcessTestCase(BaseTestCase):
                                  "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
@@ -2065,8 +2066,8 @@ class POSIXProcessTestCase(BaseTestCase):
                                  "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
@@ -2114,7 +2115,8 @@ class POSIXProcessTestCase(BaseTestCase):
                     [sys.executable, "-c",
                      "import os, sys, json; json.dump(os.getgroups(), sys.stdout)"],
                     extra_groups=group_list)
-        except PermissionError:
+        except PermissionError as e:
+            self.assertIsNone(e.filename)
             self.skipTest("setgroup() EPERM; this test may require root.")
         else:
             parent_groups = os.getgroups()

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 (file)
index 0000000..ca98094
--- /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 d0dd8f064e03955d2ce6e3a5edba2a1451060413..aa1a300e4378dd9dd926f0b53fd24d07855e3010 100644 (file)
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -673,9 +673,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];
 
@@ -735,8 +736,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. */
@@ -784,7 +789,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);
@@ -842,16 +847,12 @@ error:
         }
         _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));
 }