From 5ec6f8f8e8b88f48300e66369840c2fafd06f4c5 Mon Sep 17 00:00:00 2001 From: Alejandro Emanuel Paredes Date: Wed, 22 Jan 2014 15:19:19 -0500 Subject: [PATCH] Extra_spec containing '/' can't be deleted This patch validates the keys of the extra specs before setting them. This will make possible to remove those keys. It allows alphanumeric characters, underscores, periods, colons and hyphens. Change-Id: Ic2e5d7ce42c8ecb7ee45cd168bd008035fd4bbdf Closes-Bug: #1259711 --- cinder/api/common.py | 16 +++++ cinder/api/contrib/types_extra_specs.py | 10 +++- .../api/contrib/test_types_extra_specs.py | 59 +++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/cinder/api/common.py b/cinder/api/common.py index d6fe9efbcd5..30a32eb7df6 100644 --- a/cinder/api/common.py +++ b/cinder/api/common.py @@ -48,6 +48,22 @@ LOG = logging.getLogger(__name__) XML_NS_V1 = 'http://docs.openstack.org/volume/api/v1' +# Regex that matches alphanumeric characters, periods, hypens, +# colons and underscores: +# ^ assert position at start of the string +# [\w\.\-\:\_] match expression +# $ assert position at end of the string +VALID_KEY_NAME_REGEX = re.compile(r"^[\w\.\-\:\_]+$", re.UNICODE) + + +def validate_key_names(key_names_list): + """Validate each item of the list to match key name regex.""" + for key_name in key_names_list: + if not VALID_KEY_NAME_REGEX.match(key_name): + return False + return True + + def get_pagination_params(request): """Return marker, limit tuple from request. diff --git a/cinder/api/contrib/types_extra_specs.py b/cinder/api/contrib/types_extra_specs.py index fd128561bfa..f2131d58ba2 100644 --- a/cinder/api/contrib/types_extra_specs.py +++ b/cinder/api/contrib/types_extra_specs.py @@ -17,6 +17,7 @@ import webob +from cinder.api import common from cinder.api import extensions from cinder.api.openstack import wsgi from cinder.api import xmlutil @@ -81,8 +82,8 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): raise webob.exc.HTTPBadRequest() self._check_type(context, type_id) - specs = body['extra_specs'] + self._check_key_names(specs.keys()) db.volume_type_extra_specs_update_or_create(context, type_id, specs) @@ -144,6 +145,13 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): notifier_api.INFO, notifier_info) return webob.Response(status_int=202) + def _check_key_names(self, keys): + if not common.validate_key_names(keys): + expl = _('Key names can only contain alphanumeric characters, ' + 'underscores, periods, colons and hyphens.') + + raise webob.exc.HTTPBadRequest(explanation=expl) + class Types_extra_specs(extensions.ExtensionDescriptor): """Type extra specs support.""" diff --git a/cinder/tests/api/contrib/test_types_extra_specs.py b/cinder/tests/api/contrib/test_types_extra_specs.py index ccf29ad949c..33ee7a138e9 100644 --- a/cinder/tests/api/contrib/test_types_extra_specs.py +++ b/cinder/tests/api/contrib/test_types_extra_specs.py @@ -18,6 +18,8 @@ from lxml import etree import webob +import mock + from cinder.api.contrib import types_extra_specs from cinder import exception from cinder.openstack.common.notifier import api as notifier_api @@ -143,6 +145,54 @@ class VolumeTypesExtraSpecsTest(test.TestCase): self.assertEqual('value1', res_dict['extra_specs']['key1']) + @mock.patch.object(cinder.db, 'volume_type_extra_specs_update_or_create') + def test_create_key_allowed_chars( + self, volume_type_extra_specs_update_or_create): + mock_return_value = {"key1": "value1", + "key2": "value2", + "key3": "value3", + "key4": "value4", + "key5": "value5"} + volume_type_extra_specs_update_or_create.\ + return_value = mock_return_value + + body = {"extra_specs": {"other_alphanum.-_:": "value1"}} + + self.assertEqual(len(test_notifier.NOTIFICATIONS), 0) + + req = fakes.HTTPRequest.blank(self.api_path) + res_dict = self.controller.create(req, 1, body) + self.assertEqual(len(test_notifier.NOTIFICATIONS), 1) + self.assertEqual('value1', + res_dict['extra_specs']['other_alphanum.-_:']) + + @mock.patch.object(cinder.db, 'volume_type_extra_specs_update_or_create') + def test_create_too_many_keys_allowed_chars( + self, volume_type_extra_specs_update_or_create): + mock_return_value = {"key1": "value1", + "key2": "value2", + "key3": "value3", + "key4": "value4", + "key5": "value5"} + volume_type_extra_specs_update_or_create.\ + return_value = mock_return_value + + body = {"extra_specs": {"other_alphanum.-_:": "value1", + "other2_alphanum.-_:": "value2", + "other3_alphanum.-_:": "value3"}} + + self.assertEqual(len(test_notifier.NOTIFICATIONS), 0) + + req = fakes.HTTPRequest.blank(self.api_path) + res_dict = self.controller.create(req, 1, body) + self.assertEqual(len(test_notifier.NOTIFICATIONS), 1) + self.assertEqual('value1', + res_dict['extra_specs']['other_alphanum.-_:']) + self.assertEqual('value2', + res_dict['extra_specs']['other2_alphanum.-_:']) + self.assertEqual('value3', + res_dict['extra_specs']['other3_alphanum.-_:']) + def test_update_item(self): self.stubs.Set(cinder.db, 'volume_type_extra_specs_update_or_create', @@ -192,6 +242,7 @@ class VolumeTypesExtraSpecsTest(test.TestCase): def _extra_specs_create_bad_body(self, body): req = fakes.HTTPRequest.blank('/v2/fake/types/1/extra_specs') req.method = 'POST' + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, req, '1', body) @@ -206,6 +257,14 @@ class VolumeTypesExtraSpecsTest(test.TestCase): body = {'extra_specs': 'string'} self._extra_specs_create_bad_body(body=body) + def test_create_invalid_key(self): + body = {"extra_specs": {"ke/y1": "value1"}} + self._extra_specs_create_bad_body(body=body) + + def test_create_invalid_too_many_key(self): + body = {"key1": "value1", "ke/y2": "value2", "key3": "value3"} + self._extra_specs_create_bad_body(body=body) + class VolumeTypeExtraSpecsSerializerTest(test.TestCase): def test_index_create_serializer(self):