Merge 6226f070f0 into 2fa04caf7c

docs/api-guide/requests.md

@@ -130,6 +130,9 @@ As REST framework's `Request` extends Django's `HttpRequest`, all the other stan
 
 Note that due to implementation reasons the `Request` class does not inherit from `HttpRequest` class, but instead extends the class using composition.
 
+# Accessing the HttpRequest
+
+The underlying `HttpRequest` can be accessed through the `.http_request` attribute. While direct manipulation of the `HttpRequest` is discouraged, there are some advanced use cases that may require it. For example, one view may delegate request handling to a secondary view function. In this case, it is necessary to pass the original `HttpRequest` to the delegated view instead of the DRF `Request` object. Be aware that duplicate processing of the `HttpRequest` may have have unintended side effects. For example, if the request stream has already been consumed, it may not be accessible for a second read and will raise an exception.
 
 [cite]: https://groups.google.com/d/topic/django-developers/dxI4qVzrBY4/discussion
 [parsers documentation]: parsers.md

rest_framework/authentication.py

@@ -121,7 +121,7 @@ class SessionAuthentication(BaseAuthentication):
         """
 
         # Get the session-based user from the underlying HttpRequest object
-        user = getattr(request._request, 'user', None)
+        user = getattr(request.http_request, 'user', None)
 
         # Unauthenticated, CSRF validation not required
         if not user or not user.is_active:

rest_framework/request.py

@@ -11,6 +11,7 @@ The wrapped request then offers a richer API, in particular :
 from __future__ import unicode_literals
 
 import sys
+import warnings
 from contextlib import contextmanager
 
 from django.conf import settings
@@ -97,7 +98,7 @@ def clone_request(request, method):
     Internal helper method to clone a request, replacing with a different
     HTTP method.  Used for checking permissions against other methods.
     """
