diff --git a/docs/api-guide/requests.md b/docs/api-guide/requests.md index 83a38d1c3..5127edee6 100644 --- a/docs/api-guide/requests.md +++ b/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 diff --git a/rest_framework/authentication.py b/rest_framework/authentication.py index 0749968bc..f3fff2793 100644 --- a/rest_framework/authentication.py +++ b/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: diff --git a/rest_framework/request.py b/rest_framework/request.py index 7e4daf274..2339712b7 100644 --- a/rest_framework/request.py +++ b/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 - # objects are handled appropriately. - self._request._post = self.POST - self._request._files = self.FILES + # if a form media type, copy data & files refs to the underlying + # http request so that closable objects are handled appropriately. + if is_form_media_type(self.content_type): + 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: - self._stream = self._request + elif not self.http_request._read_started: + 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 diff --git a/tests/test_request.py b/tests/test_request.py index 8c680baa0..9ccd7781e 100644 --- a/tests/test_request.py +++ b/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): - def test_attribute_access(self): - wsgi_request = factory.get('/') - request = Request(wsgi_request) +class TestHttpRequest(TestCase): + def test_attribute_access_proxy(self): + http_request = factory.get('/') + 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('/') - request = Request(wsgi_request) + http_request = factory.get('/') + 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']}