From 400fc6ec7de96f1119fd4d9dc4130374d6ca5895 Mon Sep 17 00:00:00 2001 From: Eva Balycheva Date: Tue, 15 Mar 2016 05:49:37 +0300 Subject: [PATCH] Validate PUT of reserved queue attributes metadata We recently introduced reserved queue metadata feature in our API v2. And because it's a feature of API v2, we decided not to change at all our older APIs. But through our older APIs it's possible to PUT invalid queue attributes, which can cause unstable work of API v2. This patch makes API v1 restrict any metadata keys which names start with "_", because they are probably reserved queue attributes. This patch makes API v1.1 validate metadata like API v2 validates it now to ensure that the reserved queue attributes have valid values, before sending them to the database. This patch also adds unit tests to APIs. APIImpact Closes-Bug: 1557247 Change-Id: Idcd063787c8d8da49a9c3d126441f9336c85e7cd --- zaqar/tests/functional/wsgi/v1/test_queues.py | 2 +- .../unit/transport/wsgi/v1/test_validation.py | 16 ++++++++ .../transport/wsgi/v1_1/test_validation.py | 39 +++++++++++++++++++ zaqar/transport/wsgi/v1_0/metadata.py | 14 +++++-- zaqar/transport/wsgi/v1_1/queues.py | 16 +++++--- zaqar/transport/wsgi/v2_0/queues.py | 17 ++++---- 6 files changed, 83 insertions(+), 21 deletions(-) diff --git a/zaqar/tests/functional/wsgi/v1/test_queues.py b/zaqar/tests/functional/wsgi/v1/test_queues.py index 9f769c9b..10a07b77 100644 --- a/zaqar/tests/functional/wsgi/v1/test_queues.py +++ b/zaqar/tests/functional/wsgi/v1/test_queues.py @@ -208,7 +208,7 @@ class TestQueueMetaData(base.V1FunctionalTestBase): self.client.set_base_url(self.queue_metadata_url) @ddt.data({}, - {'_queue': 'Top Level field with _'}, + {'@queue': 'Top Level field with @'}, annotated('test_insert_queue_metadata_unicode', { u'\u6c49\u5b57': u'Unicode: \u6c49\u5b57' }), diff --git a/zaqar/tests/unit/transport/wsgi/v1/test_validation.py b/zaqar/tests/unit/transport/wsgi/v1/test_validation.py index 2da061fa..a0327ce3 100644 --- a/zaqar/tests/unit/transport/wsgi/v1/test_validation.py +++ b/zaqar/tests/unit/transport/wsgi/v1/test_validation.py @@ -109,3 +109,19 @@ class TestValidation(base.V1Base): self.addCleanup(self.simulate_delete, queue_path_with_v2, self.project_id) self.assertEqual(falcon.HTTP_201, self.srmock.status) + + def test_queue_metadata_putting(self): + # Ensure setting reserved queue attributes (which names start with + # '_') is not allowed in API v1. + + # Try set real _default_message_ttl queue attribute. + self.simulate_put(self.queue_path + '/metadata', + self.project_id, + body='{"_default_message_ttl": 60}') + self.assertEqual(falcon.HTTP_400, self.srmock.status) + + # Try set a fictional queue attribute. + self.simulate_put(self.queue_path + '/metadata', + self.project_id, + body='{"_min_message_niceness": 9000}') + self.assertEqual(falcon.HTTP_400, self.srmock.status) diff --git a/zaqar/tests/unit/transport/wsgi/v1_1/test_validation.py b/zaqar/tests/unit/transport/wsgi/v1_1/test_validation.py index 8c3d86c5..b8925a91 100644 --- a/zaqar/tests/unit/transport/wsgi/v1_1/test_validation.py +++ b/zaqar/tests/unit/transport/wsgi/v1_1/test_validation.py @@ -97,3 +97,42 @@ class TestValidation(base.V1_1Base): self.project_id, headers=empty_headers) self.assertEqual(falcon.HTTP_400, self.srmock.status) + + def test_queue_metadata_putting(self): + # Test _default_message_ttl + # TTL normal case + queue_1 = self.url_prefix + '/queues/queue1' + self.simulate_put(queue_1, + self.project_id, + body='{"_default_message_ttl": 60}') + self.addCleanup(self.simulate_delete, queue_1, self.project_id, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # TTL under min + self.simulate_put(queue_1, + self.project_id, + body='{"_default_message_ttl": 59}') + self.assertEqual(falcon.HTTP_400, self.srmock.status) + + # TTL over max + self.simulate_put(queue_1, + self.project_id, + body='{"_default_message_ttl": 1209601}') + self.assertEqual(falcon.HTTP_400, self.srmock.status) + + # Test _max_messages_post_size + # Size normal case + queue_2 = self.url_prefix + '/queues/queue2' + self.simulate_put(queue_2, + self.project_id, + body='{"_max_messages_post_size": 255}') + self.addCleanup(self.simulate_delete, queue_2, self.project_id, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Size over max + self.simulate_put(queue_2, + self.project_id, + body='{"_max_messages_post_size": 257}') + self.assertEqual(falcon.HTTP_400, self.srmock.status) diff --git a/zaqar/transport/wsgi/v1_0/metadata.py b/zaqar/transport/wsgi/v1_0/metadata.py index 5e1741e2..0fb7d146 100644 --- a/zaqar/transport/wsgi/v1_0/metadata.py +++ b/zaqar/transport/wsgi/v1_0/metadata.py @@ -61,14 +61,20 @@ class Resource(object): try: # Place JSON size restriction before parsing self._validate.queue_metadata_length(req.content_length) + # Deserialize queue metadata + document = wsgi_utils.deserialize(req.stream, req.content_length) + metadata = wsgi_utils.sanitize(document, spec=None) + # Restrict setting any reserved queue attributes + for key in metadata: + if key.startswith('_'): + description = _(u'Reserved queue attributes in metadata ' + u'(which names start with "_") can not be ' + u'set in API v1.') + raise validation.ValidationFailed(description) except validation.ValidationFailed as ex: LOG.debug(ex) raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) - # Deserialize queue metadata - document = wsgi_utils.deserialize(req.stream, req.content_length) - metadata = wsgi_utils.sanitize(document, spec=None) - try: self._queue_ctrl.set_metadata(queue_name, metadata=metadata, diff --git a/zaqar/transport/wsgi/v1_1/queues.py b/zaqar/transport/wsgi/v1_1/queues.py index ce14a429..b031ac79 100644 --- a/zaqar/transport/wsgi/v1_1/queues.py +++ b/zaqar/transport/wsgi/v1_1/queues.py @@ -61,16 +61,20 @@ class ItemResource(object): try: # Place JSON size restriction before parsing self._validate.queue_metadata_length(req.content_length) + # Deserialize queue metadata + metadata = None + if req.content_length: + document = wsgi_utils.deserialize(req.stream, + req.content_length) + metadata = wsgi_utils.sanitize(document, spec=None) + # NOTE(Eva-i): reserved queue attributes is Zaqar's feature since + # API v2. But we have to ensure the bad data will not come from + # older APIs, so we validate metadata here. + self._validate.queue_metadata_putting(metadata) except validation.ValidationFailed as ex: LOG.debug(ex) raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) - # Deserialize queue metadata - metadata = None - if req.content_length: - document = wsgi_utils.deserialize(req.stream, req.content_length) - metadata = wsgi_utils.sanitize(document, spec=None) - try: created = self._queue_controller.create(queue_name, metadata=metadata, diff --git a/zaqar/transport/wsgi/v2_0/queues.py b/zaqar/transport/wsgi/v2_0/queues.py index 0019ed9b..6fb5aee1 100644 --- a/zaqar/transport/wsgi/v2_0/queues.py +++ b/zaqar/transport/wsgi/v2_0/queues.py @@ -63,18 +63,18 @@ class ItemResource(object): try: # Place JSON size restriction before parsing self._validate.queue_metadata_length(req.content_length) + # Deserialize queue metadata + metadata = None + if req.content_length: + document = wsgi_utils.deserialize(req.stream, + req.content_length) + metadata = wsgi_utils.sanitize(document, spec=None) + self._validate.queue_metadata_putting(metadata) except validation.ValidationFailed as ex: LOG.debug(ex) raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) - # Deserialize queue metadata - metadata = None - if req.content_length: - document = wsgi_utils.deserialize(req.stream, req.content_length) - metadata = wsgi_utils.sanitize(document, spec=None) - try: - self._validate.queue_metadata_putting(metadata) created = self._queue_controller.create(queue_name, metadata=metadata, project=project_id) @@ -82,9 +82,6 @@ class ItemResource(object): except storage_errors.FlavorDoesNotExist as ex: LOG.exception(ex) raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) - except validation.ValidationFailed as ex: - LOG.debug(ex) - raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) except Exception as ex: LOG.exception(ex) description = _(u'Queue could not be created.')