From 41cb29962a04fae45536f94730c055d80f00b3ad Mon Sep 17 00:00:00 2001 From: Charles Machalow Date: Sat, 23 Sep 2023 22:30:21 -0700 Subject: [PATCH] gh-109590 - Fix shutil.which to more closely match CMD on win32 An extensionless file will on be attempted if '.' is in PATHEXT Fix up the tests to make better use of the bytes testing of shutil.which --- Lib/shutil.py | 12 +++++-- Lib/test/test_shutil.py | 69 +++++++++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index b37bd082eee0c6..0a65ae228e094c 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -1551,11 +1551,17 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None): pathext_source = os.getenv("PATHEXT") or _WIN_DEFAULT_PATHEXT pathext = [ext for ext in pathext_source.split(os.pathsep) if ext] + dot = '.' if use_bytes: pathext = [os.fsencode(ext) for ext in pathext] - - # Always try checking the originally given cmd, if it doesn't match, try pathext - files = [cmd] + [cmd + ext for ext in pathext] + dot = b'.' + + # Attempt to match CMD behavior: + # Only try the given cmd if it has an extension (therefore has a dot) + # or a dot is a pathext in PATHEXT. + # Otherwise use PATHEXT to formulate paths to check. + files = (([cmd] if (dot in cmd or dot in pathext) else []) + + [cmd + ext for ext in pathext]) else: # On other platforms you don't have things like PATHEXT to tell you # what file suffixes are executable, so just pass on cmd as-is. diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index a2ca4df135846f..9a9c79239f5724 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -2067,6 +2067,14 @@ def setUp(self): self.curdir = os.curdir self.ext = ".EXE" + def to_text_type(self, s): + ''' + In this class we're testing with str, so convert s to a str + ''' + if isinstance(s, bytes): + return s.decode() + return s + def test_basic(self): # Given an EXE in a directory, it should be returned. rv = shutil.which(self.file, path=self.dir) @@ -2254,9 +2262,9 @@ def test_empty_path_no_PATH(self): @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows') def test_pathext(self): - ext = ".xyz" + ext = self.to_text_type(".xyz") temp_filexyz = tempfile.NamedTemporaryFile(dir=self.temp_dir, - prefix="Tmp2", suffix=ext) + prefix=self.to_text_type("Tmp2"), suffix=ext) os.chmod(temp_filexyz.name, stat.S_IXUSR) self.addCleanup(temp_filexyz.close) @@ -2265,16 +2273,16 @@ def test_pathext(self): program = os.path.splitext(program)[0] with os_helper.EnvironmentVarGuard() as env: - env['PATHEXT'] = ext + env['PATHEXT'] = ext if isinstance(ext, str) else ext.decode() rv = shutil.which(program, path=self.temp_dir) self.assertEqual(rv, temp_filexyz.name) # Issue 40592: See https://bugs.python.org/issue40592 @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows') def test_pathext_with_empty_str(self): - ext = ".xyz" + ext = self.to_text_type(".xyz") temp_filexyz = tempfile.NamedTemporaryFile(dir=self.temp_dir, - prefix="Tmp2", suffix=ext) + prefix=self.to_text_type("Tmp2"), suffix=ext) self.addCleanup(temp_filexyz.close) # strip path and extension @@ -2282,7 +2290,7 @@ def test_pathext_with_empty_str(self): program = os.path.splitext(program)[0] with os_helper.EnvironmentVarGuard() as env: - env['PATHEXT'] = f"{ext};" # note the ; + env['PATHEXT'] = f"{ext if isinstance(ext, str) else ext.decode()};" # note the ; rv = shutil.which(program, path=self.temp_dir) self.assertEqual(rv, temp_filexyz.name) @@ -2290,13 +2298,14 @@ def test_pathext_with_empty_str(self): @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows') def test_pathext_applied_on_files_in_path(self): with os_helper.EnvironmentVarGuard() as env: - env["PATH"] = self.temp_dir + env["PATH"] = self.temp_dir if isinstance(self.temp_dir, str) else self.temp_dir.decode() env["PATHEXT"] = ".test" - test_path = pathlib.Path(self.temp_dir) / "test_program.test" - test_path.touch(mode=0o755) + test_path = os.path.join(self.temp_dir, self.to_text_type("test_program.test")) + open(test_path, 'w').close() + os.chmod(test_path, 0o755) - self.assertEqual(shutil.which("test_program"), str(test_path)) + self.assertEqual(shutil.which(self.to_text_type("test_program")), test_path) # See GH-75586 @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows') @@ -2312,6 +2321,37 @@ def test_win_path_needs_curdir(self): self.assertFalse(shutil._win_path_needs_curdir('dontcare', os.X_OK)) need_curdir_mock.assert_called_once_with('dontcare') + # See GH-109590 + @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows') + def test_extensionless_file_resolution_no_dot_in_pathext(self): + with os_helper.EnvironmentVarGuard() as env: + env['PATHEXT'] = ".test;" + env['PATH'] = self.temp_dir if isinstance(self.temp_dir, str) else self.temp_dir.decode() + + extensionless_file_in_path = os.path.join(self.temp_dir, self.to_text_type("file")) + open(extensionless_file_in_path, 'w').close() + + extensioned_file_in_path = os.path.join(self.temp_dir, self.to_text_type("file.test")) + open(extensioned_file_in_path, 'w').close() + + + self.assertEqual(shutil.which(self.to_text_type('file'), os.F_OK), extensioned_file_in_path) + + # See GH-109590 + @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows') + def test_extensionless_file_resolution_dot_in_pathext(self): + with os_helper.EnvironmentVarGuard() as env: + env['PATHEXT'] = ".test;.;" + env['PATH'] = self.temp_dir if isinstance(self.temp_dir, str) else self.temp_dir.decode() + + extensionless_file_in_path = os.path.join(self.temp_dir, self.to_text_type("file")) + open(extensionless_file_in_path, 'w').close() + + extensioned_file_in_path = os.path.join(self.temp_dir, self.to_text_type("file.test")) + open(extensioned_file_in_path, 'w').close() + + self.assertEqual(shutil.which(self.to_text_type('file')), extensionless_file_in_path) + class TestWhichBytes(TestWhich): def setUp(self): @@ -2319,9 +2359,18 @@ def setUp(self): self.dir = os.fsencode(self.dir) self.file = os.fsencode(self.file) self.temp_file.name = os.fsencode(self.temp_file.name) + self.temp_dir = os.fsencode(self.temp_dir) self.curdir = os.fsencode(self.curdir) self.ext = os.fsencode(self.ext) + def to_text_type(self, s): + ''' + In this class we're testing with bytes, so convert s to a bytes + ''' + if isinstance(s, str): + return s.encode() + return s + class TestMove(BaseTest, unittest.TestCase):