| 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) *  |
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) * |
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) * |
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) * |
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) * |
Date: 2013-01-21 10:30 |
| LGTM. |
|
|
| msg180375 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
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) * |
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 |
|
|