From 8010c8faf9f030d2c0264189f9e6c70e10a093f2 Mon Sep 17 00:00:00 2001 From: Ken'ichi Ohmichi Date: Mon, 24 Feb 2014 16:28:41 +0900 Subject: [PATCH] Fix the validation of flavor_extraspecs v2 API "create flavor_extraspecs" v2 API does not validate the data type of a request body. If invalid parameter is passed, an internal error happens. If many invalid requests come, a log file would be occupied with traceback. In addition, it does not validate the lengths of both key and value of extra_specs. extra_specs are stored into the instance_type_extra_specs table, and the key and value are defined as String(255). This patch fixes the validation code from the viewpoint of data type and key/value length. Closes-Bug: #1264220 Change-Id: I195bd5d45a896e9b26dd81dab1e49c9f939b4805 --- .../compute/contrib/flavorextraspecs.py | 22 +++++-- .../contrib/test_flavors_extra_specs.py | 61 ++++++++++++++----- 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/nova/api/openstack/compute/contrib/flavorextraspecs.py b/nova/api/openstack/compute/contrib/flavorextraspecs.py index ca35f0fc90d8..1627e90e295e 100644 --- a/nova/api/openstack/compute/contrib/flavorextraspecs.py +++ b/nova/api/openstack/compute/contrib/flavorextraspecs.py @@ -24,6 +24,7 @@ from nova.compute import flavors from nova import db from nova import exception from nova.openstack.common.gettextutils import _ +from nova import utils authorize = extensions.extension_authorizer('compute', 'flavorextraspecs') @@ -55,12 +56,25 @@ class FlavorExtraSpecsController(object): expl = _('No Request Body') raise exc.HTTPBadRequest(explanation=expl) - def _check_key_names(self, keys): + def _check_extra_specs(self, specs): + if type(specs) is not dict: + msg = _('Bad extra_specs provided') + raise exc.HTTPBadRequest(explanation=msg) + try: - flavors.validate_extra_spec_keys(keys) + flavors.validate_extra_spec_keys(specs.keys()) except exception.InvalidInput as error: raise exc.HTTPBadRequest(explanation=error.format_message()) + for key, value in specs.iteritems(): + try: + utils.check_string_length(key, 'extra_specs key', + min_length=1, max_length=255) + utils.check_string_length(value, 'extra_specs value', + max_length=255) + except exception.InvalidInput as error: + raise exc.HTTPBadRequest(explanation=error.format_message()) + @wsgi.serializers(xml=ExtraSpecsTemplate) def index(self, req, flavor_id): """Returns the list of extra specs for a given flavor.""" @@ -74,7 +88,7 @@ class FlavorExtraSpecsController(object): authorize(context, action='create') self._check_body(body) specs = body.get('extra_specs') - self._check_key_names(specs.keys()) + self._check_extra_specs(specs) try: db.flavor_extra_specs_update_or_create(context, flavor_id, @@ -87,7 +101,7 @@ class FlavorExtraSpecsController(object): def update(self, req, flavor_id, id, body): context = req.environ['nova.context'] authorize(context, action='update') - self._check_body(body) + self._check_extra_specs(body) if id not in body: expl = _('Request body and URI mismatch') raise exc.HTTPBadRequest(explanation=expl) diff --git a/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py b/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py index 6336a3ce6b91..16b9c5dd7192 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavors_extra_specs.py @@ -142,7 +142,7 @@ class FlavorsExtraSpecsTest(test.TestCase): self.assertRaises(exception.NotAuthorized, self.controller.create, req, 1, body) - def test_create_empty_body(self): + def _test_create_bad_request(self, body): self.stubs.Set(nova.db, 'flavor_extra_specs_update_or_create', return_create_flavor_extra_specs) @@ -150,7 +150,27 @@ class FlavorsExtraSpecsTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/fake/flavors/1/os-extra_specs', use_admin_context=True) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, - req, 1, '') + req, 1, body) + + def test_create_empty_body(self): + self._test_create_bad_request('') + + def test_create_non_dict_extra_specs(self): + self._test_create_bad_request({"extra_specs": "non_dict"}) + + def test_create_non_string_value(self): + self._test_create_bad_request({"extra_specs": {"key1": None}}) + + def test_create_zero_length_key(self): + self._test_create_bad_request({"extra_specs": {"": "value1"}}) + + def test_create_long_key(self): + key = "a" * 256 + self._test_create_bad_request({"extra_specs": {key: "value1"}}) + + def test_create_long_value(self): + value = "a" * 256 + self._test_create_bad_request({"extra_specs": {"key1": value}}) @mock.patch('nova.db.flavor_extra_specs_update_or_create') def test_create_invalid_specs_key(self, mock_flavor_extra_specs): @@ -199,27 +219,40 @@ class FlavorsExtraSpecsTest(test.TestCase): self.assertRaises(exception.NotAuthorized, self.controller.update, req, 1, 'key1', body) - def test_update_item_empty_body(self): + def _test_update_item_bad_request(self, body): self.stubs.Set(nova.db, 'flavor_extra_specs_update_or_create', return_create_flavor_extra_specs) - req = fakes.HTTPRequest.blank('/v2/fake/flavors/1/os-extra_specs' + - '/key1', use_admin_context=True) - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - req, 1, 'key1', '') - - def test_update_item_too_many_keys(self): - self.stubs.Set(nova.db, - 'flavor_extra_specs_update_or_create', - return_create_flavor_extra_specs) - body = {"key1": "value1", "key2": "value2"} - req = fakes.HTTPRequest.blank('/v2/fake/flavors/1/os-extra_specs' + '/key1', use_admin_context=True) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, 1, 'key1', body) + def test_update_item_empty_body(self): + self._test_update_item_bad_request('') + + def test_update_item_too_many_keys(self): + body = {"key1": "value1", "key2": "value2"} + self._test_update_item_bad_request(body) + + def test_update_item_non_dict_extra_specs(self): + self._test_update_item_bad_request("non_dict") + + def test_update_item_non_string_value(self): + self._test_update_item_bad_request({"key1": None}) + + def test_update_item_zero_length_key(self): + self._test_update_item_bad_request({"": "value1"}) + + def test_update_item_long_key(self): + key = "a" * 256 + self._test_update_item_bad_request({key: "value1"}) + + def test_update_item_long_value(self): + value = "a" * 256 + self._test_update_item_bad_request({"key1": value}) + def test_update_item_body_uri_mismatch(self): self.stubs.Set(nova.db, 'flavor_extra_specs_update_or_create',