fix(Request): Improve various related aspects of stream handling (#898)

* Add a new bounded_stream property that can be used for more
  predictable behavior vs. stream, albeit with a slight performance
  overhead (the app developer is free to decide whether or not to
  use it).
* Only automatically consume the incoming stream on POST requests,
  since that is the only time form-encoded params should be included
  in the body (vs. the query string). This guards against unexpected
  side-effects caused by misbehaving or even malicious clients.
* Check Content-Length to ensure a body is expected, before attempting
  to parse form-encoded POSTs. Also pass the Content-Length to
  stream.read as an extra safety measure to guard against differences
  in WSGI input read() behavior.
* Improve the documentation surrounding all of these behaviors.

Fixes #407

falcon/api.py

@@ -104,8 +104,8 @@ class API(object):
             See also: :ref:`Routing <routing>`.
 
     Attributes:
-        req_options (RequestOptions): A set of behavioral options related to
+        req_options: A set of behavioral options related to incoming
-            incoming requests.
+            requests. See also: :py:class:`~.RequestOptions`
     """
 
     # PERF(kgriffs): Reference via self since that is faster than

falcon/request.py

@@ -50,10 +50,6 @@ TRUE_STRINGS = ('true', 'True', 'yes', '1')
 FALSE_STRINGS = ('false', 'False', 'no', '0')
 WSGI_CONTENT_HEADERS = ('CONTENT_TYPE', 'CONTENT_LENGTH')
 
-
-_maybe_wrap_wsgi_stream = True
-
-
 # PERF(kgriffs): Avoid an extra namespace lookup when using these functions
 strptime = datetime.strptime
 now = datetime.now
@@ -173,7 +169,38 @@ class Request(object):
             the header is missing.
         content_length (int): Value of the Content-Length header converted
             to an ``int``, or ``None`` if the header is missing.
-        stream: File-like object for reading the body of the request, if any.
+        stream: File-like input object for reading the body of the
+            request, if any. Since this object is provided by the WSGI
+            server itself, rather than by Falcon, it may behave
+            differently depending on how you host your app. For example,
+            attempting to read more bytes than are expected (as
+            determined by the Content-Length header) may or may not
+            block indefinitely. It's a good idea to test your WSGI
+            server to find out how it behaves.
+
+            This can be particulary problematic when a request body is
+            expected, but none is given. In this case, the following
+            call blocks under certain WSGI servers::
+
+                # Blocks if Content-Length is 0
+                data = req.stream.read()
+
+            The workaround is fairly straightforward, if verbose::
+
+                # If Content-Length happens to be 0, or the header is
+                # missing altogether, this will not block.
+                data = req.stream.read(req.content_length or 0)
+
+            Alternatively, when passing the stream directly to a
+            consumer, it may be necessary to branch off the
+            value of the Content-Length header::
+
+                if req.content_length:
+                    doc = json.load(req.stream)
+
+            For a slight performance cost, you may instead wish to use
+            :py:attr:`bounded_stream`, which wraps the native WSGI
+            input object to normalize its behavior.
 
             Note:
                 If an HTML form is POSTed to the API using the
@@ -184,6 +211,23 @@ class Request(object):
                 and merge them into the query string parameters. In this
                 case, the stream will be left at EOF.
 
+        bounded_stream: File-like wrapper around `stream` to normalize
+            certain differences between the native input objects
+            employed by different WSGI servers. In particular,
+            `bounded_stream` is aware of the expected Content-Length of
+            the body, and will never block on out-of-bounds reads,
+            assuming the client does not stall while transmitting the
+            data to the server.
+
+            For example, the following will not block when
+            Content-Length is 0 or the header is missing altogether::
+
+                data = req.bounded_stream.read()
+
+            This is also safe::
+
+                doc = json.load(req.stream)
+
         date (datetime): Value of the Date header, converted to a
             ``datetime`` instance. The header value is assumed to
             conform to RFC 1123.
@@ -246,6 +290,7 @@ class Request(object):
         'path',
         'query_string',
         'stream',
+        '_bounded_stream',
         'context',
         '_wsgierrors',
         'options',
@@ -258,14 +303,14 @@ class Request(object):
     # Child classes may override this
     context_type = None
 
-    def __init__(self, env, options=None):
+    _wsgi_input_type_known = False
-        global _maybe_wrap_wsgi_stream
+    _always_wrap_wsgi_input = False
 
+    def __init__(self, env, options=None):
         self.env = env
         self.options = options if options else RequestOptions()
 
         self._wsgierrors = env['wsgi.errors']
-        self.stream = env['wsgi.input']
         self.method = env['REQUEST_METHOD']
 
         self.uri_template = None
@@ -316,21 +361,30 @@ class Request(object):
 
         # NOTE(kgriffs): Wrap wsgi.input if needed to make read() more robust,
         # normalizing semantics between, e.g., gunicorn and wsgiref.
-        if _maybe_wrap_wsgi_stream:
+        if not Request._wsgi_input_type_known:
-            if isinstance(self.stream, (NativeStream, InputWrapper,)):
+            Request._always_wrap_wsgi_input = isinstance(
-                self._wrap_stream()
+                env['wsgi.input'],
-            else:
+                (NativeStream, InputWrapper)
-                # PERF(kgriffs): If self.stream does not need to be wrapped
+            )
-                # this time, it never needs to be wrapped since the server
+
-                # will continue using the same type for wsgi.input.
+            Request._wsgi_input_type_known = True
-                _maybe_wrap_wsgi_stream = False
+
+        if Request._always_wrap_wsgi_input:
+            # TODO(kgriffs): In Falcon 2.0, stop wrapping stream since it is
+            # less useful now that we have bounded_stream.
+            self.stream = self._get_wrapped_wsgi_input()
+            self._bounded_stream = self.stream
+        else:
+            self.stream = env['wsgi.input']
+            self._bounded_stream = None  # Lazy wrapping
 
         # PERF(kgriffs): Technically, we should spend a few more
         # cycles and parse the content type for real, but
         # this heuristic will work virtually all the time.
         if (self.options.auto_parse_form_urlencoded and
                 self.content_type is not None and
-                'application/x-www-form-urlencoded' in self.content_type):
+                'application/x-www-form-urlencoded' in self.content_type and
+                self.method == 'POST'):
             self._parse_form_urlencoded()
 
         if self.context_type is None:
@@ -402,6 +456,13 @@ class Request(object):
 
         return value_as_int
 
+    @property
+    def bounded_stream(self):
+        if self._bounded_stream is None:
+            self._bounded_stream = self._get_wrapped_wsgi_input()
+
+        return self._bounded_stream
+
     @property
     def date(self):
         return self.get_header_as_datetime('Date')
@@ -750,9 +811,12 @@ class Request(object):
 
         Note:
             If an HTML form is POSTed to the API using the
-            *application/x-www-form-urlencoded* media type, the
+            *application/x-www-form-urlencoded* media type, Falcon can
-            parameters from the request body will be merged into
+            automatically parse the parameters from the request body
-            the query string parameters.
+            and merge them into the query string parameters. To enable
+            this functionality, set
+            :py:attr:`~.RequestOptions.auto_parse_form_urlencoded` to
+            ``True`` via :any:`API.req_options`.
 
             If a key appears more than once in the form data, one of the
             values will be returned as a string, but it is undefined which
@@ -1126,7 +1190,7 @@ class Request(object):
     # Helpers
     # ------------------------------------------------------------------------
 
-    def _wrap_stream(self):
+    def _get_wrapped_wsgi_input(self):
         try:
             content_length = self.content_length or 0
 
@@ -1137,18 +1201,14 @@ class Request(object):
             # but it had an invalid value. Assume no content.
             content_length = 0
 
-        self.stream = helpers.Body(self.stream, content_length)
+        return helpers.BoundedStream(self.env['wsgi.input'], content_length)
 
     def _parse_form_urlencoded(self):
-        # NOTE(kgriffs): This assumes self.stream has been patched
+        content_length = self.content_length
-        # above in the case of wsgiref, so that self.content_length
+        if not content_length:
-        # is not needed. Normally we just avoid accessing
+            return
-        # self.content_length, because it is a little expensive
+
-        # to call. We could cache self.content_length, but the
+        body = self.stream.read(content_length)
-        # overhead to do that won't usually be helpful, since
-        # content length will only ever be read once per
-        # request in most cases.
-        body = self.stream.read()
 
         # NOTE(kgriffs): According to http://goo.gl/6rlcux the
         # body should be US-ASCII. Enforcing this also helps
@@ -1201,7 +1261,10 @@ class Request(object):
 
 # PERF: To avoid typos and improve storage space and speed over a dict.
 class RequestOptions(object):
-    """This class is a container for ``Request`` options.
+    """Defines a set of configurable request options.
+
+    An instance of this class is exposed via :any:`API.req_options` for
+    configuring certain :py:class:`~.Request` behaviors.
 
     Attributes:
         keep_blank_qs_values (bool): Set to ``True`` to keep query string

falcon/request_helpers.py

@@ -36,7 +36,7 @@ def header_property(wsgi_name):
     return property(fget)
 
 
-class Body(object):
+class BoundedStream(object):
     """Wrap *wsgi.input* streams to make them more robust.
 
     ``socket._fileobject`` and ``io.BufferedReader`` are sometimes used
@@ -74,11 +74,11 @@ class Body(object):
         """Helper function for proxing reads to the underlying stream.
 
         Args:
-            size (int): Maximum number of bytes/characters to read.
+            size (int): Maximum number of bytes to read. Will be
-                Will be coerced, if None or -1, to `self.stream_len`. Will
+                coerced, if None or -1, to the number of remaining bytes
-                likewise be coerced if greater than `self.stream_len`, so
+                in the stream. Will likewise be coerced if greater than
-                that if the stream doesn't follow standard io semantics,
+                the number of remaining bytes, to avoid making a
-                the read won't block.
+                blocking call to the wrapped stream.
             target (callable): Once `size` has been fixed up, this function
                 will be called to actually do the work.
 
@@ -137,3 +137,7 @@ class Body(object):
         """
 
         return self._read(hint, self.stream.readlines)
+
+
+# NOTE(kgriffs): Alias for backwards-compat
+Body = BoundedStream

falcon/testing/base.py

@@ -27,7 +27,10 @@ from falcon.testing.helpers import create_environ
 from falcon.testing.srmock import StartResponseMock
 
 
-class TestBase(unittest.TestCase):
+# NOTE(kgriffs): Since this class is deprecated and we will be using it
+# less and less for Falcon's own tests, coverage may be reduced, hence
+# the pragma to ignore coverage errors from now on.
+class TestBase(unittest.TestCase):  # pragma nocover
     """Extends :py:mod:`unittest` to support WSGI functional testing.
 
     Warning:

falcon/testing/resource.py

@@ -66,9 +66,10 @@ class SimpleTestResource(object):
     as needed to test middleware, hooks, and the Falcon framework
     itself.
 
-    Only the ``on_get()`` responder is implemented; when adding
+    Only noop ``on_get()`` and ``on_post()`` responders are implemented;
-    additional responders in child classes, they can be decorated
+    when overriding these, or adding additional responders in child
-    with the :py:meth:`falcon.testing.capture_responder_args` hook in
+    classes, they can be decorated with the
+    :py:meth:`falcon.testing.capture_responder_args` hook in
     order to capture the *req*, *resp*, and *params* arguments that
     are passed to the responder. Responders may also be decorated with
     the :py:meth:`falcon.testing.set_resp_defaults` hook in order to
@@ -108,11 +109,20 @@ class SimpleTestResource(object):
         else:
             self._default_body = body
 
+        self.captured_req = None
+        self.captured_resp = None
+        self.captured_kwargs = None
+
     @falcon.before(capture_responder_args)
     @falcon.before(set_resp_defaults)
     def on_get(self, req, resp, **kwargs):
         pass
 
+    @falcon.before(capture_responder_args)
+    @falcon.before(set_resp_defaults)
+    def on_post(self, req, resp, **kwargs):
+        pass
+
 
 class TestResource(object):
     """Mock resource for functional testing.

tests/conftest.py

@@ -0,0 +1,12 @@
+import pytest
+
+import falcon
+
+
+# NOTE(kgriffs): Some modules actually run a wsgiref server, so
+# to ensure we reset the detection for the other modules, we just
+# run this fixture before each one is tested.
+@pytest.fixture(autouse=True, scope='module')
+def reset_request_stream_detection():
+    falcon.Request._wsgi_input_type_known = False
+    falcon.Request._always_wrap_wsgi_input = False

tests/test_query_params.py

@@ -12,14 +12,14 @@ import falcon.testing as testing
 class _TestQueryParams(testing.TestBase):
 
     def before(self):
-        self.resource = testing.TestResource()
+        self.resource = testing.SimpleTestResource()
         self.api.add_route('/', self.resource)
 
     def test_none(self):
         query_string = ''
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         store = {}
         self.assertIs(req.get_param('marker'), None)
         self.assertIs(req.get_param('limit', store), None)
@@ -32,7 +32,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'marker='
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertIs(req.get_param('marker'), None)
 
         store = {}
@@ -43,7 +43,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'marker=deadbeef&limit=25'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         store = {}
         self.assertEqual(req.get_param('marker', store=store) or 'nada',
                          'deadbeef')
@@ -56,7 +56,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'id=23,42&q=%e8%b1%86+%e7%93%a3'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         # NOTE(kgriffs): For lists, get_param will return one of the
         # elements, but which one it will choose is undefined.
@@ -71,7 +71,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'id=23,42,,&id=2'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         self.assertEqual(req.params['id'], [u'23,42,,', u'2'])
         self.assertIn(req.get_param('id'), [u'23,42,,', u'2'])
@@ -83,7 +83,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'id=23,42,,&id=2'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         self.assertEqual(req.params['id'], [u'23', u'42', u'2'])
         self.assertIn(req.get_param('id'), [u'23', u'42', u'2'])
@@ -102,7 +102,7 @@ class _TestQueryParams(testing.TestBase):
 
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         self.assertIn(req.get_param('colors'), 'red,green,blue')
         self.assertEqual(req.get_param_as_list('colors'), [u'red,green,blue'])
@@ -124,7 +124,7 @@ class _TestQueryParams(testing.TestBase):
         self.simulate_request('/', query_string=query_string)
         self.assertEqual(self.srmock.status, falcon.HTTP_200)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param('x'), '% % %')
         self.assertEqual(req.get_param('y'), 'peregrine')
         self.assertEqual(req.get_param('z'), '%a%z%zz%1 e')
