From cf67960607844ef3426cae4d6e2ab96f16187b99 Mon Sep 17 00:00:00 2001 From: PranaliDeore Date: Wed, 17 Jun 2015 04:49:24 -0700 Subject: [PATCH] Validate string, integer limit for input parameter 1. Below apis will return 500 error code on passing name or description parameters with more than 255 characters: a. consisgroup-create b. consisgroup-update c. cgsnapshot-create d. quota-class-update e. quota-update f. qos-create g. volume-manage h. volume-transfer 2. Below apis will return 500 error code on passing 'hard_limit' value greater than mysql INT type: a. quota-class-update b. quota-update c. encryption-type-create 3. Below apis accept name as string with whitespaces: a. consisgroup-create b. cgsnapshot-create c. qos-create d. volume-transfer 4. Type-key api will return 500 error code on passing key or value with more than 255 characters. Added new method 1. validate_name_and_description() in cinder.api.openstack.wsgi.Controller to validate length of name and description and returned 400 if it exceeds the limit and removing leading or trailing whitespaces and string containing only whitespaces. 2. validate_string_length() in cinder.api.openstack.wsgi.Controller to validate length of string and returned 400 if it exceeds the limit. 3. validate_integer() method in cinder.utils to validate integer limit and returned 400 if limit exceeds. APIImpact 1. For all above apis 400 response will be returned. 2. Earlier it was possible to pass only whitespaces or leading-trailing spaces to 'name' parameters and 'key' while updating key-value in type-key api. Now it will raise 400 error if only whitespaces are passed and will remove leading-trailing spaces if present in other cases. Closes-Bug: 1466351 Closes-Bug: 1463379 Closes-Bug: 1465967 Change-Id: I0c0029d61ba2b293b579d1afffec0bdf062b22a8 --- cinder/api/contrib/cgsnapshots.py | 1 + cinder/api/contrib/consistencygroups.py | 3 ++ cinder/api/contrib/qos_specs_manage.py | 6 ++- cinder/api/contrib/quota_classes.py | 16 +++---- cinder/api/contrib/quotas.py | 28 +++-------- cinder/api/contrib/types_extra_specs.py | 16 +++++++ cinder/api/contrib/volume_manage.py | 1 + cinder/api/contrib/volume_transfer.py | 5 ++ cinder/api/contrib/volume_type_encryption.py | 14 ++---- cinder/api/openstack/wsgi.py | 48 +++++++++++++++++++ .../unit/api/contrib/test_cgsnapshots.py | 5 +- .../api/contrib/test_consistencygroups.py | 22 ++++++--- .../unit/api/contrib/test_qos_specs_manage.py | 5 +- cinder/tests/unit/api/contrib/test_quotas.py | 17 +++++++ .../unit/api/contrib/test_quotas_classes.py | 14 ++++++ .../api/contrib/test_types_extra_specs.py | 21 ++++++-- .../unit/api/contrib/test_volume_manage.py | 15 ++++-- .../unit/api/contrib/test_volume_transfer.py | 20 +++++--- .../contrib/test_volume_type_encryption.py | 17 +++++-- cinder/tests/unit/api/openstack/test_wsgi.py | 34 ++++++++++++- 20 files changed, 237 insertions(+), 71 deletions(-) diff --git a/cinder/api/contrib/cgsnapshots.py b/cinder/api/contrib/cgsnapshots.py index a316993d3..16ebf9917 100644 --- a/cinder/api/contrib/cgsnapshots.py +++ b/cinder/api/contrib/cgsnapshots.py @@ -159,6 +159,7 @@ class CgsnapshotsController(wsgi.Controller): context = req.environ['cinder.context'] cgsnapshot = body['cgsnapshot'] + self.validate_name_and_description(cgsnapshot) try: group_id = cgsnapshot['consistencygroup_id'] diff --git a/cinder/api/contrib/consistencygroups.py b/cinder/api/contrib/consistencygroups.py index 284e6234c..934b53f43 100644 --- a/cinder/api/contrib/consistencygroups.py +++ b/cinder/api/contrib/consistencygroups.py @@ -216,6 +216,7 @@ class ConsistencyGroupsController(wsgi.Controller): context = req.environ['cinder.context'] consistencygroup = body['consistencygroup'] + self.validate_name_and_description(consistencygroup) name = consistencygroup.get('name', None) description = consistencygroup.get('description', None) volume_types = consistencygroup.get('volume_types', None) @@ -260,6 +261,7 @@ class ConsistencyGroupsController(wsgi.Controller): context = req.environ['cinder.context'] consistencygroup = body['consistencygroup-from-src'] + self.validate_name_and_description(consistencygroup) name = consistencygroup.get('name', None) description = consistencygroup.get('description', None) cgsnapshot_id = consistencygroup.get('cgsnapshot_id', None) @@ -329,6 +331,7 @@ class ConsistencyGroupsController(wsgi.Controller): context = req.environ['cinder.context'] consistencygroup = body.get('consistencygroup', None) + self.validate_name_and_description(consistencygroup) name = consistencygroup.get('name', None) description = consistencygroup.get('description', None) add_volumes = consistencygroup.get('add_volumes', None) diff --git a/cinder/api/contrib/qos_specs_manage.py b/cinder/api/contrib/qos_specs_manage.py index 3ba9e15d9..fe632f684 100644 --- a/cinder/api/contrib/qos_specs_manage.py +++ b/cinder/api/contrib/qos_specs_manage.py @@ -127,10 +127,14 @@ class QoSSpecsController(wsgi.Controller): specs = body['qos_specs'] name = specs.get('name', None) - if name is None or name == "": + if name is None: msg = _("Please specify a name for QoS specs.") raise webob.exc.HTTPBadRequest(explanation=msg) + self.validate_string_length(name, 'name', min_length=1, + max_length=255, remove_whitespaces=True) + name = name.strip() + try: qos_specs.create(context, name, specs) spec = qos_specs.get_qos_specs_by_name(context, name) diff --git a/cinder/api/contrib/quota_classes.py b/cinder/api/contrib/quota_classes.py index ef74413d2..dbb081e52 100644 --- a/cinder/api/contrib/quota_classes.py +++ b/cinder/api/contrib/quota_classes.py @@ -68,24 +68,20 @@ class QuotaClassSetsController(wsgi.Controller): def update(self, req, id, body): context = req.environ['cinder.context'] authorize(context) + self.validate_string_length(id, 'quota_class_name', + min_length=1, max_length=255) + quota_class = id if not self.is_valid_body(body, 'quota_class_set'): msg = (_("Missing required element quota_class_set" " in request body.")) raise webob.exc.HTTPBadRequest(explanation=msg) - for key in body['quota_class_set'].keys(): + for key, value in body['quota_class_set'].items(): if key in QUOTAS: try: - value = int(body['quota_class_set'][key]) - except ValueError: - msg = _("Quota class limit must be specified as an" - " integer value.") - raise webob.exc.HTTPBadRequest(explanation=msg) - if value < -1: - msg = _("Quota class limit must be -1 or greater.") - raise webob.exc.HTTPBadRequest(explanation=msg) - try: + value = self.validate_integer(value, key, min_value=-1, + max_value=db.MAX_INT) db.quota_class_update(context, quota_class, key, value) except exception.QuotaClassNotFound: db.quota_class_create(context, quota_class, key, value) diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py index 7ad7f4870..6d46de944 100644 --- a/cinder/api/contrib/quotas.py +++ b/cinder/api/contrib/quotas.py @@ -56,26 +56,6 @@ class QuotaSetsController(wsgi.Controller): return dict(quota_set=quota_set) - def _validate_quota_limit(self, limit): - try: - limit = int(limit) - except (ValueError, TypeError): - msg = _("Quota limit must be specified as an integer value.") - raise webob.exc.HTTPBadRequest(explanation=msg) - - # NOTE: -1 is a flag value for unlimited, maximum value is limited - # by SQL standard integer type `INT` which is `0x7FFFFFFF`, it's a - # general value for SQL, using a hardcoded value here is not a - # `nice` way, but it seems like the only way for now: - # http://dev.mysql.com/doc/refman/5.0/en/integer-types.html - # http://www.postgresql.org/docs/9.1/static/datatype-numeric.html - if limit < -1 or limit > db.MAX_INT: - msg = _("Quota limit must be in the range of -1 " - "to %s.") % db.MAX_INT - raise webob.exc.HTTPBadRequest(explanation=msg) - - return limit - def _get_quotas(self, context, id, usages=False, parent_project_id=None): values = QUOTAS.get_project_quotas(context, id, usages=usages, parent_project_id=parent_project_id) @@ -107,6 +87,9 @@ class QuotaSetsController(wsgi.Controller): def update(self, req, id, body): context = req.environ['cinder.context'] authorize_update(context) + self.validate_string_length(id, 'quota_set_name', + min_length=1, max_length=255) + project_id = id self.assert_valid_body(body, 'quota_set') @@ -131,8 +114,9 @@ class QuotaSetsController(wsgi.Controller): if key in NON_QUOTA_KEYS: continue - value = self._validate_quota_limit(body['quota_set'][key]) - valid_quotas[key] = value + valid_quotas[key] = self.validate_integer( + body['quota_set'][key], key, min_value=-1, + max_value=db.MAX_INT) # NOTE(ankit): Pass #3 - At this point we know that all the keys and # values are valid and we can iterate and update them all in one shot diff --git a/cinder/api/contrib/types_extra_specs.py b/cinder/api/contrib/types_extra_specs.py index a97ea0dd2..4a3b19b5a 100644 --- a/cinder/api/contrib/types_extra_specs.py +++ b/cinder/api/contrib/types_extra_specs.py @@ -74,6 +74,17 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): self._check_type(context, type_id) return self._get_extra_specs(context, type_id) + def _validate_extra_specs(self, specs): + """Validating key and value of extra specs.""" + for key, value in specs.items(): + if key is not None: + self.validate_string_length(key, 'Key "%s"' % key, + min_length=1, max_length=255) + + if value is not None: + self.validate_string_length(value, 'Value for key "%s"' % key, + min_length=0, max_length=255) + @wsgi.serializers(xml=VolumeTypeExtraSpecsTemplate) def create(self, req, type_id, body=None): context = req.environ['cinder.context'] @@ -84,6 +95,8 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): self._check_type(context, type_id) specs = body['extra_specs'] self._check_key_names(specs.keys()) + self._validate_extra_specs(specs) + db.volume_type_extra_specs_update_or_create(context, type_id, specs) @@ -107,6 +120,9 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): if len(body) > 1: expl = _('Request body contains too many items') raise webob.exc.HTTPBadRequest(explanation=expl) + self._check_key_names(body.keys()) + self._validate_extra_specs(body) + db.volume_type_extra_specs_update_or_create(context, type_id, body) diff --git a/cinder/api/contrib/volume_manage.py b/cinder/api/contrib/volume_manage.py index bfdb82ef7..3c4167b6e 100644 --- a/cinder/api/contrib/volume_manage.py +++ b/cinder/api/contrib/volume_manage.py @@ -99,6 +99,7 @@ class VolumeManageController(wsgi.Controller): self.assert_valid_body(body, 'volume') volume = body['volume'] + self.validate_name_and_description(volume) # Check that the required keys are present, return an error if they # are not. diff --git a/cinder/api/contrib/volume_transfer.py b/cinder/api/contrib/volume_transfer.py index bab74f678..b1358ebd8 100644 --- a/cinder/api/contrib/volume_transfer.py +++ b/cinder/api/contrib/volume_transfer.py @@ -161,6 +161,11 @@ class VolumeTransferController(wsgi.Controller): raise exc.HTTPBadRequest(explanation=msg) name = transfer.get('name', None) + if name is not None: + self.validate_string_length(name, 'Transfer name', + min_length=1, max_length=255, + remove_whitespaces=True) + name = name.strip() LOG.info(_LI("Creating transfer of volume %s"), volume_id, diff --git a/cinder/api/contrib/volume_type_encryption.py b/cinder/api/contrib/volume_type_encryption.py index 857a574f0..c0aeb72bf 100644 --- a/cinder/api/contrib/volume_type_encryption.py +++ b/cinder/api/contrib/volume_type_encryption.py @@ -57,16 +57,10 @@ class VolumeTypeEncryptionController(wsgi.Controller): raise webob.exc.HTTPNotFound(explanation=ex.msg) def _check_encryption_input(self, encryption, create=True): - if 'key_size' in encryption.keys(): - key_size = encryption['key_size'] - if key_size is not None: - if isinstance(key_size, (int, long)): - if key_size < 0: - msg = _('key_size must be non-negative') - raise exception.InvalidInput(reason=msg) - else: - msg = _('key_size must be an integer') - raise exception.InvalidInput(reason=msg) + if encryption.get('key_size') is not None: + encryption['key_size'] = self.validate_integer( + encryption['key_size'], 'key_size', + min_value=0, max_value=db.MAX_INT) if create: msg = None diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py index cad8c908e..0c9b59b63 100644 --- a/cinder/api/openstack/wsgi.py +++ b/cinder/api/openstack/wsgi.py @@ -1240,6 +1240,54 @@ class Controller(object): except exception.InvalidInput as error: raise webob.exc.HTTPBadRequest(explanation=error.msg) + @staticmethod + def validate_string_length(value, entity_name, min_length=0, + max_length=None, remove_whitespaces=False): + """Check the length of specified string. + + :param value: the value of the string + :param entity_name: the name of the string + :param min_length: the min_length of the string + :param max_length: the max_length of the string + :param remove_whitespaces: True if trimming whitespaces is needed + else False + """ + if isinstance(value, six.string_types) and remove_whitespaces: + value = value.strip() + try: + utils.check_string_length(value, entity_name, + min_length=min_length, + max_length=max_length) + except exception.InvalidInput as error: + raise webob.exc.HTTPBadRequest(explanation=error.msg) + + @staticmethod + def validate_integer(value, name, min_value=None, max_value=None): + """Make sure that value is a valid integer, potentially within range. + + :param value: the value of the integer + :param name: the name of the integer + :param min_length: the min_length of the integer + :param max_length: the max_length of the integer + :returns: integer + """ + try: + value = int(value) + except (TypeError, ValueError, UnicodeEncodeError): + raise webob.exc.HTTPBadRequest(explanation=( + _('%s must be an integer.') % name)) + + if min_value is not None and value < min_value: + raise webob.exc.HTTPBadRequest( + explanation=(_('%(value_name)s must be >= %(min_value)d') % + {'value_name': name, 'min_value': min_value})) + if max_value is not None and value > max_value: + raise webob.exc.HTTPBadRequest( + explanation=(_('%(value_name)s must be <= %(max_value)d') % + {'value_name': name, 'max_value': max_value})) + + return value + class Fault(webob.exc.HTTPException): """Wrap webob.exc.HTTPException to provide API friendly response.""" diff --git a/cinder/tests/unit/api/contrib/test_cgsnapshots.py b/cinder/tests/unit/api/contrib/test_cgsnapshots.py index 5a2fa35eb..49903cfa0 100644 --- a/cinder/tests/unit/api/contrib/test_cgsnapshots.py +++ b/cinder/tests/unit/api/contrib/test_cgsnapshots.py @@ -331,7 +331,9 @@ class CgsnapshotsAPITestCase(test.TestCase): db.consistencygroup_destroy(context.get_admin_context(), consistencygroup_id) - def test_create_cgsnapshot_json(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_create_cgsnapshot_json(self, mock_validate): cgsnapshot_id = "1" consistencygroup_id = utils.create_consistencygroup(self.context)['id'] @@ -353,6 +355,7 @@ class CgsnapshotsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['cgsnapshot']) + self.assertTrue(mock_validate.called) db.cgsnapshot_destroy(context.get_admin_context(), cgsnapshot_id) diff --git a/cinder/tests/unit/api/contrib/test_consistencygroups.py b/cinder/tests/unit/api/contrib/test_consistencygroups.py index 4abba1a88..56758df7d 100644 --- a/cinder/tests/unit/api/contrib/test_consistencygroups.py +++ b/cinder/tests/unit/api/contrib/test_consistencygroups.py @@ -304,7 +304,9 @@ class ConsistencyGroupsAPITestCase(test.TestCase): db.consistencygroup_destroy(context.get_admin_context(), consistencygroup_id1) - def test_create_consistencygroup_json(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_create_consistencygroup_json(self, mock_validate): group_id = "1" # Create volume type @@ -325,6 +327,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['consistencygroup']) + self.assertTrue(mock_validate.called) db.consistencygroup_destroy(context.get_admin_context(), group_id) @@ -497,7 +500,9 @@ class ConsistencyGroupsAPITestCase(test.TestCase): 'consistency group %s.') % name) self.assertEqual(msg, res_dict['badRequest']['message']) - def test_update_consistencygroup_success(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_update_consistencygroup_success(self, mock_validate): volume_type_id = '123456' ctxt = context.RequestContext('fake', 'fake') consistencygroup_id = self._create_consistencygroup(status='available', @@ -546,6 +551,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase): self.assertEqual('updating', self._get_consistencygroup_attrib(consistencygroup_id, 'status')) + self.assertTrue(mock_validate.called) db.consistencygroup_destroy(ctxt.elevated(), consistencygroup_id) @@ -616,8 +622,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase): self.assertEqual(400, res.status_int) self.assertEqual(400, res_dict['badRequest']['code']) - self.assertEqual('Name, description, add_volumes, and remove_volumes ' - 'can not be all empty in the request body.', + self.assertEqual('Name has a minimum character requirement of 1.', res_dict['badRequest']['message']) db.consistencygroup_destroy(ctxt.elevated(), consistencygroup_id) @@ -635,7 +640,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase): req.method = 'PUT' req.headers['Content-Type'] = 'application/json' add_volumes = add_volume_id - body = {"consistencygroup": {"name": "", + body = {"consistencygroup": {"name": "cg1", "description": "", "add_volumes": add_volumes, "remove_volumes": None, }} @@ -668,7 +673,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase): req.method = 'PUT' req.headers['Content-Type'] = 'application/json' add_volumes = add_volume_id - body = {"consistencygroup": {"name": "", + body = {"consistencygroup": {"name": "cg1", "description": "", "add_volumes": add_volumes, "remove_volumes": None, }} @@ -713,7 +718,9 @@ class ConsistencyGroupsAPITestCase(test.TestCase): db.consistencygroup_destroy(ctxt.elevated(), consistencygroup_id) - def test_create_consistencygroup_from_src_cgsnapshot(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_create_consistencygroup_from_src(self, mock_validate): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) ctxt = context.RequestContext('fake', 'fake', auth_token=True) @@ -745,6 +752,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['consistencygroup']) self.assertEqual(test_cg_name, res_dict['consistencygroup']['name']) + self.assertTrue(mock_validate.called) db.consistencygroup_destroy(ctxt.elevated(), res_dict['consistencygroup']['id']) diff --git a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py index c3e94cda5..cfbdbc9c0 100644 --- a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py +++ b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py @@ -290,7 +290,9 @@ class QoSSpecManageApiTest(test.TestCase): side_effect=return_qos_specs_create) @mock.patch('cinder.volume.qos_specs.get_qos_specs_by_name', side_effect=return_qos_specs_get_by_name) - def test_create(self, mock_qos_get_specs, mock_qos_spec_create): + @mock.patch('cinder.api.openstack.wsgi.Controller.validate_string_length') + def test_create(self, mock_validate, mock_qos_get_specs, + mock_qos_spec_create): body = {"qos_specs": {"name": "qos_specs_1", "key1": "value1"}} @@ -302,6 +304,7 @@ class QoSSpecManageApiTest(test.TestCase): self.assertEqual(1, notifier.get_notification_count()) self.assertEqual('qos_specs_1', res_dict['qos_specs']['name']) + self.assertTrue(mock_validate.called) @mock.patch('cinder.volume.qos_specs.create', side_effect=return_qos_specs_create) diff --git a/cinder/tests/unit/api/contrib/test_quotas.py b/cinder/tests/unit/api/contrib/test_quotas.py index 318386676..e9544e4f4 100644 --- a/cinder/tests/unit/api/contrib/test_quotas.py +++ b/cinder/tests/unit/api/contrib/test_quotas.py @@ -18,6 +18,9 @@ Tests for cinder.api.contrib.quotas.py """ + +import mock + from lxml import etree import webob.exc @@ -88,6 +91,20 @@ class QuotaSetsControllerTest(test.TestCase): result = self.controller.update(self.req, 'foo', body) self.assertDictMatch(result, body) + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_integer') + def test_update_limit(self, mock_validate_integer, mock_validate): + mock_validate_integer.return_value = 10 + + body = {'quota_set': {'volumes': 10}} + result = self.controller.update(self.req, 'foo', body) + + self.assertEqual(10, result['quota_set']['volumes']) + self.assertTrue(mock_validate.called) + self.assertTrue(mock_validate_integer.called) + def test_update_wrong_key(self): body = {'quota_set': {'bad': 'bad'}} self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, diff --git a/cinder/tests/unit/api/contrib/test_quotas_classes.py b/cinder/tests/unit/api/contrib/test_quotas_classes.py index cb39b9585..17bdffb44 100644 --- a/cinder/tests/unit/api/contrib/test_quotas_classes.py +++ b/cinder/tests/unit/api/contrib/test_quotas_classes.py @@ -17,6 +17,9 @@ Tests for cinder.api.contrib.quota_classes.py """ + +import mock + from lxml import etree import webob.exc @@ -106,6 +109,17 @@ class QuotaClassSetsControllerTest(test.TestCase): result = self.controller.update(self.req, 'foo', body) self.assertDictMatch(result, body) + @mock.patch('cinder.api.openstack.wsgi.Controller.validate_string_length') + @mock.patch('cinder.api.openstack.wsgi.Controller.validate_integer') + def test_update_limit(self, mock_validate_integer, mock_validate): + mock_validate_integer.return_value = 5 + volume_types.create(self.ctxt, 'fake_type') + body = make_body(volumes=5) + result = self.controller.update(self.req, 'foo', body) + self.assertEqual(5, result['quota_class_set']['volumes']) + self.assertTrue(mock_validate.called) + self.assertTrue(mock_validate_integer.called) + def test_update_wrong_key(self): volume_types.create(self.ctxt, 'fake_type') body = {'quota_class_set': {'bad': 'bad'}} diff --git a/cinder/tests/unit/api/contrib/test_types_extra_specs.py b/cinder/tests/unit/api/contrib/test_types_extra_specs.py index ffad5bfc4..e62fa5572 100644 --- a/cinder/tests/unit/api/contrib/test_types_extra_specs.py +++ b/cinder/tests/unit/api/contrib/test_types_extra_specs.py @@ -123,7 +123,9 @@ class VolumeTypesExtraSpecsTest(test.TestCase): self.assertRaises(webob.exc.HTTPNotFound, self.controller.delete, req, 1, 'key6') - def test_create(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') + def test_create(self, mock_validate): self.stubs.Set(cinder.db, 'volume_type_extra_specs_update_or_create', return_create_volume_type_extra_specs) @@ -133,12 +135,14 @@ class VolumeTypesExtraSpecsTest(test.TestCase): req = fakes.HTTPRequest.blank(self.api_path) res_dict = self.controller.create(req, 1, body) self.assertEqual(1, len(self.notifier.notifications)) - + self.assertTrue(mock_validate.called) self.assertEqual('value1', res_dict['extra_specs']['key1']) @mock.patch.object(cinder.db, 'volume_type_extra_specs_update_or_create') + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') def test_create_key_allowed_chars( - self, volume_type_extra_specs_update_or_create): + self, mock_validate, volume_type_extra_specs_update_or_create): mock_return_value = {"key1": "value1", "key2": "value2", "key3": "value3", @@ -154,12 +158,15 @@ class VolumeTypesExtraSpecsTest(test.TestCase): req = fakes.HTTPRequest.blank(self.api_path) res_dict = self.controller.create(req, 1, body) self.assertEqual(1, len(self.notifier.notifications)) + self.assertTrue(mock_validate.called) self.assertEqual('value1', res_dict['extra_specs']['other_alphanum.-_:']) @mock.patch.object(cinder.db, 'volume_type_extra_specs_update_or_create') + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') def test_create_too_many_keys_allowed_chars( - self, volume_type_extra_specs_update_or_create): + self, mock_validate, volume_type_extra_specs_update_or_create): mock_return_value = {"key1": "value1", "key2": "value2", "key3": "value3", @@ -177,6 +184,7 @@ class VolumeTypesExtraSpecsTest(test.TestCase): req = fakes.HTTPRequest.blank(self.api_path) res_dict = self.controller.create(req, 1, body) self.assertEqual(1, len(self.notifier.notifications)) + self.assertTrue(mock_validate.called) self.assertEqual('value1', res_dict['extra_specs']['other_alphanum.-_:']) self.assertEqual('value2', @@ -184,7 +192,9 @@ class VolumeTypesExtraSpecsTest(test.TestCase): self.assertEqual('value3', res_dict['extra_specs']['other3_alphanum.-_:']) - def test_update_item(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') + def test_update_item(self, mock_validate): self.stubs.Set(cinder.db, 'volume_type_extra_specs_update_or_create', return_create_volume_type_extra_specs) @@ -194,6 +204,7 @@ class VolumeTypesExtraSpecsTest(test.TestCase): req = fakes.HTTPRequest.blank(self.api_path + '/key1') res_dict = self.controller.update(req, 1, 'key1', body) self.assertEqual(1, len(self.notifier.notifications)) + self.assertTrue(mock_validate.called) self.assertEqual('value1', res_dict['key1']) diff --git a/cinder/tests/unit/api/contrib/test_volume_manage.py b/cinder/tests/unit/api/contrib/test_volume_manage.py index d33a931bd..bf4532d49 100644 --- a/cinder/tests/unit/api/contrib/test_volume_manage.py +++ b/cinder/tests/unit/api/contrib/test_volume_manage.py @@ -135,7 +135,9 @@ class VolumeManageTest(test.TestCase): return res @mock.patch('cinder.volume.api.API.manage_existing', wraps=api_manage) - def test_manage_volume_ok(self, mock_api_manage): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_manage_volume_ok(self, mock_validate, mock_api_manage): """Test successful manage volume execution. Tests for correct operation when valid arguments are passed in the @@ -153,6 +155,7 @@ class VolumeManageTest(test.TestCase): args = mock_api_manage.call_args[0] self.assertEqual(args[1], body['volume']['host']) self.assertEqual(args[2], body['volume']['ref']) + self.assertTrue(mock_validate.called) def test_manage_volume_missing_host(self): """Test correct failure when host is not specified.""" @@ -168,7 +171,9 @@ class VolumeManageTest(test.TestCase): pass @mock.patch('cinder.volume.api.API.manage_existing', api_manage) - def test_manage_volume_volume_type_by_uuid(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_manage_volume_volume_type_by_uuid(self, mock_validate): """Tests for correct operation when a volume type is specified by ID. We wrap cinder.volume.api.API.manage_existing so that managing is not @@ -180,10 +185,13 @@ class VolumeManageTest(test.TestCase): 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'}} res = self._get_resp(body) self.assertEqual(202, res.status_int, res) + self.assertTrue(mock_validate.called) pass @mock.patch('cinder.volume.api.API.manage_existing', api_manage) - def test_manage_volume_volume_type_by_name(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') + def test_manage_volume_volume_type_by_name(self, mock_validate): """Tests for correct operation when a volume type is specified by name. We wrap cinder.volume.api.API.manage_existing so that managing is not @@ -194,6 +202,7 @@ class VolumeManageTest(test.TestCase): 'volume_type': 'good_fakevt'}} res = self._get_resp(body) self.assertEqual(202, res.status_int, res) + self.assertTrue(mock_validate.called) pass def test_manage_volume_bad_volume_type_by_uuid(self): diff --git a/cinder/tests/unit/api/contrib/test_volume_transfer.py b/cinder/tests/unit/api/contrib/test_volume_transfer.py index a026a7e7a..1ff9461ac 100644 --- a/cinder/tests/unit/api/contrib/test_volume_transfer.py +++ b/cinder/tests/unit/api/contrib/test_volume_transfer.py @@ -18,6 +18,7 @@ Tests for volume transfer code. """ import json +import mock from xml.dom import minidom import webob @@ -252,9 +253,11 @@ class VolumeTransferAPITestCase(test.TestCase): db.transfer_destroy(context.get_admin_context(), transfer1['id']) db.volume_destroy(context.get_admin_context(), volume_id_1) - def test_create_transfer_json(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') + def test_create_transfer_json(self, mock_validate): volume_id = self._create_volume(status='available', size=5) - body = {"transfer": {"display_name": "transfer1", + body = {"transfer": {"name": "transfer1", "volume_id": volume_id}} req = webob.Request.blank('/v2/fake/os-volume-transfer') @@ -271,10 +274,13 @@ class VolumeTransferAPITestCase(test.TestCase): self.assertIn('created_at', res_dict['transfer']) self.assertIn('name', res_dict['transfer']) self.assertIn('volume_id', res_dict['transfer']) + self.assertTrue(mock_validate.called) db.volume_destroy(context.get_admin_context(), volume_id) - def test_create_transfer_xml(self): + @mock.patch( + 'cinder.api.openstack.wsgi.Controller.validate_string_length') + def test_create_transfer_xml(self, mock_validate): volume_size = 2 volume_id = self._create_volume(status='available', size=volume_size) @@ -294,6 +300,8 @@ class VolumeTransferAPITestCase(test.TestCase): self.assertTrue(transfer.item(0).hasAttribute('created_at')) self.assertEqual(transfer.item(0).getAttribute('name'), 'transfer-001') self.assertTrue(transfer.item(0).hasAttribute('volume_id')) + self.assertTrue(mock_validate.called) + db.volume_destroy(context.get_admin_context(), volume_id) def test_create_transfer_with_no_body(self): @@ -312,7 +320,7 @@ class VolumeTransferAPITestCase(test.TestCase): res_dict['badRequest']['message']) def test_create_transfer_with_body_KeyError(self): - body = {"transfer": {"display_name": "transfer1"}} + body = {"transfer": {"name": "transfer1"}} req = webob.Request.blank('/v2/fake/os-volume-transfer') req.method = 'POST' req.headers['Content-Type'] = 'application/json' @@ -326,7 +334,7 @@ class VolumeTransferAPITestCase(test.TestCase): res_dict['badRequest']['message']) def test_create_transfer_with_VolumeNotFound(self): - body = {"transfer": {"display_name": "transfer1", + body = {"transfer": {"name": "transfer1", "volume_id": 1234}} req = webob.Request.blank('/v2/fake/os-volume-transfer') @@ -343,7 +351,7 @@ class VolumeTransferAPITestCase(test.TestCase): def test_create_transfer_with_InvalidVolume(self): volume_id = self._create_volume(status='attached') - body = {"transfer": {"display_name": "transfer1", + body = {"transfer": {"name": "transfer1", "volume_id": volume_id}} req = webob.Request.blank('/v2/fake/os-volume-transfer') req.method = 'POST' diff --git a/cinder/tests/unit/api/contrib/test_volume_type_encryption.py b/cinder/tests/unit/api/contrib/test_volume_type_encryption.py index faebb3e1d..05634df3e 100644 --- a/cinder/tests/unit/api/contrib/test_volume_type_encryption.py +++ b/cinder/tests/unit/api/contrib/test_volume_type_encryption.py @@ -14,9 +14,11 @@ # under the License. import json +import mock import webob +from cinder.api.openstack import wsgi from cinder import context from cinder import db from cinder import test @@ -201,7 +203,11 @@ class VolumeTypeEncryptionTest(test.TestCase): db.volume_type_destroy(context.get_admin_context(), volume_type['id']) def test_create_json(self): - self._create('fake_cipher', 'front-end', 128, 'fake_encryptor') + with mock.patch.object(wsgi.Controller, + 'validate_integer') as mock_validate_integer: + mock_validate_integer.return_value = 128 + self._create('fake_cipher', 'front-end', 128, 'fake_encryptor') + self.assertTrue(mock_validate_integer.called) def test_create_xml(self): volume_type = self._default_volume_type @@ -337,7 +343,7 @@ class VolumeTypeEncryptionTest(test.TestCase): 'key_size': -128, 'provider': 'fake_provider', 'volume_type_id': 'volume_type'}} - msg = 'Invalid input received: key_size must be non-negative' + msg = 'key_size must be >= 0' self._encryption_create_bad_body(body=body, msg=msg) def test_create_none_key_size(self): @@ -498,7 +504,9 @@ class VolumeTypeEncryptionTest(test.TestCase): self.assertEqual(expected, json.loads(res.body)) db.volume_type_destroy(context.get_admin_context(), volume_type['id']) - def test_update_item(self): + @mock.patch('cinder.api.openstack.wsgi.Controller.validate_integer') + def test_update_item(self, mock_validate_integer): + mock_validate_integer.return_value = 512 volume_type = self._default_volume_type # Create Encryption Specs @@ -530,6 +538,7 @@ class VolumeTypeEncryptionTest(test.TestCase): # Confirm Encryption Specs self.assertEqual(512, res_dict['key_size']) self.assertEqual('fake_provider2', res_dict['provider']) + self.assertTrue(mock_validate_integer.called) db.volume_type_destroy(context.get_admin_context(), volume_type['id']) @@ -567,7 +576,7 @@ class VolumeTypeEncryptionTest(test.TestCase): def test_update_key_size_non_integer(self): update_body = {"encryption": {'key_size': 'abc'}} - msg = 'Invalid input received: key_size must be an integer' + msg = 'key_size must be an integer.' self._encryption_update_bad_body(update_body, msg) def test_update_item_invalid_body(self): diff --git a/cinder/tests/unit/api/openstack/test_wsgi.py b/cinder/tests/unit/api/openstack/test_wsgi.py index b9dbf8d77..d1ff6dc8c 100644 --- a/cinder/tests/unit/api/openstack/test_wsgi.py +++ b/cinder/tests/unit/api/openstack/test_wsgi.py @@ -984,6 +984,20 @@ class ValidBodyTest(test.TestCase): body = {'foo': 'bar'} self.assertFalse(self.controller.is_valid_body(body, 'foo')) + def test_validate_string_length_with_name_too_long(self): + name = 'a' * 256 + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.validate_string_length, + name, 'Name', min_length=1, max_length=255, + remove_whitespaces=False) + + def test_validate_string_length_with_name_contains_white_spaces( + self): + body = {'name': 'a' * 255 + " "} + self.controller.validate_string_length( + body['name'], 'name', min_length=1, max_length=255, + remove_whitespaces=True) + def test_validate_name_and_description_with_name_too_long(self): body = {'name': 'a' * 256} self.assertRaises(webob.exc.HTTPBadRequest, @@ -1019,8 +1033,26 @@ class ValidBodyTest(test.TestCase): self.controller.validate_name_and_description(body) self.assertEqual('', body['description']) - def test_validate_name_and_description_with_name_contains_whitespaces( + def test_validate_name_and_description_with_name_contains_white_spaces( self): body = {'name': 'a' * 255 + " "} self.controller.validate_name_and_description(body) self.assertEqual('a' * 255, body['name']) + + def test_validate_integer_greater_than_max_int_limit(self): + value = (2 ** 31) + 1 + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.validate_integer, + value, 'limit', min_value=-1, max_value=(2 ** 31)) + + def test_validate_integer_less_than_min_int_limit(self): + value = -12 + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.validate_integer, + value, 'limit', min_value=-1, max_value=(2 ** 31)) + + def test_validate_integer_invalid_limit(self): + value = "should_be_int" + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.validate_integer, + value, 'limit', min_value=-1, max_value=(2 ** 31)) -- 2.45.2