classification
Title: shutil.which() shouldn't look in working directory on unix-y systems
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: eric.araujo, hynek, python-dev, serhiy.storchaka, takluyver, tarek
Priority: normal Keywords: patch

Created on 2013-01-13 22:13 by takluyver, last changed 2013-01-25 08:18 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
shutil_which_cwd.patch takluyver, 2013-01-17 23:29 Patch, including test review
shutil_which_cwd2.patch takluyver, 2013-01-18 11:19 Patch, version 2 review
shutil_which_cwd3.patch takluyver, 2013-01-18 22:31 version 2 + comment change review
shutil_which_cwd4.patch serhiy.storchaka, 2013-01-22 12:11 review
Messages (13)
msg179897 - (view) Author: Thomas Kluyver (takluyver) * Date: 2013-01-13 22:13
There's a 'short circuit' in shutil.which(), described as 'If we're given a full path which matches the mode and it exists, we're done here.'

It also matches if an executable file of the same name is present in the working directory, although on most Unix-y systems you need ./ to execute such files in a shell (i.e. ./foo, not just foo).

This could fool code calling which() into thinking that a program is installed, when it is not.

If we consider this a bug, one simple fix would be to only allow the short circuit with an absolute path, so the line

    if _access_check(cmd, mode):

would become

    if os.path.isabs(cmd) and _access_check(cmd, mode):
msg180160 - (view) Author: Thomas Kluyver (takluyver) * Date: 2013-01-17 23:29
I've added a patch with my suggested fix, as well as a test for this.

test_shutil all passes on Linux - I haven't run the tests on Windows.
msg180177 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-18 07:37
I think it should be

    if os.path.dirname(cmd) and _access_check(cmd, mode):
msg180178 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-18 07:42
No, it should be

    if os.path.dirname(cmd):
        if _access_check(cmd, mode):
            return cmd
        return None
msg180188 - (view) Author: Thomas Kluyver (takluyver) * Date: 2013-01-18 11:19
That makes sense - foo/setup.py can be run from the working directory, but you can't refer to subdirectories on $PATH like that.

I've added a revised version of the patch.
msg180199 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2013-01-18 17:52
I assume that ./script is working just like dir/script (what the tests exercize is not crystal clear to me).
msg180215 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-18 19:36
Perhaps the comment in which() is misleading now, if I understand correctly the meaning of the words "short circuit". Native speakers, please correct me if I wrong.
msg180228 - (view) Author: Thomas Kluyver (takluyver) * Date: 2013-01-18 22:31
Yes, as far as I know, ./script works in the same way as dir/script - from the current directory, but not from $PATH.

The first test added is for the case I reported - which('script') shouldn't look in the current directory on Unix. The second test would have failed without Serhiy's correction.

Serhiy, I agree that the comment wasn't quite right. I've uploaded a new version of the patch with a modified comment.
msg180335 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-21 10:30
LGTM.
msg180375 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-22 12:11
I have reorganized tests a little.
msg180450 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-23 08:48
New changeset f18d11ab53a0 by Serhiy Storchaka in branch '3.3':
Issue #16957: shutil.which() no longer searches a bare file name in the
http://hg.python.org/cpython/rev/f18d11ab53a0

New changeset 7b51568cfbae by Serhiy Storchaka in branch 'default':
Issue #16957: shutil.which() no longer searches a bare file name in the
http://hg.python.org/cpython/rev/7b51568cfbae
msg180451 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-23 08:53
Committed. Thank you for for the patch.
msg180557 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-25 08:18
New changeset 99db73ce8374 by Serhiy Storchaka in branch '3.3':
Fix pathext test for shutil.which() which was
http://hg.python.org/cpython/rev/99db73ce8374

New changeset ab0ff935126c by Serhiy Storchaka in branch 'default':
Fix pathext test for shutil.which() which was
http://hg.python.org/cpython/rev/ab0ff935126c
History
Date User Action Args
2013-01-25 08:18:02python-devsetmessages: + msg180557
2013-01-23 08:53:05serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg180451

stage: commit review -> resolved
2013-01-23 08:48:04python-devsetnosy: + python-dev
messages: + msg180450
2013-01-22 12:11:37serhiy.storchakasetfiles: + shutil_which_cwd4.patch
versions: + Python 3.4
messages: + msg180375

assignee: serhiy.storchaka
stage: patch review -> commit review
2013-01-21 10:30:09serhiy.storchakasetmessages: + msg180335
2013-01-18 22:31:22takluyversetfiles: + shutil_which_cwd3.patch

messages: + msg180228
versions: - Python 3.4
2013-01-18 19:36:37serhiy.storchakasetmessages: + msg180215
2013-01-18 17:52:24eric.araujosetmessages: + msg180199
2013-01-18 17:49:53eric.araujosetnosy: + eric.araujo
stage: patch review

versions: + Python 3.4
2013-01-18 11:19:36takluyversetfiles: + shutil_which_cwd2.patch

messages: + msg180188
2013-01-18 07:42:34serhiy.storchakasetmessages: + msg180178
2013-01-18 07:37:50serhiy.storchakasetnosy: + hynek, tarek, serhiy.storchaka
messages: + msg180177
2013-01-17 23:29:30takluyversetfiles: + shutil_which_cwd.patch
keywords: + patch
messages: + msg180160
2013-01-13 22:13:51takluyvercreate