@@ -135,7 +135,7 @@ class _TestQueryParams(testing.TestBase):
                         '_thing=42&_charset_=utf-8')
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param('p'), '0')
         self.assertEqual(req.get_param('p1'), '23')
         self.assertEqual(req.get_param('2p'), 'foo')
@@ -153,7 +153,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = ''
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         try:
             getattr(req, method_name)('marker', required=True)
@@ -168,7 +168,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'marker=deadbeef&limit=25'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         try:
             req.get_param_as_int('marker')
@@ -230,7 +230,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'marker=deadbeef&pos=-7'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_int('pos'), -7)
 
         self.assertEqual(
@@ -260,7 +260,7 @@ class _TestQueryParams(testing.TestBase):
                         't1=True&f1=False&t2=yes&f2=no&blank&one=1&zero=0')
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertRaises(falcon.HTTPBadRequest, req.get_param_as_bool,
                           'bogus')
 
@@ -296,7 +296,7 @@ class _TestQueryParams(testing.TestBase):
             query_string='blank&blank2=',
         )
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param('blank'), '')
         self.assertEqual(req.get_param('blank2'), '')
         self.assertRaises(falcon.HTTPInvalidParam, req.get_param_as_bool,
@@ -316,7 +316,7 @@ class _TestQueryParams(testing.TestBase):
                         '&thing_two=1&thing_two=&thing_two=3')
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         # NOTE(kgriffs): For lists, get_param will return one of the
         # elements, but which one it will choose is undefined.
