From 9f9629ada550c7ddf66fd03d6604e16a5712881f Mon Sep 17 00:00:00 2001 From: Kamil Rykowski Date: Mon, 18 May 2015 13:19:54 +0200 Subject: [PATCH] Change status code from 500 to 400 for image update request Invalid 'op' (operation) value provided for image update PATCH request results in raising Internal Server Error (500). There is no helpful information what went wrong and how to fix this particular request. Proposed fix change the status code from 500 to 400 and return proper message, so the user won't be confused and will be able to adjust the request. Change-Id: Ie20a3f350cbaa46c558b4e49b46091d1e4a74807 Closes-Bug: 1456157 --- glance/api/v2/images.py | 20 +++++++++++++++----- glance/tests/unit/v2/test_images_resource.py | 1 + 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 3901d00c53..1f72e651dd 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -329,6 +329,8 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer): _default_sort_dir = 'desc' + _supported_operations = ('add', 'remove', 'replace') + def __init__(self, schema=None): super(RequestDeserializer, self).__init__() self.schema = schema or get_schema() @@ -372,15 +374,23 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer): return dict(image=image, extra_properties=properties, tags=tags) def _get_change_operation_d10(self, raw_change): - try: - return raw_change['op'] - except KeyError: - msg = _("Unable to find '%s' in JSON Schema change") % 'op' + op = raw_change.get('op') + if op is None: + msg = _('Unable to find `op` in JSON Schema change. ' + 'It must be one of the following: %(available)s.') % \ + {'available': ', '.join(self._supported_operations)} raise webob.exc.HTTPBadRequest(explanation=msg) + if op not in self._supported_operations: + msg = _('Invalid operation: `%(op)s`. ' + 'It must be one of the following: %(available)s.') % \ + {'op': op, + 'available': ', '.join(self._supported_operations)} + raise webob.exc.HTTPBadRequest(explanation=msg) + return op def _get_change_operation_d4(self, raw_change): op = None - for key in ['replace', 'add', 'remove']: + for key in self._supported_operations: if key in raw_change: if op is not None: msg = _('Operation objects must contain only one member' diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 8eb032f511..40482c53f1 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -2191,6 +2191,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase): True, False, None, + {'op': 'invalid', 'path': '/name', 'value': 'fedora'} ] for change in changes: request = self._get_fake_patch_request()