Skip to content

Fix request formdata handling #5800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions rest_framework/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,11 @@ def _load_data_and_files(self):
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):
"""
Expand Down
63 changes: 55 additions & 8 deletions tests/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
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

Expand Down Expand Up @@ -137,6 +138,11 @@ def post(self, request):
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()]
Expand All @@ -149,6 +155,7 @@ def post(self, request):

urlpatterns = [
url(r'^$', MockView.as_view()),
url(r'^echo/$', EchoView.as_view()),
url(r'^upload/$', FileUploadView.as_view())
]

Expand Down Expand Up @@ -271,24 +278,64 @@ def test_default_secure_true(self):
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']}