@@ -366,7 +366,7 @@ class _TestQueryParams(testing.TestBase):
             query_string=query_string
         )
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         # NOTE(kgriffs): For lists, get_param will return one of the
         # elements, but which one it will choose is undefined.
@@ -412,7 +412,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'coord=1.4,13,15.1&limit=100&things=4,,1'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
 
         # NOTE(kgriffs): For lists, get_param will return one of the
         # elements, but which one it will choose is undefined.
@@ -443,7 +443,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'ant=4&bee=3&cat=2&dog=1'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(
             sorted(req.params.items()),
             [('ant', '4'), ('bee', '3'), ('cat', '2'), ('dog', '1')])
@@ -452,7 +452,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'ant=1&ant=2&bee=3&cat=6&cat=5&cat=4'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         # By definition, we cannot guarantee which of the multiple keys will
         # be returned by .get_param().
         self.assertIn(req.get_param('ant'), ('1', '2'))
@@ -464,20 +464,20 @@ class _TestQueryParams(testing.TestBase):
     def test_multiple_keys_as_bool(self):
         query_string = 'ant=true&ant=yes&ant=True'
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_bool('ant'), True)
 
     def test_multiple_keys_as_int(self):
         query_string = 'ant=1&ant=2&ant=3'
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertIn(req.get_param_as_int('ant'), (1, 2, 3))
 
     def test_multiple_form_keys_as_list(self):
         query_string = 'ant=1&ant=2&bee=3&cat=6&cat=5&cat=4'
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         # There are two 'ant' keys.
         self.assertEqual(req.get_param_as_list('ant'), ['1', '2'])
         # There is only one 'bee' key..
