shutil.which wrong result on Windows

[bobjalex]

BPO	24505
Nosy	@pfmoore, @tjguk, @bitdancer, @zware, @SpecLad, @eryksun, @zooba, @bharel
Files	which2.py: A sample of 'which' function corrected issue 24505 proposed patch windows cmd semantics.patch: Proposed patch following cmd.exe semantics issue-24505-proposed-patch-2.patch new_which_bug_files.zip: ZIP file with suggested file updates

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2015-06-24.22:09:39.162>
labels = ['type-bug', '3.8', '3.9', '3.10', 'library', 'OS-windows']
title = 'shutil.which wrong result on Windows'
updated_at = <Date 2021-03-20.14:30:08.693>
user = 'https://bugs.python.org/bobjalex'

bugs.python.org fields:

activity = <Date 2021-03-20.14:30:08.693>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2015-06-24.22:09:39.162>
creator = 'bobjalex'
dependencies = []
files = ['39805', '41338', '41459', '42365']
hgrepos = []
issue_num = 24505
keywords = ['patch']
message_count = 20.0
messages = ['245780', '245792', '245885', '245887', '252329', '252341', '252355', '252358', '252359', '252367', '256242', '256373', '256613', '257248', '258413', '258415', '262854', '262875', '348640', '389137']
nosy_count = 10.0
nosy_names = ['paul.moore', 'tim.golden', 'r.david.murray', 'zach.ware', 'SpecLad', 'eryksun', 'steve.dower', 'bobjalex', 'bar.harel', 'tobytobkin']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue24505'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[bobjalex]

Python session with 3.5b2

Showing existing error:
>>> from shutil import which

Works OK
>>> which("python")
'C:\\Python27\\python.EXE'

Also works OK
>>> which('C:\\Python27\\python.EXE')
'C:\\Python27\\python.EXE'

Fails
>>> which('C:\\Python27\\python')
>>>

Showing better results with my fix (attached):
>>> from which2 import which
>>> which("python")
'C:\\Python27\\python.exe'
>>> which('C:\\Python27\\python.exe')
'C:\\Python27\\python.exe'
>>> which('C:\\Python27\\python')
'C:\\Python27\\python.exe'

Problem is with the short-circuiting code near the beginning of the function. It fails to check the extensions for Windows when short-circuited.

My fix has a few other improvements -- efficiency and nicer output without those ugly upper-case extensions.

[bitdancer]

Could you please submit your proposal as a patch? (A diff -u against the existing version).

[bobjalex]

Hi R. David --

My report is just to notify y'all of a bug that I noticed. The bug is
causing me no problem, and it's your option as to whether to fix it or not.

I offered a fix, but I haven't the time to perform diffs, etc. You could
make that diff yourself, since I sent my modified "which" function. Just
replace the existing "which" function in the version of shutil you would
like to fix, and perform the diff.

Bob

On Wed, Jun 24, 2015 at 8:26 PM, R. David Murray <report@bugs.python.org>
wrote:

R. David Murray added the comment:

Could you please submit your proposal as a patch? (A diff -u against the
existing version).

----------
nosy: +r.david.murray

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue24505\>

---

[bitdancer]

That's fine. Perhaps someone will be interested enough to pick this up and work on it.

[bharel]

Unfortunately that patch will not help for files like "python.exe.exe", and double ext which sometimes happens.
I'm on it. Should take me a day or two, more problems might arise along the way. Thanks Bob for alerting.

Regarding the uppercase, I believe it is better to leave the casing as it is. It should be up to the user to mess with it as converting case is a one way ticket. If someone entered an uppercase, perhaps he wants it like that. Also it's less prune to errors and behaves the same for Unix and Windows.

[eryksun]

Please don't default to searching the current directory:

 if is_windows:
 # The current directory takes precedence on Windows.
 if not os.curdir in path:
 path.insert(0, os.curdir)

Add a keyword option to enable this, with a warning that including the current directory isn't secure. This option should not be able to override the environment variable NoDefaultCurrentDirectoryInExePath.

[bitdancer]

Well, that's a newish situation for Windows, something I certainly wasn't aware of. Which is trying to find the executable that would be found if typed at the prompt. So, what is the correct algorithm for emulating how Windows does that search? (Or some way to hook directly in to Windows search...)

[zooba]

The Win32 API function linked by eryksun is the correct way to determine whether to include the current directory in the search - the doc specifically refers to matching cmd.exe's behaviour (searching ".;%PATH%" vs. just "%PATH%").

The PATHEXT behaviour should be to look for the name as provided first, followed be appending each extension. So looking for "python" will look for ["python", *("python" + p for p in getenv('PATHEXT').split(';'))] in that order.

[eryksun]

My suggestion is only for 3.5 / 3.6, because Windows XP is no longer supported. Starting with Windows Vista, both cmd.exe and CreateProcess support this behavior. I realize that disabling searching the current director by default is going beyond what cmd.exe does, which is to require opting in via the environment variable. But Python can choose to follow the more-modern behavior of PowerShell as a precedent here.

As Steve mentioned, technically a program isn't supposed to check for the environment variable, but instead call NeedCurrentDirectoryForExePath. This would require either adding a built-in function, e.g. to the _winapi module, or require ctypes:

 >>> kernel32.NeedCurrentDirectoryForExePathW('command')
 1
 >>> os.environ['NoDefaultCurrentDirectoryInExePath'] = '1'
 >>> kernel32.NeedCurrentDirectoryForExePathW('command')
 0

Note that it requires first normalizing the path to replace slashes with backslashes, which is unusual for the Windows API.

 >>> kernel32.NeedCurrentDirectoryForExePathW(r'.\command')
 1
 >>> kernel32.NeedCurrentDirectoryForExePathW('./command')
 0

[bharel]

Thanks eryksun.

Regarding the slashes, it should be fine as the input should be with
backslashes anyway. We don't need to fix the input as fixing it would
result in inaccurate data. Just as it won't work without backslashes on
CMD, it shouldn't work here.

On Mon, Oct 5, 2015 at 11:43 PM eryksun <report@bugs.python.org> wrote:

eryksun added the comment:

My suggestion is only for 3.5 / 3.6, because Windows XP is no longer
supported. Starting with Windows Vista, both cmd.exe and CreateProcess
support this behavior. I realize that disabling searching the current
director by default is going beyond what cmd.exe does, which is to require
opting in via the environment variable. But Python can choose to follow the
more-modern behavior of PowerShell as a precedent here.

As Steve mentioned, technically a program isn't supposed to check for the
environment variable, but instead call NeedCurrentDirectoryForExePath. This
would require either adding a built-in function, e.g. to the _winapi
module, or require ctypes:

\>\>\> kernel32.NeedCurrentDirectoryForExePathW('command')
1
\>\>\> os.environ['NoDefaultCurrentDirectoryInExePath'] = '1'
\>\>\> kernel32.NeedCurrentDirectoryForExePathW('command')
0

Note that it requires first normalizing the path to replace slashes with
backslashes, which is unusual for the Windows API.

\>\>\> kernel32.NeedCurrentDirectoryForExePathW(r'.\\command')
1
\>\>\> kernel32.NeedCurrentDirectoryForExePathW('./command')
0

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue24505\>

---

[tobytobkin]

I'm working on a patch and an accompanying unit test for this now. Should have it out today.