[Python-checkins] [3.7] bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12862)
Victor Stinner
webhook-mailer at python.org
Wed Apr 17 12:38:11 EDT 2019
https://github.com/python/cpython/commit/394b991e41a2a4ce3afc8e6fde44de46e73bbb34
commit: 394b991e41a2a4ce3afc8e6fde44de46e73bbb34
branch: 3.7
author: Victor Stinner <vstinner at redhat.com>
committer: GitHub <noreply at github.com>
date: 2019-04-17T18:38:06+02:00
summary:
[3.7] bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12862)
* bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12858)
shutil.which() and distutils.spawn.find_executable() now use
os.confstr("CS_PATH") if available instead of os.defpath, if the PATH
environment variable is not set.
Don't use os.confstr("CS_PATH") nor os.defpath if the PATH
environment variable is set to an empty string.
Changes:
* find_executable() now starts by checking for the executable in the
current working directly case. Add an explicit
"if not path: return None".
* Add tests for PATH='' (empty string), PATH=':' and for PATHEXT.
(cherry picked from commit 228a3c99bdb2d02771bead66a0beabafad3a90d3)
* bpo-35755: Remove current directory from posixpath.defpath (GH-11586)
Document the change in a NEWS entry of the Security category.
(cherry picked from commit 2c4c02f8a876fcf084575dcaf857a0236c81261a)
files:
A Misc/NEWS.d/next/Library/2019-04-16-17-50-39.bpo-35755.Fg4EXb.rst
A Misc/NEWS.d/next/Security/2019-01-17-10-03-48.bpo-35755.GmllIs.rst
M Lib/distutils/spawn.py
M Lib/distutils/tests/test_spawn.py
M Lib/posixpath.py
M Lib/shutil.py
M Lib/test/test_shutil.py
diff --git a/Lib/distutils/spawn.py b/Lib/distutils/spawn.py
index 538768809327..d3a12c283397 100644
--- a/Lib/distutils/spawn.py
+++ b/Lib/distutils/spawn.py
@@ -172,21 +172,32 @@ def find_executable(executable, path=None):
A string listing directories separated by 'os.pathsep'; defaults to
os.environ['PATH']. Returns the complete filename or None if not found.
"""
- if path is None:
- path = os.environ.get('PATH', os.defpath)
-
- paths = path.split(os.pathsep)
- base, ext = os.path.splitext(executable)
-
+ _, ext = os.path.splitext(executable)
if (sys.platform == 'win32') and (ext != '.exe'):
executable = executable + '.exe'
- if not os.path.isfile(executable):
- for p in paths:
- f = os.path.join(p, executable)
- if os.path.isfile(f):
- # the file exists, we have a shot at spawn working
- return f
- return None
- else:
+ if os.path.isfile(executable):
return executable
+
+ if path is None:
+ path = os.environ.get('PATH', None)
+ if path is None:
+ try:
+ path = os.confstr("CS_PATH")
+ except (AttributeError, ValueError):
+ # os.confstr() or CS_PATH is not available
+ path = os.defpath
+ # bpo-35755: Don't use os.defpath if the PATH environment variable is
+ # set to an empty string
+
+ # PATH='' doesn't match, whereas PATH=':' looks in the current directory
+ if not path:
+ return None
+
+ paths = path.split(os.pathsep)
+ for p in paths:
+ f = os.path.join(p, executable)
+ if os.path.isfile(f):
+ # the file exists, we have a shot at spawn working
+ return f
+ return None
diff --git a/Lib/distutils/tests/test_spawn.py b/Lib/distutils/tests/test_spawn.py
index 0d455385d8ac..f9ae69ef86b3 100644
--- a/Lib/distutils/tests/test_spawn.py
+++ b/Lib/distutils/tests/test_spawn.py
@@ -87,11 +87,52 @@ def test_find_executable(self):
rv = find_executable(dont_exist_program , path=tmp_dir)
self.assertIsNone(rv)
- # test os.defpath: missing PATH environment variable
+ # PATH='': no match, except in the current directory
with test_support.EnvironmentVarGuard() as env:
- with mock.patch('distutils.spawn.os.defpath', tmp_dir):
- env.pop('PATH')
+ env['PATH'] = ''
+ with unittest.mock.patch('distutils.spawn.os.confstr',
+ return_value=tmp_dir, create=True), \
+ unittest.mock.patch('distutils.spawn.os.defpath',
+ tmp_dir):
+ rv = find_executable(program)
+ self.assertIsNone(rv)
+
+ # look in current directory
+ with test_support.change_cwd(tmp_dir):
+ rv = find_executable(program)
+ self.assertEqual(rv, program)
+
+ # PATH=':': explicitly looks in the current directory
+ with test_support.EnvironmentVarGuard() as env:
+ env['PATH'] = os.pathsep
+ with unittest.mock.patch('distutils.spawn.os.confstr',
+ return_value='', create=True), \
+ unittest.mock.patch('distutils.spawn.os.defpath', ''):
+ rv = find_executable(program)
+ self.assertIsNone(rv)
+
+ # look in current directory
+ with test_support.change_cwd(tmp_dir):
+ rv = find_executable(program)
+ self.assertEqual(rv, program)
+
+ # missing PATH: test os.confstr("CS_PATH") and os.defpath
+ with test_support.EnvironmentVarGuard() as env:
+ env.pop('PATH', None)
+
+ # without confstr
+ with unittest.mock.patch('distutils.spawn.os.confstr',
+ side_effect=ValueError,
+ create=True), \
+ unittest.mock.patch('distutils.spawn.os.defpath',
+ tmp_dir):
+ rv = find_executable(program)
+ self.assertEqual(rv, filename)
+ # with confstr
+ with unittest.mock.patch('distutils.spawn.os.confstr',
+ return_value=tmp_dir, create=True), \
+ unittest.mock.patch('distutils.spawn.os.defpath', ''):
rv = find_executable(program)
self.assertEqual(rv, filename)
diff --git a/Lib/posixpath.py b/Lib/posixpath.py
index ca578a5df35c..785aa728b365 100644
--- a/Lib/posixpath.py
+++ b/Lib/posixpath.py
@@ -18,7 +18,7 @@
extsep = '.'
sep = '/'
pathsep = ':'
-defpath = ':/bin:/usr/bin'
+defpath = '/bin:/usr/bin'
altsep = None
devnull = '/dev/null'
diff --git a/Lib/shutil.py b/Lib/shutil.py
index f32c66b3550c..b0a53dba3a34 100644
--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -1138,7 +1138,17 @@ def _access_check(fn, mode):
return None
if path is None:
- path = os.environ.get("PATH", os.defpath)
+ path = os.environ.get("PATH", None)
+ if path is None:
+ try:
+ path = os.confstr("CS_PATH")
+ except (AttributeError, ValueError):
+ # os.confstr() or CS_PATH is not available
+ path = os.defpath
+ # bpo-35755: Don't use os.defpath if the PATH environment variable is
+ # set to an empty string
+
+ # PATH='' doesn't match, whereas PATH=':' looks in the current directory
if not path:
return None
path = path.split(os.pathsep)
diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index 6e2b1004d309..197dd130a964 100644
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -1500,6 +1500,57 @@ def test_environ_path(self):
rv = shutil.which(self.file)
self.assertEqual(rv, self.temp_file.name)
+ def test_environ_path_empty(self):
+ # PATH='': no match
+ with support.EnvironmentVarGuard() as env:
+ env['PATH'] = ''
+ with unittest.mock.patch('os.confstr', return_value=self.dir, \
+ create=True), \
+ support.swap_attr(os, 'defpath', self.dir), \
+ support.change_cwd(self.dir):
+ rv = shutil.which(self.file)
+ self.assertIsNone(rv)
+
+ def test_environ_path_cwd(self):
+ expected_cwd = os.path.basename(self.temp_file.name)
+ if sys.platform == "win32":
+ curdir = os.curdir
+ if isinstance(expected_cwd, bytes):
+ curdir = os.fsencode(curdir)
+ expected_cwd = os.path.join(curdir, expected_cwd)
+
+ # PATH=':': explicitly looks in the current directory
+ with support.EnvironmentVarGuard() as env:
+ env['PATH'] = os.pathsep
+ with unittest.mock.patch('os.confstr', return_value=self.dir, \
+ create=True), \
+ support.swap_attr(os, 'defpath', self.dir):
+ rv = shutil.which(self.file)
+ self.assertIsNone(rv)
+
+ # look in current directory
+ with support.change_cwd(self.dir):
+ rv = shutil.which(self.file)
+ self.assertEqual(rv, expected_cwd)
+
+ def test_environ_path_missing(self):
+ with support.EnvironmentVarGuard() as env:
+ env.pop('PATH', None)
+
+ # without confstr
+ with unittest.mock.patch('os.confstr', side_effect=ValueError, \
+ create=True), \
+ support.swap_attr(os, 'defpath', self.dir):
+ rv = shutil.which(self.file)
+ self.assertEqual(rv, self.temp_file.name)
+
+ # with confstr
+ with unittest.mock.patch('os.confstr', return_value=self.dir, \
+ create=True), \
+ support.swap_attr(os, 'defpath', ''):
+ rv = shutil.which(self.file)
+ self.assertEqual(rv, self.temp_file.name)
+
def test_empty_path(self):
base_dir = os.path.dirname(self.dir)
with support.change_cwd(path=self.dir), \
@@ -1514,6 +1565,23 @@ def test_empty_path_no_PATH(self):
rv = shutil.which(self.file)
self.assertIsNone(rv)
+ @unittest.skipUnless(sys.platform == "win32", 'test specific to Windows')
+ def test_pathext(self):
+ ext = ".xyz"
+ temp_filexyz = tempfile.NamedTemporaryFile(dir=self.temp_dir,
+ prefix="Tmp2", suffix=ext)
+ os.chmod(temp_filexyz.name, stat.S_IXUSR)
+ self.addCleanup(temp_filexyz.close)
+
+ # strip path and extension
+ program = os.path.basename(temp_filexyz.name)
+ program = os.path.splitext(program)[0]
+
+ with support.EnvironmentVarGuard() as env:
+ env['PATHEXT'] = ext
+ rv = shutil.which(program, path=self.temp_dir)
+ self.assertEqual(rv, temp_filexyz.name)
+
class TestMove(unittest.TestCase):
diff --git a/Misc/NEWS.d/next/Library/2019-04-16-17-50-39.bpo-35755.Fg4EXb.rst b/Misc/NEWS.d/next/Library/2019-04-16-17-50-39.bpo-35755.Fg4EXb.rst
new file mode 100644
index 000000000000..d84f63bf7b83
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-04-16-17-50-39.bpo-35755.Fg4EXb.rst
@@ -0,0 +1,5 @@
+:func:`shutil.which` and :func:`distutils.spawn.find_executable` now use
+``os.confstr("CS_PATH")`` if available instead of :data:`os.defpath`, if the
+``PATH`` environment variable is not set. Moreover, don't use
+``os.confstr("CS_PATH")`` nor :data:`os.defpath` if the ``PATH`` environment
+variable is set to an empty string.
diff --git a/Misc/NEWS.d/next/Security/2019-01-17-10-03-48.bpo-35755.GmllIs.rst b/Misc/NEWS.d/next/Security/2019-01-17-10-03-48.bpo-35755.GmllIs.rst
new file mode 100644
index 000000000000..959aafd73449
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2019-01-17-10-03-48.bpo-35755.GmllIs.rst
@@ -0,0 +1,5 @@
+:func:`shutil.which` now uses ``os.confstr("CS_PATH")`` if available and if the
+:envvar:`PATH` environment variable is not set. Remove also the current
+directory from :data:`posixpath.defpath`. On Unix, :func:`shutil.which` and the
+:mod:`subprocess` module no longer search the executable in the current
+directory if the :envvar:`PATH` environment variable is not set.
More information about the Python-checkins
mailing list