@@ -489,14 +489,14 @@ class _TestQueryParams(testing.TestBase):
         date_value = '2015-04-20'
         query_string = 'thedate={0}'.format(date_value)
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_date('thedate'),
                          date(2015, 4, 20))
 
     def test_get_date_missing_param(self):
         query_string = 'notthedate=2015-04-20'
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_date('thedate'),
                          None)
 
@@ -505,7 +505,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'thedate={0}'.format(date_value)
         format_string = '%Y%m%d'
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_date('thedate',
                          format_string=format_string),
                          date(2015, 4, 20))
@@ -514,7 +514,7 @@ class _TestQueryParams(testing.TestBase):
         date_value = '2015-04-20'
         query_string = 'thedate={0}'.format(date_value)
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         store = {}
         req.get_param_as_date('thedate', store=store)
         self.assertNotEqual(len(store), 0)
@@ -524,7 +524,7 @@ class _TestQueryParams(testing.TestBase):
         query_string = 'thedate={0}'.format(date_value)
         format_string = '%Y%m%d'
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertRaises(HTTPInvalidParam, req.get_param_as_date,
                           'thedate', format_string=format_string)
 
@@ -532,7 +532,7 @@ class _TestQueryParams(testing.TestBase):
         payload_dict = {'foo': 'bar'}
         query_string = 'payload={0}'.format(json.dumps(payload_dict))
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_dict('payload'),
                          payload_dict)
 
