openstack
/
cinder
Code Issues Proposed changes

Fix metadata api's raises TypeError 

Volume and Snapshot related metadata api's are raising
"TypeError: object of type 'NoneType' has no len()" if incorrect
formatted metadata is passed. The common
'utils.check_metadata_properties' method is not validating the
metadata properly. This TypeError exception raised is converted
into HTTPBadRequest at wsgi layer.

Fixed this issue by raising an 'InvalidVolumeMetadata' exception
if incorrect formatted metadata is passed to these api's giving
400 HTTPBadRequest to the user. There will no defference in terms
of response status code from user's point of view.

Closes-Bug: #1674233
Change-Id: Id86bb9d0887cebdb007df973cf87257924560fbe
This commit is contained in:
dineshbhor 2017-03-20 17:00:30 +05:30
parent 39e7f70dee
commit 4d4ba60f9b
6 changed files with 80 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cinder/tests/unit/api/contrib/test_volume_manage.py
View File

@ -453,6 +453,7 @@ class VolumeManageTest(test.TestCase):
@ddt.data({'a' * 256: 'a'},
{'a': 'a' * 256},
{'': 'a'},
{'a': None},
)
def test_manage_volume_with_invalid_metadata(self, value):
body = {'volume': {'host': 'host_ok',

42
cinder/tests/unit/api/v2/test_snapshot_metadata.py
View File

@ -15,6 +15,7 @@
import uuid
import ddt
import mock
from oslo_serialization import jsonutils
import webob
@ -104,6 +105,7 @@ def return_snapshot_nonexistent(context, snapshot_id):
raise exception.SnapshotNotFound(snapshot_id=snapshot_id)
@ddt.ddt
class SnapshotMetaDataTest(test.TestCase):
def setUp(self):
@ -605,12 +607,21 @@ class SnapshotMetaDataTest(test.TestCase):
self.controller.update,
req, self.req_id, "key1", body)
def test_update_item_too_many_keys(self):
@ddt.data({"meta": {"key1": "value1", "key2": "value2"}},
{"meta": {"key1": None}})
@mock.patch('cinder.objects.Snapshot.get_by_id')
def test_update_invalid_metadata(self, body, snapshot_get_by_id):
snapshot = {
'id': self.req_id,
'expected_attrs': ['metadata']
}
self.mock_object(cinder.db, 'snapshot_metadata_update',
return_create_snapshot_metadata)
ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True)
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctx, **snapshot)
snapshot_get_by_id.return_value = snapshot_obj
req = fakes.HTTPRequest.blank(self.url + '/key1')
req.method = 'PUT'
body = {"meta": {"key1": "value1", "key2": "value2"}}
req.body = jsonutils.dump_as_bytes(body)
req.headers["content-type"] = "application/json"
@ -631,8 +642,12 @@ class SnapshotMetaDataTest(test.TestCase):
self.controller.update, req, self.req_id, 'bad',
body)
@ddt.data({"metadata": {"a" * 260: "value1"}},
{"metadata": {"key": "v" * 260}},
{"metadata": {"": "value1"}},
{"metadata": {"key": None}})
@mock.patch('cinder.objects.Snapshot.get_by_id')
def test_invalid_metadata_items_on_create(self, snapshot_get_by_id):
def test_invalid_metadata_items_on_create(self, data, snapshot_get_by_id):
snapshot = {
'id': self.req_id,
'expected_attrs': ['metadata']
@ -647,20 +662,11 @@ class SnapshotMetaDataTest(test.TestCase):
req.method = 'POST'
req.headers["content-type"] = "application/json"
# test for long key
data = {"metadata": {"a" * 260: "value1"}}
req.body = jsonutils.dump_as_bytes(data)
self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
self.controller.create, req, self.req_id, data)
exc = webob.exc.HTTPBadRequest
if (len(list(data['metadata'].keys())[0]) > 255 or
(list(data['metadata'].values())[0] is not None and
len(list(data['metadata'].values())[0]) > 255)):
exc = webob.exc.HTTPRequestEntityTooLarge
# test for long value
data = {"metadata": {"key": "v" * 260}}
req.body = jsonutils.dump_as_bytes(data)
self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
self.controller.create, req, self.req_id, data)
# test for empty key.
data = {"metadata": {"": "value1"}}
req.body = jsonutils.dump_as_bytes(data)
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, self.req_id, data)
self.assertRaises(exc, self.controller.create, req, self.req_id, data)

23
cinder/tests/unit/api/v2/test_volume_metadata.py
View File

