fix(Request): Allow form posts for all methods except GET and HEAD (#923)

Fixes #919

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

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

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()
 

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

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')