diff --git a/.changes/next-release/bugfix-Aliases-70773.json b/.changes/next-release/bugfix-Aliases-70773.json new file mode 100644 index 000000000000..1928e933dd0e --- /dev/null +++ b/.changes/next-release/bugfix-Aliases-70773.json @@ -0,0 +1,5 @@ +{ + "description": "Properly quote alias parameters that have spaces in them. Fixes `#2653 `__.", + "category": "Aliases", + "type": "bugfix" +} diff --git a/awscli/alias.py b/awscli/alias.py index 90feb6d779bf..31e58adfc790 100644 --- a/awscli/alias.py +++ b/awscli/alias.py @@ -17,6 +17,7 @@ from botocore.configloader import raw_config_parse +from awscli.compat import compat_shell_quote from awscli.commands import CLICommand from awscli.utils import emit_top_level_args_parsed_event @@ -274,7 +275,7 @@ def __call__(self, args, parsed_globals): command_components = [ self._alias_value[1:] ] - command_components.extend(args) + command_components.extend(compat_shell_quote(a) for a in args) command = ' '.join(command_components) LOG.debug( 'Using external alias %r with value: %r to run: %r', diff --git a/awscli/compat.py b/awscli/compat.py index 0a7a7bec71fd..19bf70acc4e4 100644 --- a/awscli/compat.py +++ b/awscli/compat.py @@ -179,3 +179,81 @@ def compat_input(prompt): sys.stdout.write(prompt) sys.stdout.flush() return raw_input() + + +def compat_shell_quote(s, platform=None): + """Return a shell-escaped version of the string *s* + + Unfortunately `shlex.quote` doesn't support Windows, so this method + provides that functionality. + """ + if platform is None: + platform = sys.platform + + if platform == "win32": + return _windows_shell_quote(s) + else: + return shlex_quote(s) + + +def _windows_shell_quote(s): + """Return a Windows shell-escaped version of the string *s* + + Windows has potentially bizarre rules depending on where you look. When + spawning a process via the Windows C runtime the rules are as follows: + + https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments + + To summarize the relevant bits: + + * Only space and tab are valid delimiters + * Double quotes are the only valid quotes + * Backslash is interpreted literally unless it is part of a chain that + leads up to a double quote. Then the backslashes escape the backslashes, + and if there is an odd number the final backslash escapes the quote. + + :param s: A string to escape + :return: An escaped string + """ + if not s: + return '""' + + buff = [] + num_backspaces = 0 + for character in s: + if character == '\\': + # We can't simply append backslashes because we don't know if + # they will need to be escaped. Instead we separately keep track + # of how many we've seen. + num_backspaces += 1 + elif character == '"': + if num_backspaces > 0: + # The backslashes are part of a chain that lead up to a + # double quote, so they need to be escaped. + buff.append('\\' * (num_backspaces * 2)) + num_backspaces = 0 + + # The double quote also needs to be escaped. The fact that we're + # seeing it at all means that it must have been escaped in the + # original source. + buff.append('\\"') + else: + if num_backspaces > 0: + # The backslashes aren't part of a chain leading up to a + # double quote, so they can be inserted directly without + # being escaped. + buff.append('\\' * num_backspaces) + num_backspaces = 0 + buff.append(character) + + # There may be some leftover backspaces if they were on the trailing + # end, so they're added back in here. + if num_backspaces > 0: + buff.append('\\' * num_backspaces) + + new_s = ''.join(buff) + if ' ' in new_s or '\t' in new_s: + # If there are any spaces or tabs then the string needs to be double + # quoted. + return '"%s"' % new_s + return new_s diff --git a/tests/functional/test_alias.py b/tests/functional/test_alias.py index be3d47bdbcc0..5d184afc5bf8 100644 --- a/tests/functional/test_alias.py +++ b/tests/functional/test_alias.py @@ -211,6 +211,12 @@ def test_external_alias_then_additonal_args(self): self.run_cmd('mkdir %s' % directory_to_make) self.assertTrue(os.path.isdir(directory_to_make)) + def test_external_alias_with_quoted_arguments(self): + directory_to_make = os.path.join(self.files.rootdir, 'new dir') + self.add_alias('mkdir', '!mkdir') + self.run_cmd(['mkdir', directory_to_make]) + self.assertTrue(os.path.isdir(directory_to_make)) + @skip_if_windows('Windows does not support BASH functions') def test_external_alias_with_wrapper_bash_function(self): # The external alias is tested by using mkdir; a command that diff --git a/tests/unit/test_compat.py b/tests/unit/test_compat.py index b4a65ed6f53b..11cbdaa46724 100644 --- a/tests/unit/test_compat.py +++ b/tests/unit/test_compat.py @@ -10,10 +10,11 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. - -from awscli.compat import ensure_text_type +from nose.tools import assert_equal from botocore.compat import six +from awscli.compat import ensure_text_type +from awscli.compat import compat_shell_quote from awscli.testutils import unittest @@ -46,3 +47,39 @@ def test_non_string_or_bytes_raises_error(self): value = 500 with self.assertRaises(ValueError): ensure_text_type(value) + + +def test_compat_shell_quote_windows(): + windows_cases = { + '': '""', + '"': '\\"', + '\\': '\\', + '\\a': '\\a', + '\\\\': '\\\\', + '\\"': '\\\\\\"', + '\\\\"': '\\\\\\\\\\"', + 'foo bar': '"foo bar"', + 'foo\tbar': '"foo\tbar"', + } + for input_string, expected_output in windows_cases.items(): + yield ShellQuoteTestCase().run, input_string, expected_output, "win32" + + +def test_comat_shell_quote_unix(): + unix_cases = { + "": "''", + "*": "'*'", + "foo": "foo", + "foo bar": "'foo bar'", + "foo\tbar": "'foo\tbar'", + "foo\nbar": "'foo\nbar'", + "foo'bar": "'foo'\"'\"'bar'", + } + for input_string, expected_output in unix_cases.items(): + yield ShellQuoteTestCase().run, input_string, expected_output, "linux2" + yield ShellQuoteTestCase().run, input_string, expected_output, "darwin" + + +class ShellQuoteTestCase(object): + def run(self, s, expected, platform=None): + assert_equal(compat_shell_quote(s, platform), expected)