From 2d4a8048762b6453b075c29c58c7ab063a9102cf Mon Sep 17 00:00:00 2001 From: dineshbhor Date: Fri, 24 Mar 2017 18:55:12 +0530 Subject: [PATCH] Validate uuid parameters strictly for create volume API 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 --- cinder/api/v2/volumes.py | 12 ++++++++++++ cinder/api/v3/volumes.py | 12 ++++++++++++ cinder/tests/unit/api/v2/test_volumes.py | 15 +++++++++++++++ cinder/tests/unit/api/v3/test_volumes.py | 15 +++++++++++++++ ...alidate_vol_create_uuids-4f08b4ef201385f6.yaml | 6 ++++++ 5 files changed, 60 insertions(+) create mode 100644 releasenotes/notes/validate_vol_create_uuids-4f08b4ef201385f6.yaml diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 756eb85e6c3..cb69ce39453 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -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) diff --git a/cinder/api/v3/volumes.py b/cinder/api/v3/volumes.py index eb587916da6..fed0346b030 100644 --- a/cinder/api/v3/volumes.py +++ b/cinder/api/v3/volumes.py @@ -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) diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 51535348151..3d4b348c8e8 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -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): diff --git a/cinder/tests/unit/api/v3/test_volumes.py b/cinder/tests/unit/api/v3/test_volumes.py index 9477f534c59..fdc1fc36238 100644 --- a/cinder/tests/unit/api/v3/test_volumes.py +++ b/cinder/tests/unit/api/v3/test_volumes.py @@ -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'}, diff --git a/releasenotes/notes/validate_vol_create_uuids-4f08b4ef201385f6.yaml b/releasenotes/notes/validate_vol_create_uuids-4f08b4ef201385f6.yaml new file mode 100644 index 00000000000..8f334f59e0f --- /dev/null +++ b/releasenotes/notes/validate_vol_create_uuids-4f08b4ef201385f6.yaml @@ -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.