@@ -540,7 +540,7 @@ class _TestQueryParams(testing.TestBase):
         payload_dict = {'foo': 'bar'}
         query_string = 'notthepayload={0}'.format(json.dumps(payload_dict))
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertEqual(req.get_param_as_dict('payload'),
                          None)
 
@@ -548,7 +548,7 @@ class _TestQueryParams(testing.TestBase):
         payload_dict = {'foo': 'bar'}
         query_string = 'payload={0}'.format(json.dumps(payload_dict))
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         store = {}
         req.get_param_as_dict('payload', store=store)
         self.assertNotEqual(len(store), 0)
@@ -557,7 +557,7 @@ class _TestQueryParams(testing.TestBase):
         payload_dict = 'foobar'
         query_string = 'payload={0}'.format(payload_dict)
         self.simulate_request('/', query_string=query_string)
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertRaises(HTTPInvalidParam, req.get_param_as_dict,
                           'payload')
 
@@ -568,23 +568,42 @@ class PostQueryParams(_TestQueryParams):
         self.api.req_options.auto_parse_form_urlencoded = True
 
     def simulate_request(self, path, query_string, **kwargs):
-        headers = {'Content-Type': 'application/x-www-form-urlencoded'}
+
+        headers = kwargs.setdefault('headers', {})
+        headers['Content-Type'] = 'application/x-www-form-urlencoded'
+
         super(PostQueryParams, self).simulate_request(
-            path, body=query_string, headers=headers, **kwargs)
+            path,
+            method='POST',
+            body=query_string,
+            **kwargs
+        )
 
     def test_non_ascii(self):
         value = u'\u8c46\u74e3'
         query_string = b'q=' + value.encode('utf-8')
         self.simulate_request('/', query_string=query_string)
 
-        req = self.resource.req
+        req = self.resource.captured_req
+        self.assertIs(req.get_param('q'), None)
+
+    def test_empty_body(self):
+        self.simulate_request('/', query_string=None)
+
+        req = self.resource.captured_req
+        self.assertIs(req.get_param('q'), None)
+
+    def test_empty_body_no_content_length(self):
+        self.simulate_request('/', query_string=None)
+
+        req = self.resource.captured_req
         self.assertIs(req.get_param('q'), None)
 
     def test_explicitly_disable_auto_parse(self):
         self.api.req_options.auto_parse_form_urlencoded = False
         self.simulate_request('/', query_string='q=42')
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertIs(req.get_param('q'), None)
 
 
@@ -596,11 +615,11 @@ class GetQueryParams(_TestQueryParams):
 
 class PostQueryParamsDefaultBehavior(testing.TestBase):
     def test_dont_auto_parse_by_default(self):