@ -336,6 +336,16 @@ class volumeMetaDataTest(test.TestCase):
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, self.req_id, None)
def test_create_metadata_keys_value_none(self):
self.mock_object(db, 'volume_metadata_update',
return_create_volume_metadata)
req = fakes.HTTPRequest.blank(self.url)
req.method = 'POST'
req.headers["content-type"] = "application/json"
body = {"meta": {"key": None}}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, self.req_id, body)
def test_create_item_empty_key(self):
self.mock_object(db, 'volume_metadata_update',
return_create_volume_metadata)
@ -542,6 +552,19 @@ class volumeMetaDataTest(test.TestCase):
self.assertEqual(expected, res_dict)
get_volume.assert_called_once_with(fake_context, self.req_id)
def test_update_metadata_item_keys_value_none(self):
self.mock_object(db, 'volume_metadata_update',
return_create_volume_metadata)
req = fakes.HTTPRequest.blank(self.url + '/key1')
req.method = 'PUT'
body = {"meta": {"a": None}}
req.body = jsonutils.dump_as_bytes(body)
req.headers["content-type"] = "application/json"
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.update,
req, self.req_id, 'key1', body)
@mock.patch.object(db, 'volume_metadata_update')
def test_update_item_volume_maintenance(self, metadata_update):
fake_volume = {'id': self.req_id, 'status': 'maintenance'}

10
cinder/tests/unit/api/v2/test_volumes.py
View File

@ -622,14 +622,15 @@ class VolumeApiTest(test.TestCase):
@ddt.data({'a' * 256: 'a'},
{'a': 'a' * 256},
{'': 'a'})
{'': 'a'},
{'a': None})
def test_volume_create_with_invalid_metadata(self, value):
vol = self._vol_in_request_body()
vol['metadata'] = value
body = {"volume": vol}
req = fakes.HTTPRequest.blank('/v2/volumes')
if len(list(value.keys())[0]) == 0:
if len(list(value.keys())[0]) == 0 or list(value.values())[0] is None:
exc = exception.InvalidVolumeMetadata
else:
exc = exception.InvalidVolumeMetadataSize
@ -789,7 +790,8 @@ class VolumeApiTest(test.TestCase):
@ddt.data({'a' * 256: 'a'},
{'a': 'a' * 256},
{'': 'a'})
{'': 'a'},
{'a': None})
@mock.patch.object(volume_api.API, 'get',
side_effect=v2_fakes.fake_volume_api_get, autospec=True)
def test_volume_update_with_invalid_metadata(self, value, get):
@ -799,7 +801,7 @@ class VolumeApiTest(test.TestCase):
body = {"volume": updates}
req = fakes.HTTPRequest.blank('/v2/volumes/%s' % fake.VOLUME_ID)
if len(list(value.keys())[0]) == 0:
if len(list(value.keys())[0]) == 0 or list(value.values())[0] is None:
exc = exception.InvalidVolumeMetadata
else:
exc = webob.exc.HTTPRequestEntityTooLarge

23
cinder/tests/unit/api/v3/test_volume_metadata.py
View File

@ -18,6 +18,7 @@ import uuid
import mock
from oslo_config import cfg
from oslo_serialization import jsonutils
import webob
from cinder.api import extensions
from cinder.api.v3 import volume_metadata
@ -232,3 +233,25 @@ class volumeMetaDataTest(test.TestCase):
expected = {'meta': {'key1': 'value1'}}
self.assertEqual(expected, res_dict)
get_volume.assert_called_once_with(fake_context, self.req_id)
def test_create_metadata_keys_value_none(self):
self.mock_object(db, 'volume_metadata_update',
return_create_volume_metadata)
req = fakes.HTTPRequest.blank(self.url, version="3.15")
req.method = 'POST'
req.headers["content-type"] = "application/json"
body = {"meta": {"key": None}}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, self.req_id, body)
def test_update_items_value_none(self):
self.mock_object(db, 'volume_metadata_update',
return_create_volume_metadata)
req = fakes.HTTPRequest.blank(self.url + '/key1', version="3.15")
req.method = 'PUT'
body = {"metadata": {"key": None}}
req.body = jsonutils.dump_as_bytes(body)
req.headers["content-type"] = "application/json"
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, self.req_id, body)

3
cinder/utils.py
View File

@ -174,6 +174,9 @@ def check_metadata_properties(metadata=None):
"characters.") % k
LOG.debug(msg)
raise exception.InvalidVolumeMetadataSize(reason=msg)
if v is None:
msg = _("Metadata property key '%s' value is None.") % k
raise exception.InvalidVolumeMetadata(reason=msg)
if len(v) > 255:
msg = _("Metadata property key %s value greater than "
"255 characters.") % k