From b7a790246e52520b47a467fe89b81199b9cf03f6 Mon Sep 17 00:00:00 2001
From: Scott Moser <smoser@ubuntu.com>
Date: Fri, 9 Mar 2018 04:22:24 +0100
Subject: shellify: raise TypeError on bad input.

This makes 2 changes to shellify's behavior:
 a.) raise a TypeError rather than a RuntimeError.
 b.) raise a TypeError if input is not a list or tuple.
---
 cloudinit/tests/test_util.py                       | 23 ++++++++++++++++++++++
 cloudinit/util.py                                  | 14 +++++++++----
 .../unittests/test_handler/test_handler_bootcmd.py |  7 ++++---
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
index ba6bf699..c3e2e404 100644
--- a/cloudinit/tests/test_util.py
+++ b/cloudinit/tests/test_util.py
@@ -44,3 +44,26 @@ class TestUtil(CiTestCase):
         m_mount_info.return_value = ('/dev/sda1', 'btrfs', '/', 'ro,relatime')
         is_rw = util.mount_is_read_write('/')
         self.assertEqual(is_rw, False)
+
+
+class TestShellify(CiTestCase):
+
+    def test_input_dict_raises_type_error(self):
+        self.assertRaisesRegex(
+            TypeError, 'Input.*was.*dict.*xpected',
+            util.shellify, {'mykey': 'myval'})
+
+    def test_input_str_raises_type_error(self):
+        self.assertRaisesRegex(
+            TypeError, 'Input.*was.*str.*xpected', util.shellify, "foobar")
+
+    def test_value_with_int_raises_type_error(self):
+        self.assertRaisesRegex(
+            TypeError, 'shellify.*int', util.shellify, ["foo", 1])
+
+    def test_supports_strings_and_lists(self):
+        self.assertEqual(
+            '\n'.join(["#!/bin/sh", "echo hi mom", "'echo' 'hi dad'",
+                       "'echo' 'hi' 'sis'", ""]),
+            util.shellify(["echo hi mom", ["echo", "hi dad"],
+                           ('echo', 'hi', 'sis')]))
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 1d6cd1c5..083a8efe 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1922,6 +1922,11 @@ def abs_join(*paths):
 #    if it is an array, shell protect it (with single ticks)
 #    if it is a string, do nothing
 def shellify(cmdlist, add_header=True):
+    if not isinstance(cmdlist, (tuple, list)):
+        raise TypeError(
+            "Input to shellify was type '%s'. Expected list or tuple." %
+            (type_utils.obj_name(cmdlist)))
+
     content = ''
     if add_header:
         content += "#!/bin/sh\n"
@@ -1930,7 +1935,7 @@ def shellify(cmdlist, add_header=True):
     for args in cmdlist:
         # If the item is a list, wrap all items in single tick.
         # If its not, then just write it directly.
-        if isinstance(args, list):
+        if isinstance(args, (list, tuple)):
             fixed = []
             for f in args:
                 fixed.append("'%s'" % (six.text_type(f).replace("'", escaped)))
@@ -1940,9 +1945,10 @@ def shellify(cmdlist, add_header=True):
             content = "%s%s\n" % (content, args)
             cmds_made += 1
         else:
-            raise RuntimeError(("Unable to shellify type %s"
-                                " which is not a list or string")
-                               % (type_utils.obj_name(args)))
+            raise TypeError(
+                "Unable to shellify type '%s'. Expected list, string, tuple. "
+                "Got: %s" % (type_utils.obj_name(args), args))
+
     LOG.debug("Shellified %s commands.", cmds_made)
     return content
 
diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py
index dbf43e0d..09d4c681 100644
--- a/tests/unittests/test_handler/test_handler_bootcmd.py
+++ b/tests/unittests/test_handler/test_handler_bootcmd.py
@@ -69,7 +69,7 @@ class TestBootcmd(CiTestCase):
             cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
         self.assertIn('Failed to shellify bootcmd', self.logs.getvalue())
         self.assertEqual(
-            "'int' object is not iterable",
+            "Input to shellify was type 'int'. Expected list or tuple.",
             str(context_manager.exception))
 
     @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
@@ -98,7 +98,7 @@ class TestBootcmd(CiTestCase):
         invalid_config = {
             'bootcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]}
         cc = self._get_cloud('ubuntu')
-        with self.assertRaises(RuntimeError) as context_manager:
+        with self.assertRaises(TypeError) as context_manager:
             cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
         expected_warnings = [
             'bootcmd.1: 20 is not valid under any of the given schemas',
@@ -110,7 +110,8 @@ class TestBootcmd(CiTestCase):
             self.assertIn(warning, logs)
         self.assertIn('Failed to shellify', logs)
         self.assertEqual(
-            'Unable to shellify type int which is not a list or string',
+            ("Unable to shellify type 'int'. Expected list, string, tuple. "
+             "Got: 20"),
             str(context_manager.exception))
 
     def test_handler_creates_and_runs_bootcmd_script_with_instance_id(self):
-- 
cgit v1.2.3