-        self.resource = testing.TestResource()
+        self.resource = testing.SimpleTestResource()
         self.api.add_route('/', self.resource)
 
         headers = {'Content-Type': 'application/x-www-form-urlencoded'}
         self.simulate_request('/', body='q=42', headers=headers)
 
-        req = self.resource.req
+        req = self.resource.captured_req
         self.assertIs(req.get_param('q'), None)

tests/test_request_body.py

@@ -1,8 +1,4 @@
 import io
-import threading
-from wsgiref import simple_server
-
-import requests
 
 import falcon
 from falcon import request_helpers
@@ -64,42 +60,21 @@ class TestRequestBody(testing.TestBase):
 
         self.assertEqual(stream.tell(), expected_len)
 
-    def test_read_socket_body(self):
+    def test_bounded_stream_property_empty_body(self):
-        expected_body = testing.rand_string(SIZE_1_KB / 2, SIZE_1_KB)
+        """Test that we can get a bounded stream outside of wsgiref."""
 
-        def server():
+        environ = testing.create_environ()
-            class Echo(object):
+        req = falcon.Request(environ)
-                def on_post(self, req, resp):
-                    # wsgiref socket._fileobject blocks when len not given,
-                    # but Falcon is smarter than that. :D
-                    body = req.stream.read()
-                    resp.body = body
 
-                def on_put(self, req, resp):
+        bounded_stream = req.bounded_stream
-                    # wsgiref socket._fileobject blocks when len too long,
-                    # but Falcon should work around that for me.
-                    body = req.stream.read(req.content_length + 1)
-                    resp.body = body
 
-            api = falcon.API()
+        # NOTE(kgriffs): Verify that we aren't creating a new object
-            api.add_route('/echo', Echo())
+        # each time the property is called. Also ensures branch
+        # coverage of the property implementation.
+        assert bounded_stream is req.bounded_stream
 
-            httpd = simple_server.make_server('127.0.0.1', 8989, api)
+        data = bounded_stream.read()
-            httpd.serve_forever()
+        self.assertEqual(len(data), 0)
-
-        thread = threading.Thread(target=server)
-        thread.daemon = True
-        thread.start()
-
-        # Let it boot
-        thread.join(1)
-
-        url = 'http://127.0.0.1:8989/echo'
-        resp = requests.post(url, data=expected_body)
-        self.assertEqual(resp.text, expected_body)
-
-        resp = requests.put(url, data=expected_body)
-        self.assertEqual(resp.text, expected_body)
 
     def test_body_stream_wrapper(self):
         data = testing.rand_string(SIZE_1_KB / 2, SIZE_1_KB)

tests/test_wsgi.py

@@ -7,25 +7,70 @@ try:
 except ImportError:
     pass  # Jython
 
+import pytest
 import requests
-from testtools.matchers import Equals, MatchesRegex
 
 import falcon
+from falcon.request_helpers import BoundedStream
 import falcon.testing as testing
 
 _SERVER_HOST = 'localhost'
 _SERVER_PORT = 9809
 _SERVER_BASE_URL = 'http://{0}:{1}/'.format(_SERVER_HOST, _SERVER_PORT)
+_SIZE_1_KB = 1024
 
 
-def _is_iterable(thing):
+@pytest.mark.skipif(
-    try:
+    # NOTE(kgriffs): Jython does not support the multiprocessing
-        for i in thing:
+    # module. We could alternatively implement these tests
-            break
+    # using threads, but then we have to force a garbage
+    # collection in between each test in order to make
+    # the server relinquish its socket, and the gc module
+    # doesn't appear to do anything under Jython.
 
-        return True
+    'java' in sys.platform,
-    except:
+    reason='Incompatible with Jython'
-        return False
+)
+@pytest.mark.usefixtures('_setup_wsgi_server')
+class TestWSGIServer(object):
+
+    def test_get(self):
+        resp = requests.get(_SERVER_BASE_URL)
+        assert resp.status_code == 200
+        assert resp.text == '127.0.0.1'
+
+    def test_put(self):
+        body = '{}'
+        resp = requests.put(_SERVER_BASE_URL, data=body)
+        assert resp.status_code == 200
+        assert resp.text == '{}'
+
+    def test_head_405(self):
+        body = '{}'
+        resp = requests.head(_SERVER_BASE_URL, data=body)
+        assert resp.status_code == 405
+
+    def test_post(self):
+        body = testing.rand_string(_SIZE_1_KB / 2, _SIZE_1_KB)
+        resp = requests.post(_SERVER_BASE_URL, data=body)
+        assert resp.status_code == 200
+        assert resp.text == body
+
+    def test_post_invalid_content_length(self):
+        headers = {'Content-Length': 'invalid'}
+        resp = requests.post(_SERVER_BASE_URL, headers=headers)
+        assert resp.status_code == 200
+        assert resp.text == ''
+
+    def test_post_read_bounded_stream(self):
+        body = testing.rand_string(_SIZE_1_KB / 2, _SIZE_1_KB)
+        resp = requests.post(_SERVER_BASE_URL + 'bucket', data=body)
+        assert resp.status_code == 200
+        assert resp.text == body
+
+    def test_post_read_bounded_stream_no_body(self):
+        resp = requests.post(_SERVER_BASE_URL + 'bucket')
+        assert not resp.text
 
 
 def _run_server(stop_event):
