Merge pull request #519 from acdha/robust-format-negotiation

Altered ``determine_format`` to handle malformed HTTP Accept values.

tastypie/api.py

@@ -2,7 +2,7 @@
 from django.conf.urls.defaults import *
 from django.core.exceptions import ImproperlyConfigured
 from django.core.urlresolvers import reverse
-from django.http import HttpResponse
+from django.http import HttpResponse, HttpResponseBadRequest
 from tastypie.exceptions import NotRegistered, BadRequest
 from tastypie.serializers import Serializer
 from tastypie.utils import trailing_slash, is_valid_jsonp_callback_value
@@ -72,7 +72,10 @@ def canonical_resource_for(self, resource_name):
 
     def wrap_view(self, view):
         def wrapper(request, *args, **kwargs):
-            return getattr(self, view)(request, *args, **kwargs)
+            try:
+                return getattr(self, view)(request, *args, **kwargs)
+            except BadRequest:
+                return HttpResponseBadRequest()
         return wrapper
 
     def override_urls(self):
@@ -137,6 +140,7 @@ def top_level(self, request, api_name=None):
             }
 
         desired_format = determine_format(request, serializer)
+
         options = {}
 
         if 'text/javascript' in desired_format:

tastypie/resources.py

@@ -223,7 +223,7 @@ def wrapper(request, *args, **kwargs):
 
                 return response
             except (BadRequest, fields.ApiFieldError), e:
-                data = {"error": e.args[0]}
+                data = {"error": e.args[0] if getattr(e, 'args') else ''}
                 return self.error_response(request, data, response_class=http.HttpBadRequest)
             except ValidationError, e:
                 data = {"error": e.messages}
@@ -1191,13 +1191,19 @@ def error_response(self, request, errors, response_class=None):
         if response_class is None:
             response_class = http.HttpBadRequest
 
+        desired_format = None
+
         if request:
             if request.GET.get('callback', None) is None:
-                desired_format = self.determine_format(request)
+                try:
+                    desired_format = self.determine_format(request)
+                except BadRequest:
+                    pass  # Fall through to default handler below
             else:
                 # JSONP can cause extra breakage.
                 desired_format = 'application/json'
-        else:
+
+        if not desired_format:
             desired_format = self._meta.default_format
 
         try:

tastypie/utils/mime.py

@@ -1,5 +1,7 @@
 import mimeparse
 
+from tastypie.exceptions import BadRequest
+
 
 def determine_format(request, serializer, default_format='application/json'):
     """
@@ -13,14 +15,17 @@ def determine_format(request, serializer, default_format='application/json'):
 
     If still no format is found, returns the ``default_format`` (which defaults
     to ``application/json`` if not provided).
+
+    NOTE: callers *must* be prepared to handle BadRequest exceptions due to
+          malformed HTTP request headers!
     """
     # First, check if they forced the format.
     if request.GET.get('format'):
         if request.GET['format'] in serializer.formats:
             return serializer.get_mime_for_format(request.GET['format'])
 
     # If callback parameter is present, use JSONP.
-    if request.GET.has_key('callback'):
+    if 'callback' in request.GET:
         return serializer.get_mime_for_format('jsonp')
 
     # Try to fallback on the Accepts header.
@@ -30,7 +35,11 @@ def determine_format(request, serializer, default_format='application/json'):
         # https://github.com/toastdriven/django-tastypie/issues#issue/12 for
         # more information.
         formats.reverse()
-        best_format = mimeparse.best_match(formats, request.META['HTTP_ACCEPT'])
+
+        try:
+            best_format = mimeparse.best_match(formats, request.META['HTTP_ACCEPT'])
+        except ValueError:
+            raise BadRequest('Invalid Accept header')
 
         if best_format:
             return best_format

tests/basic/tests/http.py

@@ -22,6 +22,25 @@ def test_get_apis_json(self):
         self.assertEqual(response.status, 200)
         self.assertEqual(data, '{"cached_users": {"list_endpoint": "/api/v1/cached_users/", "schema": "/api/v1/cached_users/schema/"}, "notes": {"list_endpoint": "/api/v1/notes/", "schema": "/api/v1/notes/schema/"}, "private_cached_users": {"list_endpoint": "/api/v1/private_cached_users/", "schema": "/api/v1/private_cached_users/schema/"}, "public_cached_users": {"list_endpoint": "/api/v1/public_cached_users/", "schema": "/api/v1/public_cached_users/schema/"}, "users": {"list_endpoint": "/api/v1/users/", "schema": "/api/v1/users/schema/"}}')
 
+    def test_get_apis_invalid_accept(self):
+        connection = self.get_connection()
+        connection.request('GET', '/api/v1/', headers={'Accept': 'invalid'})
+        response = connection.getresponse()
+        connection.close()
+        data = response.read()
+        self.assertEqual(response.status, 400, "Invalid HTTP Accept headers should return HTTP 400")
+
+    def test_get_resource_invalid_accept(self):
+        """Invalid HTTP Accept headers should return HTTP 400"""
+        # We need to test this twice as there's a separate dispatch path for resources:
+
+        connection = self.get_connection()
+        connection.request('GET', '/api/v1/notes/', headers={'Accept': 'invalid'})
+        response = connection.getresponse()
+        connection.close()
+        data = response.read()
+        self.assertEqual(response.status, 400, "Invalid HTTP Accept headers should return HTTP 400")
+
     def test_get_apis_xml(self):
         connection = self.get_connection()
         connection.request('GET', '/api/v1/', headers={'Accept': 'application/xml'})

tests/core/tests/utils.py

@@ -1,5 +1,7 @@
 from django.http import HttpRequest
 from django.test import TestCase
+
+from tastypie.exceptions import BadRequest
 from tastypie.serializers import Serializer
 from tastypie.utils.mime import determine_format, build_content_type
 
@@ -84,3 +86,6 @@ def test_determine_format(self):
 
         request.META = {'HTTP_ACCEPT': 'text/javascript,application/json'}
         self.assertEqual(determine_format(request, serializer), 'application/json')
+
+        request.META = {'HTTP_ACCEPT': 'bogon'}
+        self.assertRaises(BadRequest, determine_format, request, serializer)