From c1d9a96df03ea81454d7d0e3583c68e270ed5043 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 3 Dec 2013 08:58:05 +0000 Subject: [PATCH 1/4] Catch errors during parsing and set empty .DATA/.FILES before re-raising. --- rest_framework/request.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/rest_framework/request.py b/rest_framework/request.py index b883d0d4f..9b551aa80 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -356,7 +356,16 @@ class Request(object): if not parser: raise exceptions.UnsupportedMediaType(media_type) - parsed = parser.parse(stream, media_type, self.parser_context) + try: + parsed = parser.parse(stream, media_type, self.parser_context) + except: + # If we get an exception during parsing, fill in empty data and + # 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('', self._request._encoding) + self._files = MultiValueDict() + raise # Parser classes may return the raw data, or a # DataAndFiles object. Unpack the result as required. From 774298f145d18292b76f2bd90469e25c1950b1af Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 3 Dec 2013 16:18:35 +0000 Subject: [PATCH 2/4] First pass at a test for ParseErrors breaking the browsable API --- rest_framework/tests/test_renderers.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rest_framework/tests/test_renderers.py b/rest_framework/tests/test_renderers.py index 76299a890..549e763b1 100644 --- a/rest_framework/tests/test_renderers.py +++ b/rest_framework/tests/test_renderers.py @@ -88,6 +88,7 @@ urlpatterns = patterns('', url(r'^cache$', MockGETView.as_view()), url(r'^jsonp/jsonrenderer$', MockGETView.as_view(renderer_classes=[JSONRenderer, JSONPRenderer])), url(r'^jsonp/nojsonrenderer$', MockGETView.as_view(renderer_classes=[JSONPRenderer])), + url(r'^parseerror$', MockGETView.as_view(renderer_classes=[JSONRenderer, BrowsableAPIRenderer])), url(r'^html$', HTMLView.as_view()), url(r'^html1$', HTMLView1.as_view()), url(r'^api', include('rest_framework.urls', namespace='rest_framework')) @@ -219,6 +220,12 @@ class RendererEndToEndTests(TestCase): self.assertEqual(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) self.assertEqual(resp.status_code, DUMMYSTATUS) + def test_parse_error_renderers_browsable_api(self): + """Invalid data should still render the browsable API correctly.""" + resp = self.client.post('/parseerror', data='foobar', content_type='application/json', HTTP_ACCEPT='text/html') + self.assertEqual(resp['Content-Type'], 'text/html; charset=utf-8') + self.assertContains(resp.content, 'Mock GET View') + self.assertEqual(resp.status_code, status.HTTP_400_) _flat_repr = '{"foo": ["bar", "baz"]}' _indented_repr = '{\n "foo": [\n "bar",\n "baz"\n ]\n}' From 06d8a31e132c99a9645e26b5def3a1d9b9585c24 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 9 Dec 2013 07:34:08 +0000 Subject: [PATCH 3/4] Catch and mask ParseErrors that occur during rendering of the BrowsableAPI. --- rest_framework/renderers.py | 9 +++++++-- rest_framework/request.py | 2 +- rest_framework/tests/test_renderers.py | 12 +++++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index fe4f43d48..2fdd33376 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -20,6 +20,7 @@ from rest_framework.compat import StringIO from rest_framework.compat import six from rest_framework.compat import smart_text from rest_framework.compat import yaml +from rest_framework.exceptions import ParseError from rest_framework.settings import api_settings from rest_framework.request import is_form_media_type, override_method from rest_framework.utils import encoders @@ -420,8 +421,12 @@ class BrowsableAPIRenderer(BaseRenderer): In the absence of the View having an associated form then return None. """ if request.method == method: - data = request.DATA - files = request.FILES + try: + data = request.DATA + files = request.FILES + except ParseError: + data = None + files = None else: data = None files = None diff --git a/rest_framework/request.py b/rest_framework/request.py index 9b551aa80..fcea25083 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -362,7 +362,7 @@ class Request(object): # If we get an exception during parsing, fill in empty data and # 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. + # logging the request or similar. self._data = QueryDict('', self._request._encoding) self._files = MultiValueDict() raise diff --git a/rest_framework/tests/test_renderers.py b/rest_framework/tests/test_renderers.py index 549e763b1..10aa4248b 100644 --- a/rest_framework/tests/test_renderers.py +++ b/rest_framework/tests/test_renderers.py @@ -69,6 +69,12 @@ class MockGETView(APIView): return Response({'foo': ['bar', 'baz']}) +class MockPOSTView(APIView): + + def post(self, request, **kwargs): + return Response({'foo': request.DATA}) + + class HTMLView(APIView): renderer_classes = (BrowsableAPIRenderer, ) @@ -88,7 +94,7 @@ urlpatterns = patterns('', url(r'^cache$', MockGETView.as_view()), url(r'^jsonp/jsonrenderer$', MockGETView.as_view(renderer_classes=[JSONRenderer, JSONPRenderer])), url(r'^jsonp/nojsonrenderer$', MockGETView.as_view(renderer_classes=[JSONPRenderer])), - url(r'^parseerror$', MockGETView.as_view(renderer_classes=[JSONRenderer, BrowsableAPIRenderer])), + url(r'^parseerror$', MockPOSTView.as_view(renderer_classes=[JSONRenderer, BrowsableAPIRenderer])), url(r'^html$', HTMLView.as_view()), url(r'^html1$', HTMLView1.as_view()), url(r'^api', include('rest_framework.urls', namespace='rest_framework')) @@ -224,8 +230,8 @@ class RendererEndToEndTests(TestCase): """Invalid data should still render the browsable API correctly.""" resp = self.client.post('/parseerror', data='foobar', content_type='application/json', HTTP_ACCEPT='text/html') self.assertEqual(resp['Content-Type'], 'text/html; charset=utf-8') - self.assertContains(resp.content, 'Mock GET View') - self.assertEqual(resp.status_code, status.HTTP_400_) + self.assertIn('Mock Post', resp.content) + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) _flat_repr = '{"foo": ["bar", "baz"]}' _indented_repr = '{\n "foo": [\n "bar",\n "baz"\n ]\n}' From 4e9385e709bcee87456a99839841ecf6b56f337a Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Mon, 9 Dec 2013 07:37:13 +0000 Subject: [PATCH 4/4] Drop unneeded assert --- rest_framework/tests/test_renderers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rest_framework/tests/test_renderers.py b/rest_framework/tests/test_renderers.py index 10aa4248b..f4818eef2 100644 --- a/rest_framework/tests/test_renderers.py +++ b/rest_framework/tests/test_renderers.py @@ -230,7 +230,6 @@ class RendererEndToEndTests(TestCase): """Invalid data should still render the browsable API correctly.""" resp = self.client.post('/parseerror', data='foobar', content_type='application/json', HTTP_ACCEPT='text/html') self.assertEqual(resp['Content-Type'], 'text/html; charset=utf-8') - self.assertIn('Mock Post', resp.content) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) _flat_repr = '{"foo": ["bar", "baz"]}'