Merge pull request #1262 from tomchristie/fix-1261

Catch errors during parsing and set empty .DATA/.FILES before re-raising.

3 files changed, 31 insertions, 3 deletions

diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py
index fe4f43d4..2fdd3337 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 b883d0d4..fcea2508 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.
diff --git a/rest_framework/tests/test_renderers.py b/rest_framework/tests/test_renderers.py
index f7de8fd7..d13b3492 100644
--- a/rest_framework/tests/test_renderers.py
+++ b/rest_framework/tests/test_renderers.py
@@ -68,12 +68,19 @@ class MockGETView(APIView):
         return Response({'foo': ['bar', 'baz']})
 
 
+
+class MockPOSTView(APIView):
+    def post(self, request, **kwargs):
+        return Response({'foo': request.DATA})
+
+
 class EmptyGETView(APIView):
     renderer_classes = (JSONRenderer,)
 
     def get(self, request, **kwargs):
         return Response(status=status.HTTP_204_NO_CONTENT)
 
+
 class HTMLView(APIView):
     renderer_classes = (BrowsableAPIRenderer, )
 
@@ -93,6 +100,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$', MockPOSTView.as_view(renderer_classes=[JSONRenderer, BrowsableAPIRenderer])),
     url(r'^html$', HTMLView.as_view()),
     url(r'^html1$', HTMLView1.as_view()),
     url(r'^empty$', EmptyGETView.as_view()),
@@ -225,6 +233,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.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
+
     def test_204_no_content_responses_have_no_content_type_set(self):
         """
         Regression test for #1196