]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Validate string, integer limit for input parameter
authorPranaliDeore <pranali11.deore@nttdata.com>
Wed, 17 Jun 2015 11:49:24 +0000 (04:49 -0700)
committerPranali Deore <pranali11.deore@nttdata.com>
Mon, 10 Aug 2015 08:51:19 +0000 (08:51 +0000)
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

20 files changed:
cinder/api/contrib/cgsnapshots.py
cinder/api/contrib/consistencygroups.py
cinder/api/contrib/qos_specs_manage.py
cinder/api/contrib/quota_classes.py
cinder/api/contrib/quotas.py
cinder/api/contrib/types_extra_specs.py
cinder/api/contrib/volume_manage.py
cinder/api/contrib/volume_transfer.py
cinder/api/contrib/volume_type_encryption.py
cinder/api/openstack/wsgi.py
cinder/tests/unit/api/contrib/test_cgsnapshots.py
cinder/tests/unit/api/contrib/test_consistencygroups.py
cinder/tests/unit/api/contrib/test_qos_specs_manage.py
cinder/tests/unit/api/contrib/test_quotas.py
cinder/tests/unit/api/contrib/test_quotas_classes.py
cinder/tests/unit/api/contrib/test_types_extra_specs.py
cinder/tests/unit/api/contrib/test_volume_manage.py
cinder/tests/unit/api/contrib/test_volume_transfer.py
cinder/tests/unit/api/contrib/test_volume_type_encryption.py
cinder/tests/unit/api/openstack/test_wsgi.py

index a316993d385ac38b76d246e3a24fe660b338be3a..16ebf991790b84bcd226158740091329504537b3 100644 (file)
@@ -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']
index 284e6234c10c5b7986e73b19c80b8b4e17707d36..934b53f439f4588c021360819324d30eedcb9a92 100644 (file)
@@ -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)
index 3ba9e15d9709edaa9ab27615b13a61c23d939b3c..fe632f6843dc65897bb61bfa4ba670d01cc4dd1a 100644 (file)
@@ -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)
index ef74413d25abfa46226ac8fdeab3a38413f5624a..dbb081e526ce0fa0264c57971032661eadc7f24a 100644 (file)
@@ -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)
index 7ad7f48709e444b886046b99ae01618ab79f1f6b..6d46de94497151b90d1f0a9f52f140a5673a59a8 100644 (file)
@@ -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
index a97ea0dd2037ff98f0af685eaef8010eab269e57..4a3b19b5a7321e91e5c0c0b0d203f30a582d1264 100644 (file)
@@ -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)
index bfdb82ef730c656c3dfcb6e120de2a5912aa745c..3c4167b6e3915bad01c5aa05a210d331f7ec48f6 100644 (file)
@@ -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.
index bab74f6787f1d1a8e2b4915952f2a8f3628af231..b1358ebd8dbdac70e85cfed9e176761dc97fc07e 100644 (file)
@@ -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,
index 857a574f070306a91f6ad33c6a3be8c7bf0f82ee..c0aeb72bfbd03528f2d9ed778e11fced10b50eb8 100644 (file)
@@ -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
index cad8c908ea30a61f7dc5209cf73651fa7b2cc583..0c9b59b6340d636b9923945dd86dbddc159837b6 100644 (file)
@@ -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."""
index 5a2fa35ebed98495683938367fd527efe229445d..49903cfa048051512eb84bc2fd1091bc6197a5cf 100644 (file)
@@ -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)
 
index 4abba1a88befdf979dc663bc0c720a5647c9396d..56758df7d8dd8e6dc99699ebafa0661f6e588303 100644 (file)
@@ -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'])
index c3e94cda5c3080939547bb3b5ee8827977a7c14b..cfbdbc9c0d5c163193783e4b13b859e4c52b2e07 100644 (file)
@@ -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)
index 318386676ab27fbd33d5abc2ced059522b6cc231..e9544e4f4a66c2532e4db8f8ad8f771f483e3592 100644 (file)
@@ -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,
index cb39b9585243e0361ba7a45db7a065a955063a78..17bdffb440c55353ba69e67027d0ecc1e7efb15a 100644 (file)
@@ -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'}}
index ffad5bfc4933d4a18ed541c51116aa1f39676485..e62fa5572118f5f9c8bc4a9dec4d7c8afe0e459b 100644 (file)
@@ -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'])
 
index d33a931bdefc2732f6c80ec090630bc993330084..bf4532d498e4b2aa7f2cd5592c934c150bf7f89e 100644 (file)
@@ -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):
index a026a7e7a5ba3b5e0c8b0dccc92ab266d31208ab..1ff9461ac5be3c48e88ebba565e232eca99303ee 100644 (file)
@@ -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'
index faebb3e1d896f733bf70cd99471e3ecd909e0c70..05634df3e8587ebcbd95dbf9345a4b3c79ce2227 100644 (file)
 #    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):
index b9dbf8d77aa4b08424d16d68a984fa76df6d80ab..d1ff6dc8c6f7bb8e6cb08915b7fbb7da729d4bbc 100644 (file)
@@ -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))