From 3b6299fb160d26da1fcd821c09f8fa74e83a2d6b Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Wed, 18 Sep 2019 17:53:35 -0700 Subject: [PATCH] Porting PR #51785 to 2019.2.1 --- salt/utils/path.py | 165 +++++++++++++++++++++------------- tests/unit/utils/test_path.py | 120 ++++++++++++++----------- 2 files changed, 170 insertions(+), 115 deletions(-) diff --git a/salt/utils/path.py b/salt/utils/path.py index b1d601e464f6..24e1d9327304 100644 --- a/salt/utils/path.py +++ b/salt/utils/path.py @@ -20,7 +20,6 @@ import salt.utils.platform import salt.utils.stringutils from salt.exceptions import CommandNotFoundError -from salt.utils.decorators import memoize as real_memoize from salt.utils.decorators.jinja import jinja_filter # Import 3rd-party libs @@ -191,70 +190,110 @@ def which(exe=None): ''' Python clone of /usr/bin/which ''' - def _is_executable_file_or_link(exe): - # check for os.X_OK doesn't suffice because directory may executable - return (os.access(exe, os.X_OK) and - (os.path.isfile(exe) or os.path.islink(exe))) - - if exe: - if _is_executable_file_or_link(exe): - # executable in cwd or fullpath - return exe - - ext_list = salt.utils.stringutils.to_str( - os.environ.get('PATHEXT', str('.EXE')) - ).split(str(';')) - - @real_memoize - def _exe_has_ext(): - ''' - Do a case insensitive test if exe has a file extension match in - PATHEXT - ''' - for ext in ext_list: - try: - pattern = r'.*\.{0}$'.format( - salt.utils.stringutils.to_unicode(ext).lstrip('.') - ) - re.match( - pattern, - salt.utils.stringutils.to_unicode(exe), - re.I).groups() - return True - except AttributeError: - continue - return False - - # Enhance POSIX path for the reliability at some environments, when $PATH is changing - # This also keeps order, where 'first came, first win' for cases to find optional alternatives - system_path = salt.utils.stringutils.to_unicode(os.environ.get('PATH', '')) - search_path = system_path.split(os.pathsep) - if not salt.utils.platform.is_windows(): - search_path.extend([ - x for x in ('/bin', '/sbin', '/usr/bin', - '/usr/sbin', '/usr/local/bin') - if x not in search_path - ]) - - for path in search_path: - full_path = join(path, exe) - if _is_executable_file_or_link(full_path): - return full_path - elif salt.utils.platform.is_windows() and not _exe_has_ext(): - # On Windows, check for any extensions in PATHEXT. - # Allows both 'cmd' and 'cmd.exe' to be matched. - for ext in ext_list: - # Windows filesystem is case insensitive so we - # safely rely on that behavior - if _is_executable_file_or_link(full_path + ext): - return full_path + ext - log.trace( - '\'%s\' could not be found in the following search path: \'%s\'', - exe, search_path - ) - else: + + if not exe: log.error('No executable was passed to be searched by salt.utils.path.which()') + return None + + ## define some utilities (we use closures here because our predecessor used them) + def is_executable_common(path): + ''' + This returns truth if posixy semantics (which python simulates on + windows) states that this is executable. + ''' + return os.path.isfile(path) and os.access(path, os.X_OK) + + def resolve(path): + ''' + This will take a path and recursively follow the link until we get to a + real file. + ''' + while os.path.islink(path): + res = os.readlink(path) + + # if the link points to a relative target, then convert it to an + # absolute path relative to the original path + if not os.path.isabs(res): + directory, _ = os.path.split(path) + res = join(directory, res) + path = res + return path + + # windows-only + def has_executable_ext(path, ext_membership): + ''' + Extract the extension from the specified path, lowercase it so we + can be insensitive, and then check it against the available exts. + ''' + p, ext = os.path.splitext(path) + return ext.lower() in ext_membership + + ## prepare related variables from the environment + res = salt.utils.stringutils.to_unicode(os.environ.get('PATH', '')) + system_path = res.split(os.pathsep) + + # add some reasonable defaults in case someone's PATH is busted + if not salt.utils.platform.is_windows(): + res = set(system_path) + extended_path = ['/sbin', '/bin', '/usr/sbin', '/usr/bin', '/usr/local/sbin', '/usr/local/bin'] + system_path.extend([p for p in extended_path if p not in res]) + + ## now to define the semantics of what's considered executable on a given platform + if salt.utils.platform.is_windows(): + # executable semantics on windows requires us to search PATHEXT + res = salt.utils.stringutils.to_str(os.environ.get('PATHEXT', str('.EXE'))) + + # generate two variables, one of them for O(n) searches (but ordered) + # and another for O(1) searches. the previous guy was trying to use + # memoization with a function that has no arguments, this provides + # the exact same benefit + pathext = res.split(os.pathsep) + res = {ext.lower() for ext in pathext} + + # check if our caller already specified a valid extension as then we don't need to match it + _, ext = os.path.splitext(exe) + if ext.lower() in res: + pathext = [''] + + is_executable = is_executable_common + + # The specified extension isn't valid, so we just assume it's part of the + # filename and proceed to walk the pathext list + else: + is_executable = lambda path, membership=res: is_executable_common(path) and has_executable_ext(path, membership) + + else: + # in posix, there's no such thing as file extensions..only zuul + pathext = [''] + + # executable semantics are pretty simple on reasonable platforms... + is_executable = is_executable_common + + ## search for the executable + + # check to see if the full path was specified as then we don't need + # to actually walk the system_path for any reason + if is_executable(exe): + return exe + # now to search through our system_path + for path in system_path: + p = join(path, exe) + + # iterate through all extensions to see which one is executable + for ext in pathext: + pext = p + ext + rp = resolve(pext) + if is_executable(rp): + return p + ext + continue + continue + + ## if something was executable, we should've found it already... + log.trace( + '\'%s\' could not be found in the following search path: \'%s\'', + exe, system_path + ) return None diff --git a/tests/unit/utils/test_path.py b/tests/unit/utils/test_path.py index a693492cc870..80df612ec85d 100644 --- a/tests/unit/utils/test_path.py +++ b/tests/unit/utils/test_path.py @@ -186,27 +186,28 @@ class TestWhich(TestCase): # The mock patch below will make sure that ALL calls to the which function # returns None def test_missing_binary_in_linux(self): - with patch('salt.utils.path.which', lambda exe: None): - self.assertTrue( - salt.utils.path.which('this-binary-does-not-exist') is None - ) + # salt.utils.path.which uses platform.is_windows to determine the platform, so we're using linux here + with patch('salt.utils.platform.is_windows', lambda: False): + with patch('salt.utils.path.which', lambda exe: None): + self.assertTrue( + salt.utils.path.which('this-binary-does-not-exist') is None + ) # The mock patch below will make sure that ALL calls to the which function # return whatever is sent to it def test_existing_binary_in_linux(self): - with patch('salt.utils.path.which', lambda exe: exe): - self.assertTrue(salt.utils.path.which('this-binary-exists-under-linux')) + # salt.utils.path.which uses platform.is_windows to determine the platform, so we're using linux here + with patch('salt.utils.platform.is_windows', lambda: False): + with patch('salt.utils.path.which', lambda exe: exe): + self.assertTrue(salt.utils.path.which('this-binary-exists-under-linux')) def test_existing_binary_in_windows(self): - with patch('os.access') as osaccess: + with patch('os.path.isfile') as isfile: # We define the side_effect attribute on the mocked object in order to - # specify which calls return which values. First call to os.access + # specify which calls return which values. First call to os.path.isfile # returns X, the second Y, the third Z, etc... - osaccess.side_effect = [ - # The first os.access should return False(the abspath one) - False, - # The second, iterating through $PATH, should also return False, - # still checking for Linux + isfile.side_effect = [ + # The first os.path.isfile should return False due to checking the explicit path (first is_executable) False, # We will now also return False once so we get a .EXE back from # the function, see PATHEXT below. @@ -214,21 +215,27 @@ def test_existing_binary_in_windows(self): # Lastly return True, this is the windows check. True ] - # Let's patch os.environ to provide a custom PATH variable - with patch.dict(os.environ, {'PATH': os.sep + 'bin', - 'PATHEXT': '.COM;.EXE;.BAT;.CMD'}): - # Let's also patch is_windows to return True - with patch('salt.utils.platform.is_windows', lambda: True): - with patch('os.path.isfile', lambda x: True): - self.assertEqual( - salt.utils.path.which('this-binary-exists-under-windows'), - os.path.join(os.sep + 'bin', 'this-binary-exists-under-windows.EXE') - ) + + # Patch os.access so that it always returns True + with patch('os.access', lambda path, mode: True): + # Disable os.path.islink + with patch('os.path.islink', lambda path: False): + # we're using ';' as os.pathsep in this test + with patch('os.pathsep', ';'): + # Let's patch os.environ to provide a custom PATH variable + with patch.dict(os.environ, {'PATH': os.sep + 'bin', + 'PATHEXT': '.COM;.EXE;.BAT;.CMD'}): + # Let's also patch is_windows to return True + with patch('salt.utils.platform.is_windows', lambda: True): + self.assertEqual( + salt.utils.path.which('this-binary-exists-under-windows'), + os.path.join(os.sep + 'bin', 'this-binary-exists-under-windows.EXE') + ) def test_missing_binary_in_windows(self): with patch('os.access') as osaccess: osaccess.side_effect = [ - # The first os.access should return False(the abspath one) + # The first os.access should return False due to checking the explicit path (first is_executable) False, # The second, iterating through $PATH, should also return False, # still checking for Linux @@ -236,27 +243,26 @@ def test_missing_binary_in_windows(self): # be called 5 times False, False, False, False, False ] - # Let's patch os.environ to provide a custom PATH variable - with patch.dict(os.environ, {'PATH': os.sep + 'bin'}): - # Let's also patch is_widows to return True - with patch('salt.utils.platform.is_windows', lambda: True): - self.assertEqual( - # Since we're passing the .exe suffix, the last True above - # will not matter. The result will be None - salt.utils.path.which('this-binary-is-missing-in-windows.exe'), - None - ) + # we're using ';' as os.pathsep in this test + with patch('os.pathsep', ';'): + # Let's patch os.environ to provide a custom PATH variable + with patch.dict(os.environ, {'PATH': os.sep + 'bin'}): + # Let's also patch is_widows to return True + with patch('salt.utils.platform.is_windows', lambda: True): + self.assertEqual( + # Since we're passing the .exe suffix, the last True above + # will not matter. The result will be None + salt.utils.path.which('this-binary-is-missing-in-windows.exe'), + None + ) def test_existing_binary_in_windows_pathext(self): - with patch('os.access') as osaccess: + with patch('os.path.isfile') as isfile: # We define the side_effect attribute on the mocked object in order to - # specify which calls return which values. First call to os.access + # specify which calls return which values. First call to os.path.isfile # returns X, the second Y, the third Z, etc... - osaccess.side_effect = [ - # The first os.access should return False(the abspath one) - False, - # The second, iterating through $PATH, should also return False, - # still checking for Linux + isfile.side_effect = [ + # The first os.path.isfile should return False due to checking the explicit path (first is_executable) False, # We will now also return False 3 times so we get a .CMD back from # the function, see PATHEXT below. @@ -264,14 +270,24 @@ def test_existing_binary_in_windows_pathext(self): False, False, False, True ] - # Let's patch os.environ to provide a custom PATH variable - with patch.dict(os.environ, {'PATH': os.sep + 'bin', - 'PATHEXT': '.COM;.EXE;.BAT;.CMD;.VBS;' - '.VBE;.JS;.JSE;.WSF;.WSH;.MSC;.PY'}): - # Let's also patch is_windows to return True - with patch('salt.utils.platform.is_windows', lambda: True): - with patch('os.path.isfile', lambda x: True): - self.assertEqual( - salt.utils.path.which('this-binary-exists-under-windows'), - os.path.join(os.sep + 'bin', 'this-binary-exists-under-windows.CMD') - ) + + # Patch os.access so that it always returns True + with patch('os.access', lambda path, mode: True): + + # Disable os.path.islink + with patch('os.path.islink', lambda path: False): + + # we're using ';' as os.pathsep in this test + with patch('os.pathsep', ';'): + + # Let's patch os.environ to provide a custom PATH variable + with patch.dict(os.environ, {'PATH': os.sep + 'bin', + 'PATHEXT': '.COM;.EXE;.BAT;.CMD;.VBS;' + '.VBE;.JS;.JSE;.WSF;.WSH;.MSC;.PY'}): + + # Let's also patch is_windows to return True + with patch('salt.utils.platform.is_windows', lambda: True): + self.assertEqual( + salt.utils.path.which('this-binary-exists-under-windows'), + os.path.join(os.sep + 'bin', 'this-binary-exists-under-windows.CMD') + )