openstack/cinder
Code Issues Proposed changes

Validate uuid parameters strictly for create volume API

Browse Source 
Parameters like "consistencygroup_id" "source_volid" and
"source_replica" are expected to be in UUID format but if user
passes non-UUID format value like '1', create api searches for
that particular entity in database and returns 404 NotFound.
Also If user passes any non-integer value like [](empty list),
{}(empty dict), it returns 500 error. This patch proposes to
validate uuid parameters are adhering to the uuid format otherwise
it will return 400 BadRequest.

APIImpact: If user passes non-uuid values to consistencygroup_id,
source_volid and source_replica parameters in the request body of
create volume API, then it will return 400 error instead of
404/500 error.

Closes-Bug: #1680709
Change-Id: I31ea4f378be380a783d1c0249552ded8794fc52e
This commit is contained in:
dineshbhor 2017-03-24 18:55:12 +05:30
parent c85dcdbe21
commit 2d4a804876
5 changed files with 60 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
cinder/api/v2/volumes.py
View File

@ -215,6 +215,10 @@ class VolumeController(wsgi.Controller):
source_volid = volume.get('source_volid')
if source_volid is not None:
if not uuidutils.is_uuid_like(source_volid):
msg = _("Source volume ID '%s' must be a "
"valid UUID.") % source_volid
raise exc.HTTPBadRequest(explanation=msg)
# Not found exception will be handled at the wsgi level
kwargs['source_volume'] = \
self.volume_api.get_volume(context,
@ -224,6 +228,10 @@ class VolumeController(wsgi.Controller):
source_replica = volume.get('source_replica')
if source_replica is not None:
if not uuidutils.is_uuid_like(source_replica):
msg = _("Source replica ID '%s' must be a "
"valid UUID") % source_replica
raise exc.HTTPBadRequest(explanation=msg)
# Not found exception will be handled at the wsgi level
src_vol = self.volume_api.get_volume(context,
source_replica)
@ -239,6 +247,10 @@ class VolumeController(wsgi.Controller):
kwargs['consistencygroup'] = None
consistencygroup_id = volume.get('consistencygroup_id')
if consistencygroup_id is not None:
if not uuidutils.is_uuid_like(consistencygroup_id):
msg = _("Consistency group ID '%s' must be a "
"valid UUID.") % consistencygroup_id
raise exc.HTTPBadRequest(explanation=msg)
# Not found exception will be handled at the wsgi level
kwargs['group'] = self.group_api.get(context, consistencygroup_id)

12
cinder/api/v3/volumes.py
View File

@ -214,6 +214,10 @@ class VolumeController(volumes_v2.VolumeController):
source_volid = volume.get('source_volid')
if source_volid is not None:
if not uuidutils.is_uuid_like(source_volid):
msg = _("Source volume ID '%s' must be a "
"valid UUID.") % source_volid
raise exc.HTTPBadRequest(explanation=msg)
# Not found exception will be handled at the wsgi level
kwargs['source_volume'] = (
self.volume_api.get_volume(context,
@ -223,6 +227,10 @@ class VolumeController(volumes_v2.VolumeController):
source_replica = volume.get('source_replica')
if source_replica is not None:
if not uuidutils.is_uuid_like(source_replica):
msg = _("Source replica ID '%s' must be a "
"valid UUID") % source_replica
raise exc.HTTPBadRequest(explanation=msg)
# Not found exception will be handled at the wsgi level
src_vol = self.volume_api.get_volume(context,
source_replica)
@ -238,6 +246,10 @@ class VolumeController(volumes_v2.VolumeController):
kwargs['consistencygroup'] = None
consistencygroup_id = volume.get('consistencygroup_id')
if consistencygroup_id is not None:
if not uuidutils.is_uuid_like(consistencygroup_id):
msg = _("Consistency group ID '%s' must be a "
"valid UUID.") % consistencygroup_id
raise exc.HTTPBadRequest(explanation=msg)
# Not found exception will be handled at the wsgi level
kwargs['group'] = self.group_api.get(context, consistencygroup_id)

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

@ -337,6 +337,21 @@ class VolumeApiTest(test.TestCase):
get_volume.assert_called_once_with(self.controller.volume_api,
context, source_volid)
@ddt.data({'source_volid': 1},
{'source_volid': []},
{'source_replica': 1},
{'source_replica': []},
{'consistencygroup_id': 1},
{'consistencygroup_id': []})
def test_volume_creation_fails_with_invalid_uuids(self, updated_uuids):
vol = self._vol_in_request_body()
vol.update(updated_uuids)
body = {"volume": vol}
req = fakes.HTTPRequest.blank('/v2/volumes')
# Raise 400 for resource requested with invalid uuids.
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
req, body)
@mock.patch.object(volume_api.API, 'get_volume', autospec=True)
def test_volume_creation_fails_with_invalid_source_replica(self,
get_volume):

15
cinder/tests/unit/api/v3/test_volumes.py
View File

@ -373,6 +373,21 @@ class VolumeApiTest(test.TestCase):
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
req, body)
@ddt.data({'source_volid': 1},
{'source_volid': []},
{'source_replica': 1},
{'source_replica': []},
{'consistencygroup_id': 1},
{'consistencygroup_id': []})
def test_volume_creation_fails_with_invalid_uuids(self, updated_uuids):
vol = self._vol_in_request_body()
vol.update(updated_uuids)
body = {"volume": vol}
req = fakes.HTTPRequest.blank('/v2/volumes')
# Raise 400 for resource requested with invalid uuids.
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
req, body)
@ddt.data({'admin': True, 'version': '3.21'},
{'admin': False, 'version': '3.21'},
{'admin': True, 'version': '3.20'},

6
releasenotes/notes/validate_vol_create_uuids-4f08b4ef201385f6.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
The create volume api will now return 400 error instead of 404/500 if user
passes non-uuid values to consistencygroup_id, source_volid and
source_replica parameters in the request body.