From ef67b861ddf7664b548f5fb26062884225b4ff60 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Wed, 25 May 2022 11:06:25 +0900 Subject: [PATCH] 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 --- heat/api/openstack/v1/software_configs.py | 12 ++++ .../api/openstack_v1/test_software_configs.py | 35 ++++++++++ heat/tests/api/openstack_v1/test_stacks.py | 64 +++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/heat/api/openstack/v1/software_configs.py b/heat/api/openstack/v1/software_configs.py index 41f9486e41..c27da4fe79 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 c1516eda07..110bfaef59 100644 --- a/heat/tests/api/openstack_v1/test_software_configs.py +++ b/heat/tests/api/openstack_v1/test_software_configs.py @@ -46,6 +46,41 @@ class SoftwareConfigControllerTest(tools.ControllerTest, common.HeatTestCase): self.assertEqual( {'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') diff --git a/heat/tests/api/openstack_v1/test_stacks.py b/heat/tests/api/openstack_v1/test_stacks.py index 48d1ebd619..6c1a464678 100644 --- a/heat/tests/api/openstack_v1/test_stacks.py +++ b/heat/tests/api/openstack_v1/test_stacks.py @@ -337,6 +337,20 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase): self.assertIn('filters', engine_args) 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) @@ -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')