Quote arguments in aliases

.changes/next-release/bugfix-Aliases-70773.json

@@ -0,0 +1,5 @@
+{
+  "description": "Properly quote alias parameters that have spaces in them. Fixes `#2653 <https://github.com/aws/aws-cli/issues/2653>`__.",
+  "category": "Aliases",
+  "type": "bugfix"
+}

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',

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

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

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)