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 63de65090a47..bc1da30dbc0e 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 625e9962f97b..7c22640ddfda 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 10cd3673b652..1801fbf522f6 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 8e38c4cb7f92..e4c41c32a4f0 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 dbfdd4eb81b2..d3c77f8c21a6 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',