Add entity body validation helper

Add a _valid_body() helper for the servers API to avoid repeating the
same tests in multiple methods. Include a check that the 'server' value
is actually a dict.

Also remove the superfluous checking of req.body which is a leftover
from when the body wasn't a method parameter.

Change-Id: If8114cc76d68567005c85c803f29e30e034db89a

4 files changed, 56 insertions, 21 deletions

diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index fc4905a11..4c4b86aa8 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -613,10 +613,7 @@ class Controller(wsgi.Controller):
     @wsgi.deserializers(xml=CreateDeserializer)
     def create(self, req, body):
         """Creates a new server for a given user."""
-        if not body:
-            raise exc.HTTPUnprocessableEntity()
-
-        if not 'server' in body:
+        if not self.is_valid_body(body, 'server'):
             raise exc.HTTPUnprocessableEntity()
 
         body['server']['key_name'] = self._get_key_name(req, body)
@@ -815,13 +812,7 @@ class Controller(wsgi.Controller):
     @wsgi.serializers(xml=ServerTemplate)
     def update(self, req, id, body):
         """Update server then pass on to version-specific controller."""
-        if len(req.body) == 0:
-            raise exc.HTTPUnprocessableEntity()
-
-        if not body:
-            raise exc.HTTPUnprocessableEntity()
-
-        if not 'server' in body:
+        if not self.is_valid_body(body, 'server'):
             raise exc.HTTPUnprocessableEntity()
 
         ctxt = req.environ['nova.context']
diff --git a/nova/api/openstack/wsgi.py b/nova/api/openstack/wsgi.py
index 658b59645..a9fa3847a 100644
--- a/nova/api/openstack/wsgi.py
+++ b/nova/api/openstack/wsgi.py
@@ -1102,6 +1102,23 @@ class Controller(object):
         else:
             self._view_builder = None
 
+    @staticmethod
+    def is_valid_body(body, entity_name):
+        if not (body and entity_name in body):
+            return False
+
+        def is_dict(d):
+            try:
+                d.get(None)
+                return True
+            except AttributeError:
+                return False
+
+        if not is_dict(body[entity_name]):
+            return False
+
+        return True
+
 
 class Fault(webob.exc.HTTPException):
     """Wrap webob.exc.HTTPException to provide API friendly response."""
diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index fa811fd2c..1a2026900 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -2765,16 +2765,6 @@ class ServersControllerCreateTest(test.TestCase):
         # The fact that the action doesn't raise is enough validation
         self.controller.create(req, body)
 
-    def test_create_instance_malformed_entity(self):
-        req = fakes.HTTPRequest.blank('/v2/fake/servers')
-        req.method = 'POST'
-        body = {'server': 'string'}
-        req.body = jsonutils.dumps(body)
-        req.headers['content-type'] = "application/json"
-
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller.create, req, body)
-
     def test_create_location(self):
         selfhref = 'http://localhost/v2/fake/servers/%s' % FAKE_UUID
         bookhref = 'http://localhost/fake/servers/%s' % FAKE_UUID
@@ -4866,6 +4856,10 @@ class ServersUnprocessableEntityTestCase(test.TestCase):
         body = {'foo': {'a': 'b'}}
         self._unprocessable_server_create(body=body)
 
+    def test_create_server_malformed_entity(self):
+        body = {'server': 'string'}
+        self._unprocessable_server_create(body=body)
+
     def _unprocessable_server_update(self, body):
         req = fakes.HTTPRequest.blank('/v2/fake/servers/%s' % FAKE_UUID)
         req.method = 'PUT'
@@ -4879,3 +4873,7 @@ class ServersUnprocessableEntityTestCase(test.TestCase):
     def test_update_server_missing_server(self):
         body = {'foo': {'a': 'b'}}
         self._unprocessable_server_update(body=body)
+
+    def test_create_update_malformed_entity(self):
+        body = {'server': 'string'}
+        self._unprocessable_server_update(body=body)
diff --git a/nova/tests/api/openstack/test_wsgi.py b/nova/tests/api/openstack/test_wsgi.py
index 7427eb2af..387940fc2 100644
--- a/nova/tests/api/openstack/test_wsgi.py
+++ b/nova/tests/api/openstack/test_wsgi.py
@@ -861,3 +861,32 @@ class ResponseObjectTest(test.TestCase):
             self.assertEqual(response.headers['X-header2'], 'header2')
             self.assertEqual(response.status_int, 202)
             self.assertEqual(response.body, mtype)
+
+
+class ValidBodyTest(test.TestCase):
+
+    def setUp(self):
+        super(ValidBodyTest, self).setUp()
+        self.controller = wsgi.Controller()
+
+    def test_is_valid_body(self):
+        body = {'foo': {}}
+        self.assertTrue(self.controller.is_valid_body(body, 'foo'))
+
+    def test_is_valid_body_none(self):
+        resource = wsgi.Resource(controller=None)
+        self.assertFalse(self.controller.is_valid_body(None, 'foo'))
+
+    def test_is_valid_body_empty(self):
+        resource = wsgi.Resource(controller=None)
+        self.assertFalse(self.controller.is_valid_body({}, 'foo'))
+
+    def test_is_valid_body_no_entity(self):
+        resource = wsgi.Resource(controller=None)
+        body = {'bar': {}}
+        self.assertFalse(self.controller.is_valid_body(body, 'foo'))
+
+    def test_is_valid_body_malformed_entity(self):
+        resource = wsgi.Resource(controller=None)
+        body = {'foo': 'bar'}
+        self.assertFalse(self.controller.is_valid_body(body, 'foo'))