From e1d92ebd896234df76a6d1a1327be7aad1310da2 Mon Sep 17 00:00:00 2001 From: Kurt Griffiths Date: Mon, 27 Oct 2014 12:35:44 -0500 Subject: [PATCH] fix(Request): Improve support for content-length under wsgiref Under wsgiref, if no content-length header is provided, an empty string is inserted. Update falcon.Request to handle this case, as well as to more gracefully handle the case that the header is set to any invalid value. --- falcon/request.py | 37 ++++++++++++++++++++++-------- falcon/testing/helpers.py | 2 +- tests/bare_bones.py | 20 ++++++++++++++++ tests/test_req_vars.py | 3 +-- tests/test_wsgi.py | 48 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 12 deletions(-) create mode 100644 tests/bare_bones.py diff --git a/falcon/request.py b/falcon/request.py index 81cf1a0..69eccea 100644 --- a/falcon/request.py +++ b/falcon/request.py @@ -236,10 +236,8 @@ class Request(object): # NOTE(kgriffs): Wrap wsgi.input if needed to make read() more robust, # normalizing semantics between, e.g., gunicorn and wsgiref. - if isinstance(self.stream, NativeStream): # pragma: nocover - # NOTE(kgriffs): coverage can't detect that this *is* actually - # covered since the test that does so uses multiprocessing. - self.stream = helpers.Body(self.stream, self.content_length) + if isinstance(self.stream, NativeStream): + self._wrap_stream() # PERF(kgriffs): Technically, we should spend a few more # cycles and parse the content type for real, but @@ -291,6 +289,15 @@ class Request(object): except KeyError: return None + # NOTE(kgriffs): Normalize an empty value to behave as if + # the header were not included; wsgiref, at least, inserts + # an empty CONTENT_LENGTH value if the request does not + # set the header. Gunicorn and uWSGI do not do this, but + # others might if they are trying to match wsgiref's + # behavior too closely. + if not value: + return None + try: value_as_int = int(value) except ValueError: @@ -847,6 +854,18 @@ class Request(object): # Helpers # ------------------------------------------------------------------------ + def _wrap_stream(self): + try: + # NOTE(kgriffs): We can only add the wrapper if the + # content-length header was provided. + if self.content_length is not None: + self.stream = helpers.Body(self.stream, self.content_length) + + except HTTPInvalidHeader: + # NOTE(kgriffs): The content-length header was specified, + # but it had an invalid value. + pass + def _parse_form_urlencoded(self): # NOTE(kgriffs): This assumes self.stream has been patched # above in the case of wsgiref, so that self.content_length @@ -870,11 +889,11 @@ class Request(object): 'application/x-www-form-urlencoded. Body ' 'will be ignored.') - if body: - extra_params = uri.parse_query_string( - uri.decode(body), - keep_blank_qs_values=self.options.keep_blank_qs_values, - ) + if body: + extra_params = uri.parse_query_string( + uri.decode(body), + keep_blank_qs_values=self.options.keep_blank_qs_values, + ) self._params.update(extra_params) diff --git a/falcon/testing/helpers.py b/falcon/testing/helpers.py index 6034369..0c686c5 100644 --- a/falcon/testing/helpers.py +++ b/falcon/testing/helpers.py @@ -96,7 +96,7 @@ def create_environ(path='/', query_string='', protocol='HTTP/1.1', # NOTE(kgriffs): nocover since this branch will never be # taken in Python3. However, the branch is tested under Py2, # in test_utils.TestFalconTesting.test_unicode_path_in_create_environ - if six.PY2 and isinstance(path, unicode): # pragma: nocover + if six.PY2 and isinstance(path, six.text_type): # pragma: nocover path = path.encode('utf-8') scheme = scheme.lower() diff --git a/tests/bare_bones.py b/tests/bare_bones.py new file mode 100644 index 0000000..246f9f1 --- /dev/null +++ b/tests/bare_bones.py @@ -0,0 +1,20 @@ +import falcon + + +class Things(object): + def on_get(self, req, resp): + pass + + +api = application = falcon.API() +api.add_route('/', Things()) + + +if __name__ == '__main__': + # import eventlet.wsgi + # import eventlet + # eventlet.wsgi.server(eventlet.listen(('localhost', 8000)), application) + + from wsgiref.simple_server import make_server + server = make_server('localhost', 8000, application) + server.serve_forever() diff --git a/tests/test_req_vars.py b/tests/test_req_vars.py index 59d72a6..94fbef8 100644 --- a/tests/test_req_vars.py +++ b/tests/test_req_vars.py @@ -445,8 +445,7 @@ class TestReqVars(testing.TestBase): headers = {'content-length': ''} req = Request(testing.create_environ(headers=headers)) - self.assertRaises(falcon.HTTPInvalidHeader, - lambda: req.content_length) + self.assertEqual(req.content_length, None) def test_bogus_content_length_nan(self): headers = {'content-length': 'fuzzy-bunnies'} diff --git a/tests/test_wsgi.py b/tests/test_wsgi.py index 78cbfeb..33b4cf1 100644 --- a/tests/test_wsgi.py +++ b/tests/test_wsgi.py @@ -1,5 +1,9 @@ import sys +import threading +import time +import requests +from six.moves import http_client import testtools from testtools.matchers import Equals, MatchesRegex @@ -17,6 +21,22 @@ def _is_iterable(thing): return False +def _run_server(): + class Things(object): + def on_get(self, req, resp): + pass + + def on_put(self, req, resp): + pass + + api = application = falcon.API() + api.add_route('/', Things()) + + from wsgiref.simple_server import make_server + server = make_server('localhost', 9803, application) + server.serve_forever() + + class TestWsgi(testtools.TestCase): def test_srmock(self): @@ -55,3 +75,31 @@ class TestWsgi(testtools.TestCase): self.assertThat(len(header), Equals(2)) self.assertTrue(isinstance(header[0], str)) self.assertTrue(isinstance(header[1], str)) + + def test_wsgiref(self): + thread = threading.Thread(target=_run_server) + thread.daemon = True + thread.start() + + # Wait a moment for the thread to start up + time.sleep(0.1) + + resp = requests.get('http://localhost:9803') + self.assertEqual(resp.status_code, 200) + + # NOTE(kgriffs): This will cause a different path to + # be taken in req._wrap_stream. Have to use httplib + # to prevent the invalid header value from being + # forced to "0". + conn = http_client.HTTPConnection('localhost', 9803) + headers = {'Content-Length': 'invalid'} + conn.request('PUT', '/', headers=headers) + resp = conn.getresponse() + self.assertEqual(resp.status, 200) + + headers = {'Content-Length': '0'} + resp = requests.put('http://localhost:9803', headers=headers) + self.assertEqual(resp.status_code, 200) + + resp = requests.post('http://localhost:9803') + self.assertEqual(resp.status_code, 405)