From f6aec5bb96424668ac8ff9ae48899dc5354ea26a Mon Sep 17 00:00:00 2001
From: wangxiyuan <wangxiyuan@huawei.com>
Date: Fri, 1 Jul 2016 16:32:38 +0800
Subject: [PATCH] check the validity of metadata when update volume

The volume update API also allow users to update the volume'metadata.
But cinder doesn't check the length limit. It'will raise 500 error
if the metadata is longer than 255. We should check it and raise
413 at the api layer to keep the same with the volume metadata
update API.

Change-Id: I3928ffff9aab6a8071d0641fa6b40b10b1f6bc10
Closes-bug: #1598007
---
 cinder/api/v2/volumes.py                 |  4 +++
 cinder/tests/unit/api/v2/test_volumes.py | 43 ++++++++++++++++++++++++
 cinder/volume/api.py                     |  2 ++
 3 files changed, 49 insertions(+)

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index dc4fc2e55a9..aa5cde49462 100644
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -341,6 +341,10 @@ class VolumeController(wsgi.Controller):
             self.volume_api.update(context, volume, update_dict)
         except exception.VolumeNotFound as error:
             raise exc.HTTPNotFound(explanation=error.msg)
+        except exception.InvalidVolumeMetadata as error:
+            raise webob.exc.HTTPBadRequest(explanation=error.msg)
+        except exception.InvalidVolumeMetadataSize as error:
+            raise webob.exc.HTTPRequestEntityTooLarge(explanation=error.msg)
 
         volume.update(update_dict)
 
diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py
index c0aef9541d5..1c365f5a14e 100644
--- a/cinder/tests/unit/api/v2/test_volumes.py
+++ b/cinder/tests/unit/api/v2/test_volumes.py
@@ -23,6 +23,7 @@ import six
 from six.moves import range
 from six.moves import urllib
 import webob
+from webob import exc
 
 from cinder.api import common
 from cinder.api import extensions
@@ -659,6 +660,48 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(2, len(self.notifier.notifications))
         self.assertTrue(mock_validate.called)
 
+    @mock.patch(
+        'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
+    def test_volume_update_metadata_value_too_long(self, mock_validate):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get)
+
+        updates = {
+            "metadata": {"key1": ("a" * 260)}
+        }
+        body = {"volume": updates}
+        req = fakes.HTTPRequest.blank('/v2/volumes/%s' % fake.VOLUME_ID)
+        self.assertEqual(0, len(self.notifier.notifications))
+        self.assertRaises(exc.HTTPRequestEntityTooLarge,
+                          self.controller.update, req, fake.VOLUME_ID, body)
+
+    @mock.patch(
+        'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
+    def test_volume_update_metadata_key_too_long(self, mock_validate):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get)
+
+        updates = {
+            "metadata": {("a" * 260): "value1"}
+        }
+        body = {"volume": updates}
+        req = fakes.HTTPRequest.blank('/v2/volumes/%s' % fake.VOLUME_ID)
+        self.assertEqual(0, len(self.notifier.notifications))
+        self.assertRaises(exc.HTTPRequestEntityTooLarge,
+                          self.controller.update, req, fake.VOLUME_ID, body)
+
+    @mock.patch(
+        'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
+    def test_volume_update_metadata_empty_key(self, mock_validate):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get)
+
+        updates = {
+            "metadata": {"": "value1"}
+        }
+        body = {"volume": updates}
+        req = fakes.HTTPRequest.blank('/v2/volumes/%s' % fake.VOLUME_ID)
+        self.assertEqual(0, len(self.notifier.notifications))
+        self.assertRaises(exc.HTTPBadRequest,
+                          self.controller.update, req, fake.VOLUME_ID, body)
+
     @mock.patch(
         'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
     def test_volume_update_with_admin_metadata(self, mock_validate):
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 46c379c280e..2162ed6427c 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -449,6 +449,8 @@ class API(base.Base):
             msg = _("The volume cannot be updated during maintenance.")
             raise exception.InvalidVolume(reason=msg)
 
+        utils.check_metadata_properties(fields.get('metadata', None))
+
         volume.update(fields)
         volume.save()
         LOG.info(_LI("Volume updated successfully."), resource=volume)