@@ -34,7 +79,7 @@ def _run_server(stop_event):
             resp.body = req.remote_addr
 
         def on_post(self, req, resp):
-            resp.body = req.stream.read(1000)
+            resp.body = req.stream.read(_SIZE_1_KB)
 
         def on_put(self, req, resp):
             # NOTE(kgriffs): Test that reading past the end does
@@ -44,8 +89,36 @@ def _run_server(stop_event):
 
             resp.body = b''.join(req_body)
 
+    class Bucket(object):
+        def on_post(self, req, resp):
+            # NOTE(kgriffs): The framework automatically detects
+            # wsgiref's input object type and wraps it; we'll probably
+            # do away with this at some point, but for now we
+            # verify the functionality,
+            assert isinstance(req.stream, BoundedStream)
+
+            # NOTE(kgriffs): Ensure we are reusing the same object for
+            # the sake of efficiency and to ensure a shared state of the
+            # stream. (only in the case that we have decided to
+            # automatically wrap the WSGI input object, i.e. when
+            # running under wsgiref or similar).
+            assert req.stream is req.bounded_stream
+
+            # NOTE(kgriffs): This would normally block when
+            # Content-Length is 0 and the WSGI input object.
+            # BoundedStream fixes that. This is just a sanity check to
+            # make sure req.bounded_stream is what we think it is;
+            # BoundedStream itself has its own unit tests in
+            # test_request_body.py
+            resp.body = req.bounded_stream.read()
+
+            # NOTE(kgriffs): No need to also test the same read() for
+            # req.stream, since we already asserted they are the same
+            # objects.
+
     api = application = falcon.API()
     api.add_route('/', Things())
+    api.add_route('/bucket', Bucket())
 
     server = make_server(_SERVER_HOST, _SERVER_PORT, application)
 
@@ -53,103 +126,29 @@ def _run_server(stop_event):
         server.handle_request()
 
 
-class TestWSGIInterface(testing.TestBase):
+@pytest.fixture
+def _setup_wsgi_server():
+    stop_event = multiprocessing.Event()
+    process = multiprocessing.Process(
+        target=_run_server,
+        args=(stop_event,)
+    )
 
-    def test_srmock(self):
+    process.start()
-        mock = testing.StartResponseMock()
-        mock(falcon.HTTP_200, ())
 
-        self.assertEqual(falcon.HTTP_200, mock.status)
+    # NOTE(kgriffs): Let the server start up
-        self.assertEqual(None, mock.exc_info)
+    time.sleep(0.2)
 
-        mock = testing.StartResponseMock()
+    yield
-        exc_info = sys.exc_info()
-        mock(falcon.HTTP_200, (), exc_info)
 
-        self.assertEqual(exc_info, mock.exc_info)
+    stop_event.set()
 
-    def test_pep3333(self):
+    # NOTE(kgriffs): Pump the request handler loop in case execution
-        api = falcon.API()
+    # made it to the next server.handle_request() before we sent the
-        mock = testing.StartResponseMock()
+    # event.
+    try:
+        requests.get(_SERVER_BASE_URL)
+    except Exception:
+        pass  # Thread already exited
 
