Validate limit query parameter for List Software Config API

This change adds validation of the limit query parameter in List
Software Config API, as was implemented for List Stack API, to avoid
internal error at database query.

story: 2009707
task: 44054
Change-Id: Ib57919faebbd4eb6aa13857e242eb5f3dc448a02

3 files changed, 111 insertions, 0 deletions

diff --git a/heat/api/openstack/v1/software_configs.py b/heat/api/openstack/v1/software_configs.py
index 41f9486e4..c27da4fe7 100644
--- a/heat/api/openstack/v1/software_configs.py
+++ b/heat/api/openstack/v1/software_configs.py
@@ -43,6 +43,14 @@ class SoftwareConfigController(object):
         except ValueError as e:
             raise exc.HTTPBadRequest(str(e))
 
+    def _extract_int_param(self, name, value,
+                           allow_zero=True, allow_negative=False):
+        try:
+            return param_utils.extract_int(name, value,
+                                           allow_zero, allow_negative)
+        except ValueError as e:
+            raise exc.HTTPBadRequest(str(e))
+
     def _index(self, req, use_admin_cnxt=False):
         param_types = {
             'limit': util.PARAM_TYPE_SINGLE,
@@ -50,6 +58,10 @@ class SoftwareConfigController(object):
         }
         params = util.get_allowed_params(req.params, param_types)
 
+        key = rpc_api.PARAM_LIMIT
+        if key in params:
+            params[key] = self._extract_int_param(key, params[key])
+
         if use_admin_cnxt:
             cnxt = context.get_admin_context()
         else:
diff --git a/heat/tests/api/openstack_v1/test_software_configs.py b/heat/tests/api/openstack_v1/test_software_configs.py
index c1516eda0..110bfaef5 100644
--- a/heat/tests/api/openstack_v1/test_software_configs.py
+++ b/heat/tests/api/openstack_v1/test_software_configs.py
@@ -47,6 +47,41 @@ class SoftwareConfigControllerTest(tools.ControllerTest, common.HeatTestCase):
                 {'software_configs': []}, resp)
 
     @mock.patch.object(policy.Enforcer, 'enforce')
+    def test_index_limit_negative(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'index')
+        params = {'limit': -1}
+
+        with mock.patch.object(
+                self.controller.rpc_client,
+                'list_software_configs',
+                return_value=[]) as mock_call:
+            req = self._get('/software_configs', params=params)
+            ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                                   self.controller.index, req,
+                                   tenant_id=self.tenant)
+            self.assertEqual("Value '-1' is invalid for 'limit' which only "
+                             "accepts non-negative integer.",
+                             str(ex))
+            self.assertFalse(mock_call.called)
+
+    @mock.patch.object(policy.Enforcer, 'enforce')
+    def test_index_limit_not_int(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'index')
+        params = {'limit': 'not-an-int'}
+
+        with mock.patch.object(
+                self.controller.rpc_client,
+                'list_software_configs',
+                return_value=[]) as mock_call:
+            req = self._get('/software_configs', params=params)
+            ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                                   self.controller.index, req,
+                                   tenant_id=self.tenant)
+            self.assertEqual("Only integer is acceptable by 'limit'.",
+                             str(ex))
+            self.assertFalse(mock_call.called)
+
+    @mock.patch.object(policy.Enforcer, 'enforce')
     def test_show(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'show')
         config_id = 'a45559cd-8736-4375-bc39-d6a7bb62ade2'
diff --git a/heat/tests/api/openstack_v1/test_stacks.py b/heat/tests/api/openstack_v1/test_stacks.py
index 48d1ebd61..6c1a46467 100644
--- a/heat/tests/api/openstack_v1/test_stacks.py
+++ b/heat/tests/api/openstack_v1/test_stacks.py
@@ -338,6 +338,20 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
         self.assertNotIn('balrog', engine_args)
 
     @mock.patch.object(rpc_client.EngineClient, 'call')
+    def test_index_limit_negative(self, mock_call, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'index', True)
+        params = {'limit': -1}
+        req = self._get('/stacks', params=params)
+
+        ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                               self.controller.index, req,
+                               tenant_id=self.tenant)
+        self.assertEqual("Value '-1' is invalid for 'limit' which only "
+                         "accepts non-negative integer.",
+                         str(ex))
+        self.assertFalse(mock_call.called)
+
+    @mock.patch.object(rpc_client.EngineClient, 'call')
     def test_index_limit_not_int(self, mock_call, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'index', True)
         params = {'limit': 'not-an-int'}
@@ -1936,6 +1950,31 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
             version='1.36'
         )
 
+    def test_update_timeout_negative(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'update', True)
+        identity = identifier.HeatIdentifier(self.tenant, 'wibble', '6')
+        template = {u'Foo': u'bar'}
+        parameters = {u'InstanceType': u'm1.xlarge'}
+        body = {'template': template,
+                'parameters': parameters,
+                'files': {},
+                'timeout_mins': -1}
+
+        req = self._put('/stacks/%(stack_name)s/%(stack_id)s' % identity,
+                        json.dumps(body))
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                               self.controller.update, req,
+                               tenant_id=identity.tenant,
+                               stack_name=identity.stack_name,
+                               stack_id=identity.stack_id,
+                               body=body)
+        self.assertEqual("Value '-1' is invalid for 'timeout_mins' which only "
+                         "accepts non-negative integer.",
+                         str(ex))
+        self.assertFalse(mock_call.called)
+
     def test_update_timeout_not_int(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'update', True)
         identity = identifier.HeatIdentifier(self.tenant, 'wibble', '6')
@@ -2149,6 +2188,31 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
             version='1.36'
         )
 
+    def test_update_with_patch_timeout_negative(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'update_patch', True)
+        identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6')
+        template = {u'Foo': u'bar'}
+        parameters = {u'InstanceType': u'm1.xlarge'}
+        body = {'template': template,
+                'parameters': parameters,
+                'files': {},
+                'timeout_mins': -1}
+
+        req = self._patch('/stacks/%(stack_name)s/%(stack_id)s' % identity,
+                          json.dumps(body))
+
+        mock_call = self.patchobject(rpc_client.EngineClient, 'call')
+        ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                               self.controller.update_patch, req,
+                               tenant_id=identity.tenant,
+                               stack_name=identity.stack_name,
+                               stack_id=identity.stack_id,
+                               body=body)
+        self.assertEqual("Value '-1' is invalid for 'timeout_mins' which only "
+                         "accepts non-negative integer.",
+                         str(ex))
+        self.assertFalse(mock_call.called)
+
     def test_update_with_patch_timeout_not_int(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'update_patch', True)
         identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6')