Validated device_name value in block device map

1. Checked value of device_name parameter provided in block device
mapping section of Create server API.
2. Added unit test coverage.

Fixes LP: #1087165

Change-Id: I32ed59a577167d9bf42cb3667bfb1ba3963d6c71

2 files changed, 85 insertions, 5 deletions

diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index d3a6fc8a9..c6bbee98b 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -561,17 +561,28 @@ class Controller(wsgi.Controller):
         req.cache_db_instance(instance)
         return instance
 
-    def _validate_server_name(self, value):
+    def _check_string_length(self, value, name, max_length=None):
         if not isinstance(value, basestring):
-            msg = _("Server name is not a string or unicode")
+            msg = _("%s is not a string or unicode") % name
             raise exc.HTTPBadRequest(explanation=msg)
 
         if not value.strip():
-            msg = _("Server name is an empty string")
+            msg = _("%s is an empty string") % name
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        if max_length and len(value) > max_length:
+            msg = _("%(name)s can be at most %(max_length)s "
+                    "characters.") % locals()
             raise exc.HTTPBadRequest(explanation=msg)
 
-        if not len(value) < 256:
-            msg = _("Server name must be less than 256 characters.")
+    def _validate_server_name(self, value):
+        self._check_string_length(value, 'Server name', max_length=255)
+
+    def _validate_device_name(self, value):
+        self._check_string_length(value, 'Device name', max_length=255)
+
+        if ' ' in value:
+            msg = _("Device name cannot include spaces.")
             raise exc.HTTPBadRequest(explanation=msg)
 
     def _get_injected_files(self, personality):
@@ -809,6 +820,7 @@ class Controller(wsgi.Controller):
         if self.ext_mgr.is_loaded('os-volumes'):
             block_device_mapping = server_dict.get('block_device_mapping', [])
             for bdm in block_device_mapping:
+                self._validate_device_name(bdm["device_name"])
                 if 'delete_on_termination' in bdm:
                     bdm['delete_on_termination'] = utils.bool_from_str(
                         bdm['delete_on_termination'])
diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index 2567558ab..01e547089 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -2197,6 +2197,74 @@ class ServersControllerCreateTest(test.TestCase):
         self.stubs.Set(compute_api.API, 'create', create)
         self._test_create_extra(params)
 
+    def test_create_instance_with_device_name_not_string(self):
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        bdm = [{'delete_on_termination': 1,
+                'device_name': 123,
+                'volume_size': 1,
+                'volume_id': '11111111-1111-1111-1111-111111111111'}]
+        params = {'block_device_mapping': bdm}
+        old_create = compute_api.API.create
+
+        def create(*args, **kwargs):
+            self.assertEqual(kwargs['block_device_mapping'], bdm)
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(compute_api.API, 'create', create)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self._test_create_extra, params)
+
+    def test_create_instance_with_device_name_empty(self):
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        bdm = [{'delete_on_termination': 1,
+                'device_name': '',
+                'volume_size': 1,
+                'volume_id': '11111111-1111-1111-1111-111111111111'}]
+        params = {'block_device_mapping': bdm}
+        old_create = compute_api.API.create
+
+        def create(*args, **kwargs):
+            self.assertEqual(kwargs['block_device_mapping'], bdm)
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(compute_api.API, 'create', create)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self._test_create_extra, params)
+
+    def test_create_instance_with_device_name_too_long(self):
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        bdm = [{'delete_on_termination': 1,
+                'device_name': 'a' * 256,
+                'volume_size': 1,
+                'volume_id': '11111111-1111-1111-1111-111111111111'}]
+        params = {'block_device_mapping': bdm}
+        old_create = compute_api.API.create
+
+        def create(*args, **kwargs):
+            self.assertEqual(kwargs['block_device_mapping'], bdm)
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(compute_api.API, 'create', create)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self._test_create_extra, params)
+
+    def test_create_instance_with_space_in_device_name(self):
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        bdm = [{'delete_on_termination': 1,
+                'device_name': 'vd a',
+                'volume_size': 1,
+                'volume_id': '11111111-1111-1111-1111-111111111111'}]
+        params = {'block_device_mapping': bdm}
+        old_create = compute_api.API.create
+
+        def create(*args, **kwargs):
+            self.assertEqual(kwargs['block_device_mapping'], bdm)
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(compute_api.API, 'create', create)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self._test_create_extra, params)
+
     def test_create_instance_with_bdm_delete_on_termination(self):
         self.ext_mgr.extensions = {'os-volumes': 'fake'}
         bdm = [{'device_name': 'foo1', 'delete_on_termination': 1},