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

diff --git a/cinder/api/common.py b/cinder/api/common.py
index d6fe9efbcd5c2d4d75bf206daa867930122200be..30a32eb7df6febb434fea68a460f196514d4f94b 100644 (file)
--- 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 fd128561bfaabc2affb6df41feaa0d77fcf09ca7..f2131d58ba28b9c627b0a1b873e85684ac321785 100644 (file)
--- 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 ccf29ad949c0c55da81ab28ac9608a848661969f..33ee7a138e9e5b532950b8905bff8c26c09d9fbe 100644 (file)
--- 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):