gh-109590 - Update shutil.which on win32 to only give back PATHEXT ma…

…tches by default
python · zooba · Oct 2, 2023 · Sep 28, 2023 · Sep 28, 2023 · Sep 29, 2023
commit e13eefeecea3fcd9dff5ee11cf661871efbbb792
diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst
@@ -476,6 +476,12 @@ Directory and files operations
       or ends with an extension that is in ``PATHEXT``; and filenames that
       have no extension can now be found.
 
+   .. versionchanged:: 3.12.1
+      On Windows, if *mode* includes ``os.X_OK``, only executables with an
+      extension in ``PATHEXT`` will be returned. Therefore extension-less
+      files cannot be returned by :func:`shutil.which` by default on Windows.
+      This brings behavior closer to that of Python 3.11.
+
 .. exception:: Error
 
    This exception collects exceptions that are raised during a multi-file

diff --git a/Lib/shutil.py b/Lib/shutil.py
@@ -1554,8 +1554,16 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None):
         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]
+        files = ([cmd] + [cmd + ext for ext in pathext])
+
+        # gh-109590. If we are looking for an executable, we need to look
+        # for a PATHEXT match. The first cmd is the direct match
+        # (e.g. python.exe instead of python)
+        # Check that direct match first if and only if the extension is in PATHEXT
+        if mode & os.X_OK and not any(
+            [os.path.splitext(files[0])[1].upper() == ext.upper() for ext in pathext]
+            ):
+            files = files[1:]
     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
@@ -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,38 +2273,39 @@ 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
         program = os.path.basename(temp_filexyz.name)
         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)
 
     # See GH-75586
     @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,16 +2321,62 @@ 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_pathext_enforced_for_execute(self):
+        with os_helper.EnvironmentVarGuard() as env:
+            env["PATH"] = self.temp_dir if isinstance(self.temp_dir, str) else self.temp_dir.decode()
+            env["PATHEXT"] = ".test"
+
+            exe = os.path.join(self.temp_dir, self.to_text_type("test.exe"))
+            open(exe, 'w').close()
+            os.chmod(exe, 0o755)
+
+            # default does not match since .exe is not in PATHEXT
+            self.assertIsNone(shutil.which(self.to_text_type("test.exe")))
+
+            # but if we don't use os.X_OK we're ok not matching PATHEXT
+            self.assertEqual(shutil.which(self.to_text_type("test.exe"), mode=os.F_OK), exe)
+
+    # See GH-109590
+    @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows')
+    def test_pathext_given_extension_preferred(self):
+        with os_helper.EnvironmentVarGuard() as env:
+            env["PATH"] = self.temp_dir if isinstance(self.temp_dir, str) else self.temp_dir.decode()
+            env["PATHEXT"] = ".exe2;.exe"
+
+            exe = os.path.join(self.temp_dir, self.to_text_type("test.exe"))
+            open(exe, 'w').close()
+            os.chmod(exe, 0o755)
+
+            exe2 = os.path.join(self.temp_dir, self.to_text_type("test.exe2"))
+            open(exe2, 'w').close()
+            os.chmod(exe2, 0o755)
+
+            # even though .exe2 is preferred in PATHEXT, we matched directly to test.exe
+            self.assertEqual(shutil.which(self.to_text_type("test.exe")), exe)
+
+            self.assertEqual(shutil.which(self.to_text_type("test")), exe2)
+
 
 class TestWhichBytes(TestWhich):
     def setUp(self):
         TestWhich.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):
 

diff --git a/Misc/NEWS.d/next/Library/2023-09-24-06-04-14.gh-issue-109590.9EMofC.rst b/Misc/NEWS.d/next/Library/2023-09-24-06-04-14.gh-issue-109590.9EMofC.rst
@@ -0,0 +1,2 @@
+:func:`shutil.which` will only match files with an extension in ``PATHEXT`` if the given mode includes ``os.X_OK`` on win32.
+This change will have :func:`shutil.which` act more similarly to previous behavior in Python 3.11.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		:func:`shutil.which` will only match files with an extension in ``PATHEXT`` if the given mode includes ``os.X_OK`` on win32.
		This change will have :func:`shutil.which` act more similarly to previous behavior in Python 3.11.