diff --git a/CHANGES.rst b/CHANGES.rst index 447333c..4dd5d39 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -64,10 +64,10 @@ Fixed - When ``auto_parse_form_urlencoded`` is set to ``True``, the framework now checks the HTTP method before attempting to consume and parse the body. -- Before attempting to read a form-encoded POST, the framework now - checks the Content-Length header to ensure that a non-empty body - is expected. This helps prevent bad requests from causing a blocking - read when running behind certain WSGI servers. +- Before attempting to read the body of a form-encoded request, the + framework now checks the Content-Length header to ensure that a + non-empty body is expected. This helps prevent bad requests from + causing a blocking read when running behind certain WSGI servers. - When the requested method is not implemented for the target resource, the framework now raises ``falcon.HTTPMethodNotAllowed``, rather than modifying the ``falcon.Request`` object directly. This diff --git a/docs/changes/1.1.0.rst b/docs/changes/1.1.0.rst index 053f2f9..b1659b1 100644 --- a/docs/changes/1.1.0.rst +++ b/docs/changes/1.1.0.rst @@ -64,10 +64,10 @@ Fixed - When :any:`auto_parse_form_urlencoded` is set to ``True``, the framework now checks the HTTP method before attempting to consume and parse the body. -- Before attempting to read a form-encoded POST, the framework now - checks the Content-Length header to ensure that a non-empty body - is expected. This helps prevent bad requests from causing a blocking - read when running behind certain WSGI servers. +- Before attempting to read the body of a form-encoded request, the + framework now checks the Content-Length header to ensure that a + non-empty body is expected. This helps prevent bad requests from + causing a blocking read when running behind certain WSGI servers. - When the requested method is not implemented for the target resource, the framework now raises :class:`~falcon.HTTPMethodNotAllowed`, rather than modifying the :class:`~falcon.Request` object directly. This diff --git a/falcon/request.py b/falcon/request.py index b5fbc68..6e2a460 100644 --- a/falcon/request.py +++ b/falcon/request.py @@ -386,10 +386,11 @@ class Request(object): self.content_type is not None and 'application/x-www-form-urlencoded' in self.content_type and - # NOTE(kgriffs): POST is what we would normally expect, but - # just in case some apps like to color outside the lines, - # we'll allow PUT and PATCH to avoid breaking them. - self.method in ('POST', 'PUT', 'PATCH') + # NOTE(kgriffs): Within HTTP, a payload for a GET or HEAD + # request has no defined semantics, so we don't expect a + # body in those cases. We would normally not expect a body + # for OPTIONS either, but RFC 7231 does allow for it. + self.method not in ('GET', 'HEAD') ): self._parse_form_urlencoded() diff --git a/falcon/testing/__init__.py b/falcon/testing/__init__.py index d390156..4e134d7 100644 --- a/falcon/testing/__init__.py +++ b/falcon/testing/__init__.py @@ -74,7 +74,7 @@ supported:: from falcon.testing.base import TestBase # NOQA from falcon.testing.client import * # NOQA from falcon.testing.helpers import * # NOQA -from falcon.testing.resource import capture_responder_args # NOQA +from falcon.testing.resource import capture_responder_args, set_resp_defaults # NOQA from falcon.testing.resource import SimpleTestResource, TestResource # NOQA from falcon.testing.srmock import StartResponseMock # NOQA from falcon.testing.test_case import TestCase # NOQA diff --git a/tests/test_query_params.py b/tests/test_query_params.py index 3cfe398..03d10db 100644 --- a/tests/test_query_params.py +++ b/tests/test_query_params.py @@ -8,11 +8,39 @@ from falcon.errors import HTTPInvalidParam import falcon.testing as testing +class Resource(testing.SimpleTestResource): + + @falcon.before(testing.capture_responder_args) + @falcon.before(testing.set_resp_defaults) + def on_put(self, req, resp, **kwargs): + pass + + @falcon.before(testing.capture_responder_args) + @falcon.before(testing.set_resp_defaults) + def on_patch(self, req, resp, **kwargs): + pass + + @falcon.before(testing.capture_responder_args) + @falcon.before(testing.set_resp_defaults) + def on_delete(self, req, resp, **kwargs): + pass + + @falcon.before(testing.capture_responder_args) + @falcon.before(testing.set_resp_defaults) + def on_head(self, req, resp, **kwargs): + pass + + @falcon.before(testing.capture_responder_args) + @falcon.before(testing.set_resp_defaults) + def on_options(self, req, resp, **kwargs): + pass + + @ddt.ddt class _TestQueryParams(testing.TestBase): def before(self): - self.resource = testing.SimpleTestResource() + self.resource = Resource() self.api.add_route('/', self.resource) def test_none(self): @@ -566,23 +594,45 @@ class _TestQueryParams(testing.TestBase): 'payload') +@ddt.ddt class PostQueryParams(_TestQueryParams): def before(self): super(PostQueryParams, self).before() self.api.req_options.auto_parse_form_urlencoded = True def simulate_request(self, path, query_string, **kwargs): - headers = kwargs.setdefault('headers', {}) headers['Content-Type'] = 'application/x-www-form-urlencoded' + if 'method' not in kwargs: + kwargs['method'] = 'POST' + super(PostQueryParams, self).simulate_request( path, - method='POST', body=query_string, **kwargs ) + @ddt.data('POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS') + def test_http_methods_body_expected(self, http_method): + query_string = 'marker=deadbeef&limit=25' + self.simulate_request('/', query_string=query_string, + method=http_method) + + req = self.resource.captured_req + self.assertEqual(req.get_param('marker'), 'deadbeef') + self.assertEqual(req.get_param('limit'), '25') + + @ddt.data('GET', 'HEAD') + def test_http_methods_body_not_expected(self, http_method): + query_string = 'marker=deadbeef&limit=25' + self.simulate_request('/', query_string=query_string, + method=http_method) + + req = self.resource.captured_req + self.assertEqual(req.get_param('marker'), None) + self.assertEqual(req.get_param('limit'), None) + def test_non_ascii(self): value = u'\u8c46\u74e3' query_string = b'q=' + value.encode('utf-8')