Improve JSONP middleware handling.

Don't add callback to non-JSON responses, or to responses that already
have a callback (presumably returned from a cache).

mapit/middleware/__init__.py

@@ -8,20 +8,31 @@
 
 class JSONPMiddleware(object):
     def process_response(self, request, response):
-        # If the response is a redirect, the callback will be dealt
-        # on the next request:
+        # If the response is a redirect, the callback will be dealt on the next request
         if response.status_code == 302:
             return response
-        else:
-            cb = request.GET.get('callback')
-            if cb and re.match('[a-zA-Z0-9_$.]+$', cb):
-                cb = cb.encode('utf-8')
-                attr = 'streaming_content' if getattr(response, 'streaming', None) else 'content'
-                callback_header = b'typeof ' + cb + b" === 'function' && " + cb + b'('
-                callback_footer = b')'
-                content = getattr(response, attr)
-                if not hasattr(content, '__iter__') or isinstance(content, (bytes, six.string_types)):
-                    content = [content]
-                setattr(response, attr, itertools.chain((callback_header,), content, (callback_footer,)))
-                response.status_code = 200  # Must return OK for JSONP to be processed
+
+        # If it's not a JSON response, bit silly to add a callback
+        if 'application/json' not in response['Content-Type']:
+            return response
+
+        # If a callback is already present (fetched from cache)
+        if getattr(response, 'content', '')[:7] == 'typeof ':
             return response
+
+        cb = request.GET.get('callback')
+
+        # Callback variable not present or not in the right format
+        if not cb or not re.match('[a-zA-Z0-9_$.]+$', cb):
+            return response
+
+        cb = cb.encode('utf-8')
+        attr = 'streaming_content' if getattr(response, 'streaming', None) else 'content'
+        callback_header = b'typeof ' + cb + b" === 'function' && " + cb + b'('
+        callback_footer = b')'
+        content = getattr(response, attr)
+        if not hasattr(content, '__iter__') or isinstance(content, (bytes, six.string_types)):
+            content = [content]
+        setattr(response, attr, itertools.chain((callback_header,), content, (callback_footer,)))
+        response.status_code = 200  # Must return OK for JSONP to be processed
+        return response

mapit/tests/test_middleware.py

@@ -19,30 +19,30 @@ def test_process_response_ignores_302_redirects(self):
 
     def test_process_response_uses_callback(self):
         request = self.factory.get("/dummy_url", {"callback": "xyz"})
-        response = http.HttpResponse(content="blah")
+        response = http.HttpResponse(content="blah", content_type='application/json')
         middleware_response = self.middleware.process_response(request, response)
         self.assertEqual(middleware_response.content, b"typeof xyz === 'function' && xyz(blah)")
 
     def test_process_response_uses_callback_streaming(self):
         request = self.factory.get("/dummy_url", {"callback": "xyz"})
-        response = http.StreamingHttpResponse("blah")
+        response = http.StreamingHttpResponse("blah", content_type='application/json')
         middleware_response = self.middleware.process_response(request, response)
         self.assertEqual(b''.join(middleware_response.streaming_content), b"typeof xyz === 'function' && xyz(blah)")
 
     def test_process_response_uses_ignores_requests_without_callback(self):
         request = self.factory.get("/dummy_url")
-        response = http.HttpResponse(content="blah")
+        response = http.HttpResponse(content="blah", content_type='application/json')
         middleware_response = self.middleware.process_response(request, response)
         self.assertEqual(middleware_response, response)
 
     def test_process_response_callback_allowed_characters(self):
         request = self.factory.get("/dummy_url", {"callback": "xyz123_$."})
-        response = http.HttpResponse(content="blah")
+        response = http.HttpResponse(content="blah", content_type='application/json')
         middleware_response = self.middleware.process_response(request, response)
         self.assertEqual(middleware_response.content, b"typeof xyz123_$. === 'function' && xyz123_$.(blah)")
 
         # Try with a character not allowed in the callback
         request = self.factory.get("/dummy_url", {"callback": "xyz123_$.["})
-        response = http.HttpResponse(content="blah")
+        response = http.HttpResponse(content="blah", content_type='application/json')
         middleware_response = self.middleware.process_response(request, response)
         self.assertEqual(middleware_response, response)