Skip to content

Commit 40edefa

Browse files
authored
gh-119646: Include subprocess path in OSError on Windows (GH-157713)
1 parent 16b357b commit 40edefa

3 files changed

Lines changed: 75 additions & 7 deletions

File tree

‎Lib/subprocess.py‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,21 +1629,31 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
16291629
assert not pass_fds, "pass_fds not supported on Windows."
16301630

16311631
if isinstance(args, str):
1632-
pass
1632+
# Filename is the program only. Later arguments can
1633+
# hold secrets. A leading quote ends at the next quote.
1634+
# Otherwise stop at the first space.
1635+
if args[:1] == '"':
1636+
end = args.find('"', 1)
1637+
orig_filename = args[1:end] if end != -1 else args
1638+
else:
1639+
orig_filename = args.split(' ', 1)[0]
16331640
elif isinstance(args, bytes):
16341641
if shell:
16351642
raise TypeError('bytes args is not allowed on Windows')
1643+
orig_filename = os.fsdecode(args)
16361644
args = list2cmdline([args])
16371645
elif isinstance(args, os.PathLike):
16381646
if shell:
16391647
raise TypeError('path-like args is not allowed when '
16401648
'shell is true')
1649+
orig_filename = os.fsdecode(args)
16411650
args = list2cmdline([args])
16421651
else:
1652+
args = list(args)
1653+
orig_filename = os.fsdecode(args[0]) if args else None
16431654
args = list2cmdline(args)
1644-
16451655
if executable is not None:
1646-
executable = os.fsdecode(executable)
1656+
orig_filename = executable = os.fsdecode(executable)
16471657

16481658
# Process startup details
16491659
if startupinfo is None:
@@ -1725,6 +1735,19 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
17251735
env,
17261736
cwd,
17271737
startupinfo)
1738+
except OSError as e:
1739+
# gh-119646: POSIX already puts the attempted path on
1740+
# OSError.filename. Windows CreateProcess did not, so
1741+
# failures (missing exe, WSL paths, invalid cwd) were
1742+
# reported without naming the command.
1743+
if e.filename is None:
1744+
# ERROR_DIRECTORY (267): CreateProcess rejected cwd.
1745+
if cwd is not None and e.winerror == 267:
1746+
name = cwd
1747+
else:
1748+
name = orig_filename
1749+
raise type(e)(e.errno, e.strerror, name, e.winerror) from None
1750+
raise
17281751
finally:
17291752
# Child is launched. Close the parent's copy of those pipe
17301753
# handles that only the child should have open. You need

‎Lib/test/test_subprocess.py‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,13 +1797,28 @@ def test_failed_child_execute_fd_leak(self):
17971797
fds_after_exception = os.listdir(fd_directory)
17981798
self.assertEqual(fds_before_popen, fds_after_exception)
17991799

1800-
@unittest.skipIf(mswindows, "behavior currently not supported on Windows")
18011800
def test_file_not_found_includes_filename(self):
1801+
missing = (r'C:\opt\nonexistent_binary' if mswindows
1802+
else '/opt/nonexistent_binary')
18021803
with self.assertRaises(FileNotFoundError) as c:
1803-
subprocess.call(['/opt/nonexistent_binary', 'with', 'some', 'args'])
1804-
self.assertEqual(c.exception.filename, '/opt/nonexistent_binary')
1804+
subprocess.call([missing, 'with', 'some', 'args'])
1805+
self.assertEqual(c.exception.filename, missing)
1806+
1807+
def test_args_filter_iterable(self):
1808+
# gh-119646: Windows used to index args[0] before list2cmdline.
1809+
# test_faulthandler.test_sys_xoptions passes a filter() object.
1810+
args = filter(None, (sys.executable, "-c", "import sys; sys.exit(17)"))
1811+
self.assertEqual(subprocess.call(args), 17)
1812+
1813+
def test_file_not_found_includes_filename_from_iterable(self):
1814+
missing = (r'C:\opt\nonexistent_binary' if mswindows
1815+
else '/opt/nonexistent_binary')
1816+
args = filter(None, (missing, "with", "some", "args"))
1817+
with self.assertRaises(FileNotFoundError) as c:
1818+
subprocess.call(args)
1819+
self.assertEqual(c.exception.filename, missing)
18051820

1806-
@unittest.skipIf(mswindows, "behavior currently not supported on Windows")
1821+
@unittest.skipIf(mswindows, "Windows reports NotADirectoryError (WinError 267)")
18071822
def test_file_not_found_with_bad_cwd(self):
18081823
with self.assertRaises(FileNotFoundError) as c:
18091824
subprocess.Popen(['exit', '0'], cwd='/some/nonexistent/directory')
@@ -3718,6 +3733,34 @@ def test_vfork_used_when_expected(self):
37183733
@unittest.skipUnless(mswindows, "Windows specific tests")
37193734
class Win32ProcessTestCase(BaseTestCase):
37203735

3736+
def test_createprocess_bad_cwd_includes_filename(self):
3737+
# gh-119646: invalid cwd should appear on OSError.filename.
3738+
missing_cwd = r'C:\some\nonexistent\directory'
3739+
with self.assertRaises(OSError) as c:
3740+
subprocess.Popen([sys.executable, '-c', 'pass'], cwd=missing_cwd)
3741+
self.assertEqual(c.exception.filename, missing_cwd)
3742+
self.assertEqual(c.exception.winerror, 267)
3743+
3744+
def test_command_string_filename_omits_later_args(self):
3745+
# gh-119646: a command-line string must not put later arguments
3746+
# on OSError.filename. Those arguments can hold secrets.
3747+
missing = r'C:\opt\nonexistent_binary'
3748+
secret = 'NOT-A-REAL-SECRET'
3749+
quoted = r'C:\Program Files\nonexistent_binary'
3750+
cases = [
3751+
(missing, missing),
3752+
(f'{missing} --token {secret}', missing),
3753+
(f'"{missing}" --token {secret}', missing),
3754+
(f'"{quoted}" --token {secret}', quoted),
3755+
]
3756+
for command, expected in cases:
3757+
with self.subTest(command=command):
3758+
with self.assertRaises(FileNotFoundError) as c:
3759+
subprocess.call(command)
3760+
self.assertEqual(c.exception.filename, expected)
3761+
self.assertNotIn(secret, c.exception.filename or '')
3762+
self.assertNotIn(secret, str(c.exception))
3763+
37213764
def test_startupinfo(self):
37223765
# startupinfo argument
37233766
# We uses hardcoded constants, because we do not want to
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
On Windows, :exc:`OSError` from :mod:`subprocess` now includes the attempted
2+
executable or working directory in ``filename``.

0 commit comments

Comments
 (0)