-    ret = Request(request=request._request,
+    ret = Request(request=request.http_request,
                   parsers=request.parsers,
                   authenticators=request.authenticators,
                   negotiator=request.negotiator,
@@ -159,7 +160,7 @@ class Request(object):
             .format(request.__class__.__module__, request.__class__.__name__)
         )
 
-        self._request = request
+        self.http_request = request
         self.parsers = parsers or ()
         self.authenticators = authenticators or ()
         self.negotiator = negotiator or self._default_negotiator()
@@ -181,12 +182,28 @@ class Request(object):
             forced_auth = ForcedAuthentication(force_user, force_token)
             self.authenticators = (forced_auth,)
 
+    @property
+    def _request(self):
+        warnings.warn(
+            "`_request` has been deprecated in favor of `http_request`, and will be removed in 3.10",
+            PendingDeprecationWarning, stacklevel=2
+        )
+        return self.http_request
+
+    @_request.setter
+    def _request(self, value):
+        warnings.warn(
+            "`_request` has been deprecated in favor of `http_request`, and will be removed in 3.10",
+            PendingDeprecationWarning, stacklevel=2
+        )
+        self.http_request = value
+
     def _default_negotiator(self):
         return api_settings.DEFAULT_CONTENT_NEGOTIATION_CLASS()
 
     @property
     def content_type(self):
-        meta = self._request.META
+        meta = self.http_request.META
         return meta.get('CONTENT_TYPE', meta.get('HTTP_CONTENT_TYPE', ''))
 
     @property
@@ -203,7 +220,7 @@ class Request(object):
         """
         More semantically correct name for request.GET.
         """
-        return self._request.GET
+        return self.http_request.GET
 
     @property
     def data(self):
@@ -233,7 +250,7 @@ class Request(object):
         instance, ensuring that it is available to any middleware in the stack.
         """
         self._user = value
-        self._request.user = value
+        self.http_request.user = value
 
     @property
     def auth(self):
@@ -253,7 +270,7 @@ class Request(object):
         request, such as an authentication token.
         """
         self._auth = value
-        self._request.auth = value
+        self.http_request.auth = value
 
     @property
     def successful_authenticator(self):
@@ -278,16 +295,17 @@ class Request(object):
             else:
                 self._full_data = self._data
 
-            # copy data & files refs to the underlying request so that closable
+            # if a form media type, copy data & files refs to the underlying
-            # objects are handled appropriately.
+            # http request so that closable objects are handled appropriately.
-            self._request._post = self.POST
+            if is_form_media_type(self.content_type):
-            self._request._files = self.FILES
+                self.http_request._post = self.POST
+                self.http_request._files = self.FILES
 
     def _load_stream(self):
         """
         Return the content body of the request, as a stream.
         """
-        meta = self._request.META
+        meta = self.http_request.META
         try:
             content_length = int(
                 meta.get('CONTENT_LENGTH', meta.get('HTTP_CONTENT_LENGTH', 0))
@@ -297,8 +315,8 @@ class Request(object):
 
         if content_length == 0:
             self._stream = None
-        elif not self._request._read_started:
+        elif not self.http_request._read_started:
-            self._stream = self._request
+            self._stream = self.http_request
         else:
             self._stream = six.BytesIO(self.body)
 
@@ -322,18 +340,18 @@ class Request(object):
         try:
             stream = self.stream
         except RawPostDataException:
-            if not hasattr(self._request, '_post'):
+            if not hasattr(self.http_request, '_post'):
                 raise
             # If request.POST has been accessed in middleware, and a method='POST'
             # request was made with 'multipart/form-data', then the request stream
             # will already have been exhausted.
             if self._supports_form_parsing():
-                return (self._request.POST, self._request.FILES)
+                return (self.http_request.POST, self.http_request.FILES)
             stream = None
 
         if stream is None or media_type is None:
             if media_type and is_form_media_type(media_type):
-                empty_data = QueryDict('', encoding=self._request._encoding)
+                empty_data = QueryDict('', encoding=self.http_request._encoding)
             else:
                 empty_data = {}
             empty_files = MultiValueDict()
@@ -351,7 +369,7 @@ class Request(object):
             # re-raise.  Ensures we don't simply repeat the error when
             # attempting to render the browsable renderer response, or when
             # logging the request or similar.
-            self._data = QueryDict('', encoding=self._request._encoding)
+            self._data = QueryDict('', encoding=self.http_request._encoding)
             self._files = MultiValueDict()
             self._full_data = self._data
             raise
@@ -407,7 +425,7 @@ class Request(object):
         to proxy it to the underlying HttpRequest object.
         """
         try:
-            return getattr(self._request, attr)
+            return getattr(self.http_request, attr)
         except AttributeError:
             return self.__getattribute__(attr)
 
@@ -425,7 +443,7 @@ class Request(object):
             self._load_data_and_files()
         if is_form_media_type(self.content_type):
             return self._data
-        return QueryDict('', encoding=self._request._encoding)
+        return QueryDict('', encoding=self.http_request._encoding)
 
     @property
     def FILES(self):
@@ -446,4 +464,4 @@ class Request(object):
     def force_plaintext_errors(self, value):
         # Hack to allow our exception handler to force choice of
         # plaintext or html error responses.
-        self._request.is_ajax = lambda: value
+        self.http_request.is_ajax = lambda: value

tests/test_request.py

@@ -13,6 +13,7 @@ from django.contrib.auth.middleware import AuthenticationMiddleware
 from django.contrib.auth.models import User
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.http.request import RawPostDataException
 from django.test import TestCase, override_settings
 from django.utils import six
 
@@ -137,6 +138,11 @@ class MockView(APIView):
         return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
 
+class EchoView(APIView):
+    def post(self, request):
+        return Response(status=status.HTTP_200_OK, data=request.data)
+
+
 class FileUploadView(APIView):
     def post(self, request):
         filenames = [file.temporary_file_path() for file in request.FILES.values()]
@@ -149,6 +155,7 @@ class FileUploadView(APIView):
 
 urlpatterns = [
     url(r'^$', MockView.as_view()),
+    url(r'^echo/$', EchoView.as_view()),
     url(r'^upload/$', FileUploadView.as_view())
 ]
 
@@ -271,24 +278,83 @@ class TestSecure(TestCase):
         assert request.scheme == 'https'
 
 
-class TestWSGIRequestProxy(TestCase):
+class TestHttpRequest(TestCase):
-    def test_attribute_access(self):
+    def test_attribute_access_proxy(self):
-        wsgi_request = factory.get('/')
+        http_request = factory.get('/')
-        request = Request(wsgi_request)
+        request = Request(http_request)
 
         inner_sentinel = object()
-        wsgi_request.inner_property = inner_sentinel
+        http_request.inner_property = inner_sentinel
         assert request.inner_property is inner_sentinel
 
         outer_sentinel = object()
         request.inner_property = outer_sentinel
         assert request.inner_property is outer_sentinel
 
-    def test_exception(self):
+    def test_exception_proxy(self):
         # ensure the exception message is not for the underlying WSGIRequest
-        wsgi_request = factory.get('/')
+        http_request = factory.get('/')
-        request = Request(wsgi_request)
+        request = Request(http_request)
 
         message = "'Request' object has no attribute 'inner_property'"
         with self.assertRaisesMessage(AttributeError, message):
             request.inner_property
+
+    def test_request_deprecation(self):
+        with pytest.warns(PendingDeprecationWarning) as record:
+            Request(factory.get('/'))._request
+
+        assert len(record) == 1
+        assert str(record[0].message) == (
+            "`_request` has been deprecated in favor of "
+            "`http_request`, and will be removed in 3.10"
+        )
+
+        with pytest.warns(PendingDeprecationWarning) as record:
+            Request(factory.get('/'))._request = None
+
+        assert len(record) == 1
+        assert str(record[0].message) == (
+            "`_request` has been deprecated in favor of "
+            "`http_request`, and will be removed in 3.10"
+        )
+
+    @override_settings(ROOT_URLCONF='tests.test_request')
+    def test_duplicate_request_stream_parsing_exception(self):
+        """
+        Check assumption that duplicate stream parsing will result in a
+        `RawPostDataException` being raised.
+        """
+        response = APIClient().post('/echo/', data={'a': 'b'}, format='json')
+        request = response.renderer_context['request']
+
+        # ensure that request stream was consumed by json parser
+        assert request.content_type.startswith('application/json')
+        assert response.data == {'a': 'b'}
+
+        # pass same HttpRequest to view, stream already consumed
+        with pytest.raises(RawPostDataException):
+            EchoView.as_view()(request.http_request)
+
+    @override_settings(ROOT_URLCONF='tests.test_request')
+    def test_duplicate_request_form_data_access(self):
+        """
+        Form data is copied to the underlying django request for middleware
+        and file closing reasons. Duplicate processing of a request with form
+        data is 'safe' in so far as accessing `request.POST` does not trigger
+        the duplicate stream parse exception.
+        """
+        response = APIClient().post('/echo/', data={'a': 'b'})
+        request = response.renderer_context['request']
+
+        # ensure that request stream was consumed by form parser
+        assert request.content_type.startswith('multipart/form-data')
+        assert response.data == {'a': ['b']}
+
+        # pass same HttpRequest to view, form data set on underlying request
+        response = EchoView.as_view()(request.http_request)
+        request = response.renderer_context['request']
+
+        # ensure that request stream was consumed by form parser
+        assert request.content_type.startswith('multipart/form-data')
+        assert response.data == {'a': ['b']}