From c456b3c510870ed8e0117cb69abc2360a7aa0fca Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Mon, 5 Feb 2018 10:24:13 -0500 Subject: [PATCH] Fix request formdata handling (#5800) * Rename 'wsgi' request test to more accurate 'http' * Test duplicate request stream parsing * Fix setting post/files on the underlying request --- rest_framework/request.py | 9 +++--- tests/test_request.py | 63 ++++++++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/rest_framework/request.py b/rest_framework/request.py index 7e4daf274..9d4f73d30 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -278,10 +278,11 @@ 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._request._post = self.POST + self._request._files = self.FILES def _load_stream(self): """ diff --git a/tests/test_request.py b/tests/test_request.py index 8c680baa0..83d295a12 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,64 @@ 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 + + @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._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._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']}