From 53b5d7df48965aea408a4cd2f04a21bc6c778743 Mon Sep 17 00:00:00 2001 From: Eva Balycheva Date: Thu, 25 Feb 2016 22:03:13 +0300 Subject: [PATCH] Fix freeze on some requests Zaqar completely freezes when it gets HTTP request with body and with "Content-type" header's value: 'application/x-www-form-urlencoded'. For example, curl program sets such value to "Content-type" header by default. If "Content-type" header has any other value, Zaqar processes the request without any problems. Zaqar freezes, because inside "zaqar.transport.utils.read_json" function there is "stream.read(len)" expression that can't finish executing, when "Content-type" header is 'application/x-www-form-urlencoded'. And here's an explanation: 1. "stream.read(len)" can't finish executing, because "stream" is already exhausted and returns EOF. 2. The "stream" is already exhausted, because the Falcon library has already read it. 3. The Falcon library has already read it, because each time it accepts request with 'application/x-www-form-urlencoded' header, it automatically parses request's body into req.params. This patch adds new 'before' hook to WSGI driver, which raises an exception if request with body has "Content-type" header with 'application/x-www-form-urlencoded' value. To ensure the bug will not appear again, the patch adds tests. Alternatives to this patch: 1. Make Zaqar require "Content-type" header with 'application/json' value for requests with bodies, as Zaqar docs are asking, but this change may be not backward compatible with the clients who are currently not following the docs. 2. Make Zaqar not read stream, but read the whole data. But Zaqar was made to read stream on purpose: see bp/streaming-serialization. APIImpact Change-Id: I72ab1dd4620870e2722f911d22427e8f95063d3a Closes-Bug: 1547100 --- zaqar/common/transport/wsgi/helpers.py | 35 +++++++++++++++++++ .../unit/transport/wsgi/v1/test_media_type.py | 32 ++++++++++++++++- .../transport/wsgi/v1_1/test_media_type.py | 33 ++++++++++++++++- .../transport/wsgi/v2_0/test_media_type.py | 33 ++++++++++++++++- zaqar/transport/wsgi/driver.py | 1 + 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/zaqar/common/transport/wsgi/helpers.py b/zaqar/common/transport/wsgi/helpers.py index fc723987f..4953e38d7 100644 --- a/zaqar/common/transport/wsgi/helpers.py +++ b/zaqar/common/transport/wsgi/helpers.py @@ -186,6 +186,41 @@ media type support with the "Accept" header.''', href_text=u'14.1 Accept, Hypertext Transfer Protocol -- HTTP/1.1') +def require_content_type_be_non_urlencoded(req, resp, params): + """Raises an exception on "x-www-form-urlencoded" content type of request. + + If request has body and "Content-Type" header has + "application/x-www-form-urlencoded" value (case-insensitive), this function + raises falcon.HTTPBadRequest exception. + + This strange function exists only to prevent bug/1547100 in a backward + compatible way. + + Meant to be used as a `before` hook. + + :param req: request sent + :type req: falcon.request.Request + :param resp: response object to return + :type resp: falcon.response.Response + :param params: additional parameters passed to responders + :type params: dict + :rtype: None + :raises: falcon.HTTPBadRequest + + """ + if req.content_length is None: + return + if req.content_type and (req.content_type.lower() == + 'application/x-www-form-urlencoded'): + title = _(u'Invalid Content-Type') + description = _(u'Endpoint does not accept ' + u'`application/x-www-form-urlencoded` content; ' + u'currently supported media type is ' + u'`application/json`; specify proper client-side ' + u'media type with the "Content-Type" header.') + raise falcon.HTTPBadRequest(title, description) + + def inject_context(req, resp, params): """Inject context value into request environment. diff --git a/zaqar/tests/unit/transport/wsgi/v1/test_media_type.py b/zaqar/tests/unit/transport/wsgi/v1/test_media_type.py index b714af144..ae64f3290 100644 --- a/zaqar/tests/unit/transport/wsgi/v1/test_media_type.py +++ b/zaqar/tests/unit/transport/wsgi/v1/test_media_type.py @@ -17,6 +17,7 @@ import uuid import falcon from falcon import testing +from oslo_serialization import jsonutils from zaqar.tests.unit.transport.wsgi import base @@ -26,7 +27,7 @@ class TestMediaType(base.V1Base): config_file = 'wsgi_mongodb.conf' - def test_json_only_endpoints(self): + def test_json_only_endpoints_with_wrong_accept_header(self): endpoints = ( ('GET', self.url_prefix + '/queues'), ('GET', self.url_prefix + '/queues/nonexistent/metadata'), @@ -50,3 +51,32 @@ class TestMediaType(base.V1Base): self.app(env, self.srmock) self.assertEqual(falcon.HTTP_406, self.srmock.status) + + def test_request_with_body_and_urlencoded_contenttype_header_fails(self): + # NOTE(Eva-i): this test case makes sure wsgi 'before' hook + # "require_content_type_be_non_urlencoded" works to prevent + # bug/1547100. + eww_queue_path = self.url_prefix + '/queues/eww' + eww_queue_messages_path = eww_queue_path + '/messages' + sample_message = jsonutils.dumps([{'body': {'eww!'}, 'ttl': 200}]) + bad_headers = { + 'Client-ID': str(uuid.uuid4()), + 'Content-Type': 'application/x-www-form-urlencoded', + } + + # Create queue request with bad headers. Should still work, because it + # has no body. + self.simulate_put(eww_queue_path, headers=bad_headers) + self.addCleanup(self.simulate_delete, eww_queue_path, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Post message request with good headers. Should work. + self.simulate_post(eww_queue_messages_path, body=sample_message, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Post message request with bad headers. Should not work. + self.simulate_post(eww_queue_messages_path, body=sample_message, + headers=bad_headers) + self.assertEqual(falcon.HTTP_400, self.srmock.status) diff --git a/zaqar/tests/unit/transport/wsgi/v1_1/test_media_type.py b/zaqar/tests/unit/transport/wsgi/v1_1/test_media_type.py index faa88e771..a75855de2 100644 --- a/zaqar/tests/unit/transport/wsgi/v1_1/test_media_type.py +++ b/zaqar/tests/unit/transport/wsgi/v1_1/test_media_type.py @@ -17,6 +17,7 @@ import uuid import falcon from falcon import testing +from oslo_serialization import jsonutils from zaqar.tests.unit.transport.wsgi import base @@ -25,7 +26,7 @@ class TestMediaType(base.V1_1Base): config_file = 'wsgi_mongodb.conf' - def test_json_only_endpoints(self): + def test_json_only_endpoints_with_wrong_accept_header(self): endpoints = ( ('GET', self.url_prefix + '/queues'), ('GET', self.url_prefix + '/queues/nonexistent/stats'), @@ -48,3 +49,33 @@ class TestMediaType(base.V1_1Base): self.app(env, self.srmock) self.assertEqual(falcon.HTTP_406, self.srmock.status) + + def test_request_with_body_and_urlencoded_contenttype_header_fails(self): + # NOTE(Eva-i): this test case makes sure wsgi 'before' hook + # "require_content_type_be_non_urlencoded" works to prevent + # bug/1547100. + eww_queue_path = self.url_prefix + '/queues/eww' + eww_queue_messages_path = eww_queue_path + '/messages' + sample_message = jsonutils.dumps({'messages': [{'body': {'eww!'}, + 'ttl': 200}]}) + bad_headers = { + 'Client-ID': str(uuid.uuid4()), + 'Content-Type': 'application/x-www-form-urlencoded', + } + + # Create queue request with bad headers. Should still work, because it + # has no body. + self.simulate_put(eww_queue_path, headers=bad_headers) + self.addCleanup(self.simulate_delete, eww_queue_path, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Post message request with good headers. Should work. + self.simulate_post(eww_queue_messages_path, body=sample_message, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Post message request with bad headers. Should not work. + self.simulate_post(eww_queue_messages_path, body=sample_message, + headers=bad_headers) + self.assertEqual(falcon.HTTP_400, self.srmock.status) diff --git a/zaqar/tests/unit/transport/wsgi/v2_0/test_media_type.py b/zaqar/tests/unit/transport/wsgi/v2_0/test_media_type.py index 7c4cc7e8f..223dea7c4 100644 --- a/zaqar/tests/unit/transport/wsgi/v2_0/test_media_type.py +++ b/zaqar/tests/unit/transport/wsgi/v2_0/test_media_type.py @@ -17,6 +17,7 @@ import uuid import falcon from falcon import testing +from oslo_serialization import jsonutils from zaqar.tests.unit.transport.wsgi import base @@ -25,7 +26,7 @@ class TestMediaType(base.V2Base): config_file = 'wsgi_mongodb.conf' - def test_json_only_endpoints(self): + def test_json_only_endpoints_with_wrong_accept_header(self): endpoints = ( ('GET', self.url_prefix + '/queues'), ('GET', self.url_prefix + '/queues/nonexistent/stats'), @@ -48,3 +49,33 @@ class TestMediaType(base.V2Base): self.app(env, self.srmock) self.assertEqual(falcon.HTTP_406, self.srmock.status) + + def test_request_with_body_and_urlencoded_contenttype_header_fails(self): + # NOTE(Eva-i): this test case makes sure wsgi 'before' hook + # "require_content_type_be_non_urlencoded" works to prevent + # bug/1547100. + eww_queue_path = self.url_prefix + '/queues/eww' + eww_queue_messages_path = eww_queue_path + '/messages' + sample_message = jsonutils.dumps({'messages': [{'body': {'eww!'}, + 'ttl': 200}]}) + bad_headers = { + 'Client-ID': str(uuid.uuid4()), + 'Content-Type': 'application/x-www-form-urlencoded', + } + + # Create queue request with bad headers. Should still work, because it + # has no body. + self.simulate_put(eww_queue_path, headers=bad_headers) + self.addCleanup(self.simulate_delete, eww_queue_path, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Post message request with good headers. Should work. + self.simulate_post(eww_queue_messages_path, body=sample_message, + headers=self.headers) + self.assertEqual(falcon.HTTP_201, self.srmock.status) + + # Post message request with bad headers. Should not work. + self.simulate_post(eww_queue_messages_path, body=sample_message, + headers=bad_headers) + self.assertEqual(falcon.HTTP_400, self.srmock.status) diff --git a/zaqar/transport/wsgi/driver.py b/zaqar/transport/wsgi/driver.py index aa7575f1c..f46da9248 100644 --- a/zaqar/transport/wsgi/driver.py +++ b/zaqar/transport/wsgi/driver.py @@ -75,6 +75,7 @@ class Driver(transport.DriverBase): """Exposed to facilitate unit testing.""" return [ self._verify_pre_signed_url, + helpers.require_content_type_be_non_urlencoded, helpers.require_accepts_json, helpers.require_client_id, helpers.extract_project_id,