From 1636b3b0530f2e8422997fff810188e4676de511 Mon Sep 17 00:00:00 2001
From: Steven Hardy <shardy@redhat.com>
Date: Wed, 13 Jan 2016 10:34:18 +0000
Subject: [PATCH] Handle invalid stack names which are non-string

If we get passed a non-string stack name, e.g a map or list, we
fail with a DB error associated with looking up the existing stack.

So instead force all stack lookups to use string identifiers, and
make the name validation for new stacks robust to fail gracefully
when there is an invalid (non string) argument passed.

Change-Id: I052dc4a715773895d070e1e9f26183c6a1cf3d7f
Closes-Bug: #1533065
---
 heat/engine/stack.py              | 12 ++++++++----
 heat/objects/stack.py             |  6 ++++--
 heat/tests/test_engine_service.py | 12 ++++++++++++
 heat/tests/test_stack.py          | 16 ++++++++++++++--
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index fdda436d9..5ad28aeac 100644
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -134,10 +134,14 @@ class Stack(collections.Mapping):
         """
 
         def _validate_stack_name(name):
-            if not re.match("[a-zA-Z][a-zA-Z0-9_.-]*$", name):
-                message = _('Invalid stack name %s must contain '
-                            'only alphanumeric or \"_-.\" characters, '
-                            'must start with alpha') % name
+            try:
+                if not re.match("[a-zA-Z][a-zA-Z0-9_.-]*$", name):
+                    message = _('Invalid stack name %s must contain '
+                                'only alphanumeric or \"_-.\" characters, '
+                                'must start with alpha') % name
+                    raise exception.StackValidationFailed(message=message)
+            except TypeError:
+                message = _('Invalid stack name %s, must be a string') % name
                 raise exception.StackValidationFailed(message=message)
 
         if owner_id is None:
diff --git a/heat/objects/stack.py b/heat/objects/stack.py
index dbaeea862..9d5117bf7 100644
--- a/heat/objects/stack.py
+++ b/heat/objects/stack.py
@@ -15,6 +15,8 @@
 
 """Stack object."""
 
+import six
+
 from oslo_versionedobjects import base
 from oslo_versionedobjects import fields
 
@@ -95,7 +97,7 @@ class Stack(
     def get_by_name_and_owner_id(cls, context, stack_name, owner_id):
         db_stack = db_api.stack_get_by_name_and_owner_id(
             context,
-            stack_name,
+            six.text_type(stack_name),
             owner_id
         )
         if not db_stack:
@@ -105,7 +107,7 @@ class Stack(
 
     @classmethod
     def get_by_name(cls, context, stack_name):
-        db_stack = db_api.stack_get_by_name(context, stack_name)
+        db_stack = db_api.stack_get_by_name(context, six.text_type(stack_name))
         if not db_stack:
             return None
         stack = cls._from_db_object(context, cls(context), db_stack)
diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py
index 9d89ba9db..aad16c84f 100644
--- a/heat/tests/test_engine_service.py
+++ b/heat/tests/test_engine_service.py
@@ -469,6 +469,18 @@ class StackServiceTest(common.HeatTestCase):
             ctx2,
             self.stack.name))
 
+    @tools.stack_context('service_badname_test_stack', False)
+    def test_stack_by_name_badname(self):
+        # If a bad name type, such as a map, is passed, we should just return
+        # None, as it's converted to a string, which won't match any name
+        ctx = utils.dummy_context(tenant_id='stack_service_test_tenant')
+        self.assertIsNone(stack_object.Stack.get_by_name(
+            ctx,
+            {'notallowed': self.stack.name}))
+        self.assertIsNone(stack_object.Stack.get_by_name_and_owner_id(
+            ctx,
+            {'notallowed': self.stack.name}, 'owner'))
+
     @tools.stack_context('service_list_all_test_stack')
     def test_stack_list_all(self):
         self.m.StubOutWithMock(parser.Stack, '_from_db')
diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py
index 92507048f..efb64fe9b 100644
--- a/heat/tests/test_stack.py
+++ b/heat/tests/test_stack.py
@@ -1077,8 +1077,20 @@ class StackTest(common.HeatTestCase):
                        'test/stack', 'test\stack', 'test::stack', 'test;stack',
                        'test~stack', '#test']
         for stack_name in stack_names:
-            self.assertRaises(exception.StackValidationFailed, stack.Stack,
-                              self.ctx, stack_name, self.tmpl)
+            ex = self.assertRaises(
+                exception.StackValidationFailed, stack.Stack,
+                self.ctx, stack_name, self.tmpl)
+            self.assertIn("Invalid stack name %s must contain" % stack_name,
+                          six.text_type(ex))
+
+    def test_stack_name_invalid_type(self):
+        stack_names = [{"bad": 123}, ["no", "lists"]]
+        for stack_name in stack_names:
+            ex = self.assertRaises(
+                exception.StackValidationFailed, stack.Stack,
+                self.ctx, stack_name, self.tmpl)
+            self.assertIn("Invalid stack name %s, must be a string"
+                          % stack_name, six.text_type(ex))
 
     def test_resource_state_get_att(self):
         tmpl = {