From 050ce0e5891ba816baaef74fcdcc91fd26935eac Mon Sep 17 00:00:00 2001 From: "Leandro I. Costantino" Date: Mon, 3 Feb 2014 09:24:16 -0500 Subject: [PATCH] Enforce FlavorExtraSpecs Key format. When creating a key name with characters like '/' that are not being encoded or cannot be handled as expected by routes when matching the url, then it's not possible to remove them anymore. The new format apply the restriction: "[\w\.\- :]+$" This change will fix the bug without API change. Change-Id: I64bc098c4bde33c40fa149191a751ea3eb19f932 Closes-Bug: #1256119 --- .../compute/contrib/flavorextraspecs.py | 9 +++++- .../compute/plugins/v3/flavors_extraspecs.py | 8 ++++++ nova/compute/flavors.py | 11 ++++++++ .../contrib/test_flavors_extra_specs.py | 25 +++++++++++++++++ .../plugins/v3/test_flavors_extra_specs.py | 28 +++++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/compute/contrib/flavorextraspecs.py b/nova/api/openstack/compute/contrib/flavorextraspecs.py index 63de65090a..bc1da30dbc 100644 --- a/nova/api/openstack/compute/contrib/flavorextraspecs.py +++ b/nova/api/openstack/compute/contrib/flavorextraspecs.py @@ -22,11 +22,11 @@ from webob import exc from nova.api.openstack import extensions from nova.api.openstack import wsgi from nova.api.openstack import xmlutil +from nova.compute import flavors from nova import db from nova import exception from nova.openstack.common.gettextutils import _ - authorize = extensions.extension_authorizer('compute', 'flavorextraspecs') @@ -57,6 +57,12 @@ class FlavorExtraSpecsController(object): expl = _('No Request Body') raise exc.HTTPBadRequest(explanation=expl) + def _check_key_names(self, keys): + try: + flavors.validate_extra_spec_keys(keys) + 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.""" @@ -70,6 +76,7 @@ class FlavorExtraSpecsController(object): authorize(context, action='create') self._check_body(body) specs = body.get('extra_specs') + self._check_key_names(specs.keys()) try: db.flavor_extra_specs_update_or_create(context, flavor_id, diff --git a/nova/api/openstack/compute/plugins/v3/flavors_extraspecs.py b/nova/api/openstack/compute/plugins/v3/flavors_extraspecs.py index 625e9962f9..7c22640ddf 100644 --- a/nova/api/openstack/compute/plugins/v3/flavors_extraspecs.py +++ b/nova/api/openstack/compute/plugins/v3/flavors_extraspecs.py @@ -19,6 +19,7 @@ import webob from nova.api.openstack import extensions from nova.api.openstack import wsgi +from nova.compute import flavors from nova import db from nova import exception from nova.openstack.common.db import exception as db_exc @@ -43,6 +44,12 @@ class FlavorExtraSpecsController(object): expl = _('No Request Body') raise webob.exc.HTTPBadRequest(explanation=expl) + def _check_key_names(self, keys): + try: + flavors.validate_extra_spec_keys(keys) + except exception.InvalidInput as error: + raise webob.exc.HTTPBadRequest(explanation=error.format_message()) + @extensions.expected_errors(()) def index(self, req, flavor_id): """Returns the list of extra specs for a given flavor.""" @@ -59,6 +66,7 @@ class FlavorExtraSpecsController(object): specs = body.get('extra_specs', {}) if not specs or type(specs) is not dict: raise webob.exc.HTTPBadRequest(_('No or bad extra_specs provided')) + self._check_key_names(specs.keys()) try: db.flavor_extra_specs_update_or_create(context, flavor_id, specs) diff --git a/nova/compute/flavors.py b/nova/compute/flavors.py index 10cd3673b6..1801fbf522 100644 --- a/nova/compute/flavors.py +++ b/nova/compute/flavors.py @@ -55,6 +55,9 @@ LOG = logging.getLogger(__name__) VALID_ID_REGEX = re.compile("^[\w\.\- ]*$") VALID_NAME_REGEX = re.compile("^[\w\.\- ]*$", re.UNICODE) +# Validate extra specs key names. +VALID_EXTRASPEC_NAME_REGEX = re.compile(r"[\w\.\- :]+$", re.UNICODE) + def _int_or_none(val): if val is not None: @@ -309,3 +312,11 @@ def delete_flavor_info(metadata, *prefixes): del metadata[to_key] pci_request.delete_flavor_pci_info(metadata, *prefixes) return metadata + + +def validate_extra_spec_keys(key_names_list): + for key_name in key_names_list: + if not VALID_EXTRASPEC_NAME_REGEX.match(key_name): + expl = _('Key Names can only contain alphanumeric characters, ' + 'periods, dashes, underscores, colons and spaces.') + raise exception.InvalidInput(message=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 8e38c4cb7f..e4c41c32a4 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 @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import webob from nova.api.openstack.compute.contrib import flavorextraspecs @@ -153,6 +154,30 @@ class FlavorsExtraSpecsTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, 1, '') + @mock.patch('nova.db.flavor_extra_specs_update_or_create') + def test_create_invalid_specs_key(self, mock_flavor_extra_specs): + invalid_keys = ("key1/", "", "$$akey$", "!akey", "") + mock_flavor_extra_specs.side_effects = return_create_flavor_extra_specs + + for key in invalid_keys: + body = {"extra_specs": {key: "value1"}} + 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, body) + + @mock.patch('nova.db.flavor_extra_specs_update_or_create') + def test_create_valid_specs_key(self, mock_flavor_extra_specs): + valid_keys = ("key1", "month.price", "I_am-a Key", "finance:g2") + mock_flavor_extra_specs.side_effects = return_create_flavor_extra_specs + + for key in valid_keys: + body = {"extra_specs": {key: "value1"}} + req = fakes.HTTPRequest.blank('/v2/fake/flavors/1/os-extra_specs', + use_admin_context=True) + res_dict = self.controller.create(req, 1, body) + self.assertEqual('value1', res_dict['extra_specs'][key]) + def test_update_item(self): self.stubs.Set(nova.db, 'flavor_extra_specs_update_or_create', diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_flavors_extra_specs.py b/nova/tests/api/openstack/compute/plugins/v3/test_flavors_extra_specs.py index dbfdd4eb81..d3c77f8c21 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_flavors_extra_specs.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_flavors_extra_specs.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import webob from nova.api.openstack.compute.plugins.v3 import flavors_extraspecs @@ -178,6 +179,33 @@ class FlavorsExtraSpecsTest(test.TestCase): self.assertRaises(webob.exc.HTTPConflict, self.controller.create, req, 1, body) + @mock.patch('nova.db.flavor_extra_specs_update_or_create') + def test_create_invalid_specs_key(self, mock_flavor_extra_specs): + invalid_keys = ("key1/", "", "$$akey$", "!akey", "") + mock_flavor_extra_specs.side_effects = return_create_flavor_extra_specs + + for key in invalid_keys: + body = {"extra_specs": {key: "value1"}} + + req = fakes.HTTPRequest.blank('/v3/flavors/1/extra-specs', + use_admin_context=True) + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, + req, 1, body) + + @mock.patch('nova.db.flavor_extra_specs_update_or_create') + def test_create_valid_specs_key(self, mock_flavor_extra_specs): + valid_keys = ("key1", "month.price", "I_am-a Key", "finance:g2") + mock_flavor_extra_specs.side_effects = return_create_flavor_extra_specs + + for key in valid_keys: + body = {"extra_specs": {key: "value1"}} + + req = fakes.HTTPRequest.blank('/v3/flavors/1/extra-specs', + use_admin_context=True) + res_dict = self.controller.create(req, 1, body) + self.assertEqual('value1', res_dict['extra_specs'][key]) + self.assertEqual(self.controller.create.wsgi_code, 201) + def test_update_item(self): self.stubs.Set(nova.db, 'flavor_extra_specs_update_or_create',