-        # Simulate a web request (normally done though a WSGI server)
+    process.join()
-        response = api(testing.create_environ(), mock)
-
-        # Verify that the response is iterable
-        self.assertTrue(_is_iterable(response))
-
-        # Make sure start_response was passed a valid status string
-        self.assertIs(mock.call_count, 1)
-        self.assertTrue(isinstance(mock.status, str))
-        self.assertThat(mock.status, MatchesRegex('^\d+[a-zA-Z\s]+$'))
-
-        # Verify headers is a list of tuples, each containing a pair of strings
-        self.assertTrue(isinstance(mock.headers, list))
-        if len(mock.headers) != 0:
-            header = mock.headers[0]
-            self.assertTrue(isinstance(header, tuple))
-            self.assertThat(len(header), Equals(2))
-            self.assertTrue(isinstance(header[0], str))
-            self.assertTrue(isinstance(header[1], str))
-
-
-class TestWSGIReference(testing.TestBase):
-
-    def before(self):
-        if 'java' in sys.platform:
-            # NOTE(kgriffs): Jython does not support the multiprocessing
-            # module. We could alternatively implement these tests
-            # using threads, but then we have to force a garbage
-            # collection in between each test in order to make
-            # the server relinquish its socket, and the gc module
-            # doesn't appear to do anything under Jython.
-            self.skip('Incompatible with Jython')
-
-        self._stop_event = multiprocessing.Event()
-        self._process = multiprocessing.Process(target=_run_server,
-                                                args=(self._stop_event,))
-        self._process.start()
-
-        # NOTE(kgriffs): Let the server start up
-        time.sleep(0.2)
-
-    def after(self):
-        self._stop_event.set()
-
-        # NOTE(kgriffs): Pump the request handler loop in case execution
-        # made it to the next server.handle_request() before we sent the
-        # event.
-        try:
-            requests.get(_SERVER_BASE_URL)
-        except Exception:
-            pass  # Thread already exited
-
-        self._process.join()
-
-    def test_wsgiref_get(self):
-        resp = requests.get(_SERVER_BASE_URL)
-        self.assertEqual(resp.status_code, 200)
-        self.assertEqual(resp.text, '127.0.0.1')
-
-    def test_wsgiref_put(self):
-        body = '{}'
-        resp = requests.put(_SERVER_BASE_URL, data=body)
-        self.assertEqual(resp.status_code, 200)
-        self.assertEqual(resp.text, '{}')
-
-    def test_wsgiref_head_405(self):
-        body = '{}'
-        resp = requests.head(_SERVER_BASE_URL, data=body)
-        self.assertEqual(resp.status_code, 405)
-
-    def test_wsgiref_post(self):
-        body = '{}'
-        resp = requests.post(_SERVER_BASE_URL, data=body)
-        self.assertEqual(resp.status_code, 200)
-        self.assertEqual(resp.text, '{}')
-
-    def test_wsgiref_post_invalid_content_length(self):
-        headers = {'Content-Length': 'invalid'}
-        resp = requests.post(_SERVER_BASE_URL, headers=headers)
-        self.assertEqual(resp.status_code, 200)
-        self.assertEqual(resp.text, '')

tests/test_wsgi_interface.py

@@ -0,0 +1,55 @@
+import re
+import sys
+
+import falcon
+import falcon.testing as testing
+
+
+class TestWSGIInterface(object):
+
+    def test_srmock(self):
+        mock = testing.StartResponseMock()
+        mock(falcon.HTTP_200, ())
+
+        assert mock.status == falcon.HTTP_200
+        assert mock.exc_info is None
+
+        mock = testing.StartResponseMock()
+        exc_info = sys.exc_info()
+        mock(falcon.HTTP_200, (), exc_info)
+
+        assert mock.exc_info == exc_info
+
+    def test_pep3333(self):
+        api = falcon.API()
+        mock = testing.StartResponseMock()
+
+        # Simulate a web request (normally done though a WSGI server)
+        response = api(testing.create_environ(), mock)
+
+        # Verify that the response is iterable
+        assert _is_iterable(response)
+
+        # Make sure start_response was passed a valid status string
+        assert mock.call_count == 1
+        assert isinstance(mock.status, str)
+        assert re.match('^\d+[a-zA-Z\s]+$', mock.status)
+
+        # Verify headers is a list of tuples, each containing a pair of strings
+        assert isinstance(mock.headers, list)
+        if len(mock.headers) != 0:
+            header = mock.headers[0]
+            assert isinstance(header, tuple)
+            assert len(header) == 2
+            assert isinstance(header[0], str)
+            assert isinstance(header[1], str)
+
+
+def _is_iterable(thing):
+    try:
+        for i in thing:
+            break
+
+        return True
+    except:
+        return False