Merge pull request #885 from loic/ticket19541

Fixed #19541 -- Fixed BaseHandler to enable reversing URLs in response middlewares...

django/core/handlers/base.py

@@ -74,110 +74,109 @@ class BaseHandler(object):
 
     def get_response(self, request):
         "Returns an HttpResponse object for the given HttpRequest"
+
+        # Setup default url resolver for this thread, this code is outside
+        # the try/except so we don't get a spurious "unbound local
+        # variable" exception in the event an exception is raised before
+        # resolver is set
+        urlconf = settings.ROOT_URLCONF
+        urlresolvers.set_urlconf(urlconf)
+        resolver = urlresolvers.RegexURLResolver(r'^/', urlconf)
         try:
-            # Setup default url resolver for this thread, this code is outside
-            # the try/except so we don't get a spurious "unbound local
-            # variable" exception in the event an exception is raised before
-            # resolver is set
-            urlconf = settings.ROOT_URLCONF
-            urlresolvers.set_urlconf(urlconf)
-            resolver = urlresolvers.RegexURLResolver(r'^/', urlconf)
-            try:
-                response = None
-                # Apply request middleware
-                for middleware_method in self._request_middleware:
-                    response = middleware_method(request)
+            response = None
+            # Apply request middleware
+            for middleware_method in self._request_middleware:
+                response = middleware_method(request)
+                if response:
+                    break
+
+            if response is None:
+                if hasattr(request, 'urlconf'):
+                    # Reset url resolver with a custom urlconf.
+                    urlconf = request.urlconf
+                    urlresolvers.set_urlconf(urlconf)
+                    resolver = urlresolvers.RegexURLResolver(r'^/', urlconf)
+
+                resolver_match = resolver.resolve(request.path_info)
+                callback, callback_args, callback_kwargs = resolver_match
+                request.resolver_match = resolver_match
+
+                # Apply view middleware
+                for middleware_method in self._view_middleware:
+                    response = middleware_method(request, callback, callback_args, callback_kwargs)
                     if response:
                         break
 
-                if response is None:
-                    if hasattr(request, 'urlconf'):
-                        # Reset url resolver with a custom urlconf.
-                        urlconf = request.urlconf
-                        urlresolvers.set_urlconf(urlconf)
-                        resolver = urlresolvers.RegexURLResolver(r'^/', urlconf)
-
-                    resolver_match = resolver.resolve(request.path_info)
-                    callback, callback_args, callback_kwargs = resolver_match
-                    request.resolver_match = resolver_match
-
-                    # Apply view middleware
-                    for middleware_method in self._view_middleware:
-                        response = middleware_method(request, callback, callback_args, callback_kwargs)
+            if response is None:
+                wrapped_callback = self.make_view_atomic(callback)
+                try:
+                    response = wrapped_callback(request, *callback_args, **callback_kwargs)
+                except Exception as e:
+                    # If the view raised an exception, run it through exception
+                    # middleware, and if the exception middleware returns a
+                    # response, use that. Otherwise, reraise the exception.
+                    for middleware_method in self._exception_middleware:
+                        response = middleware_method(request, e)
                         if response:
                             break
+                    if response is None:
+                        raise
 
-                if response is None:
-                    wrapped_callback = self.make_view_atomic(callback)
-                    try:
-                        response = wrapped_callback(request, *callback_args, **callback_kwargs)
-                    except Exception as e:
-                        # If the view raised an exception, run it through exception
-                        # middleware, and if the exception middleware returns a
-                        # response, use that. Otherwise, reraise the exception.
-                        for middleware_method in self._exception_middleware:
-                            response = middleware_method(request, e)
-                            if response:
-                                break
-                        if response is None:
-                            raise
+            # Complain if the view returned None (a common error).
+            if response is None:
+                if isinstance(callback, types.FunctionType):    # FBV
+                    view_name = callback.__name__
+                else:                                           # CBV
+                    view_name = callback.__class__.__name__ + '.__call__'
+                raise ValueError("The view %s.%s didn't return an HttpResponse object." % (callback.__module__, view_name))
 
-                # Complain if the view returned None (a common error).
-                if response is None:
-                    if isinstance(callback, types.FunctionType):    # FBV
-                        view_name = callback.__name__
-                    else:                                           # CBV
-                        view_name = callback.__class__.__name__ + '.__call__'
-                    raise ValueError("The view %s.%s didn't return an HttpResponse object." % (callback.__module__, view_name))
+            # If the response supports deferred rendering, apply template
+            # response middleware and then render the response
+            if hasattr(response, 'render') and callable(response.render):
+                for middleware_method in self._template_response_middleware:
+                    response = middleware_method(request, response)
+                response = response.render()
 
-                # If the response supports deferred rendering, apply template
-                # response middleware and then render the response
-                if hasattr(response, 'render') and callable(response.render):
-                    for middleware_method in self._template_response_middleware:
-                        response = middleware_method(request, response)
-                    response = response.render()
-
-            except http.Http404 as e:
-                logger.warning('Not Found: %s', request.path,
-                            extra={
-                                'status_code': 404,
-                                'request': request
-                            })
-                if settings.DEBUG:
-                    response = debug.technical_404_response(request, e)
-                else:
-                    try:
-                        callback, param_dict = resolver.resolve404()
-                        response = callback(request, **param_dict)
-                    except:
-                        signals.got_request_exception.send(sender=self.__class__, request=request)
-                        response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
-            except PermissionDenied:
-                logger.warning(
-                    'Forbidden (Permission denied): %s', request.path,
-                    extra={
-                        'status_code': 403,
-                        'request': request
-                    })
+        except http.Http404 as e:
+            logger.warning('Not Found: %s', request.path,
+                        extra={
+                            'status_code': 404,
+                            'request': request
+                        })
+            if settings.DEBUG:
+                response = debug.technical_404_response(request, e)
+            else:
                 try:
-                    callback, param_dict = resolver.resolve403()
+                    callback, param_dict = resolver.resolve404()
                     response = callback(request, **param_dict)
                 except:
-                    signals.got_request_exception.send(
-                            sender=self.__class__, request=request)
-                    response = self.handle_uncaught_exception(request,
-                            resolver, sys.exc_info())
-            except SystemExit:
-                # Allow sys.exit() to actually exit. See tickets #1023 and #4701
-                raise
-            except: # Handle everything else, including SuspiciousOperation, etc.
-                # Get the exception info now, in case another exception is thrown later.
-                signals.got_request_exception.send(sender=self.__class__, request=request)
-                response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
-        finally:
-            # Reset URLconf for this thread on the way out for complete
-            # isolation of request.urlconf
-            urlresolvers.set_urlconf(None)
+                    signals.got_request_exception.send(sender=self.__class__, request=request)
+                    response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
+
+        except PermissionDenied:
+            logger.warning(
+                'Forbidden (Permission denied): %s', request.path,
+                extra={
+                    'status_code': 403,
+                    'request': request
+                })
+            try:
+                callback, param_dict = resolver.resolve403()
+                response = callback(request, **param_dict)
+            except:
+                signals.got_request_exception.send(
+                        sender=self.__class__, request=request)
+                response = self.handle_uncaught_exception(request,
+                        resolver, sys.exc_info())
+
+        except SystemExit:
+            # Allow sys.exit() to actually exit. See tickets #1023 and #4701
+            raise
+
+        except: # Handle everything else, including SuspiciousOperation, etc.
+            # Get the exception info now, in case another exception is thrown later.
+            signals.got_request_exception.send(sender=self.__class__, request=request)
+            response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
 
         try:
             # Apply response middleware, regardless of the response

django/test/testcases.py

@@ -26,7 +26,7 @@ from django.core.management import call_command
 from django.core.management.color import no_style
 from django.core.servers.basehttp import (WSGIRequestHandler, WSGIServer,
     WSGIServerException)
-from django.core.urlresolvers import clear_url_caches
+from django.core.urlresolvers import clear_url_caches, set_urlconf
 from django.db import connection, connections, DEFAULT_DB_ALIAS, transaction
 from django.forms.fields import CharField
 from django.http import QueryDict
@@ -497,6 +497,7 @@ class TransactionTestCase(SimpleTestCase):
                              **{'verbosity': 0, 'database': db_name, 'skip_validation': True})
 
     def _urlconf_setup(self):
+        set_urlconf(None)
         if hasattr(self, 'urls'):
             self._old_root_urlconf = settings.ROOT_URLCONF
             settings.ROOT_URLCONF = self.urls
@@ -527,6 +528,7 @@ class TransactionTestCase(SimpleTestCase):
                          skip_validation=True, reset_sequences=False)
 
     def _urlconf_teardown(self):
+        set_urlconf(None)
         if hasattr(self, '_old_root_urlconf'):
             settings.ROOT_URLCONF = self._old_root_urlconf
             clear_url_caches()

tests/urlpatterns_reverse/middleware.py

@@ -1,5 +1,8 @@
 from __future__ import absolute_import
 
+from django.core.urlresolvers import reverse
+from django.http import HttpResponse, StreamingHttpResponse
+
 from . import urlconf_inner
 
 
@@ -10,3 +13,23 @@ class ChangeURLconfMiddleware(object):
 class NullChangeURLconfMiddleware(object):
     def process_request(self, request):
         request.urlconf = None
+
+class ReverseInnerInResponseMiddleware(object):
+    def process_response(self, *args, **kwargs):
+        return HttpResponse(reverse('inner'))
+
+class ReverseOuterInResponseMiddleware(object):
+    def process_response(self, *args, **kwargs):
+        return HttpResponse(reverse('outer'))
+
+class ReverseInnerInStreaming(object):
+    def process_view(self, *args, **kwargs):
+        def stream():
+            yield reverse('inner')
+        return StreamingHttpResponse(stream())
+
+class ReverseOuterInStreaming(object):
+    def process_view(self, *args, **kwargs):
+        def stream():
+            yield reverse('outer')
+        return StreamingHttpResponse(stream())

tests/urlpatterns_reverse/tests.py

@@ -462,6 +462,59 @@ class RequestURLconfTests(TestCase):
         )
         self.assertRaises(ImproperlyConfigured, self.client.get, '/test/me/')
 
+    def test_reverse_inner_in_response_middleware(self):
+        """
+        Test reversing an URL from the *overridden* URLconf from inside
+        a response middleware.
+        """
+        settings.MIDDLEWARE_CLASSES += (
+            '%s.ChangeURLconfMiddleware' % middleware.__name__,
+            '%s.ReverseInnerInResponseMiddleware' % middleware.__name__,
+        )
+        response = self.client.get('/second_test/')
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, b'/second_test/')
+
+    def test_reverse_outer_in_response_middleware(self):
+        """
+        Test reversing an URL from the *default* URLconf from inside
+        a response middleware.
+        """
+        settings.MIDDLEWARE_CLASSES += (
+            '%s.ChangeURLconfMiddleware' % middleware.__name__,
+            '%s.ReverseOuterInResponseMiddleware' % middleware.__name__,
+        )
+        message = "Reverse for 'outer' with arguments '()' and keyword arguments '{}' not found."
+        with self.assertRaisesMessage(NoReverseMatch, message):
+            self.client.get('/second_test/')
+
+    def test_reverse_inner_in_streaming(self):
+        """
+        Test reversing an URL from the *overridden* URLconf from inside
+        a streaming response.
+        """
+        settings.MIDDLEWARE_CLASSES += (
+            '%s.ChangeURLconfMiddleware' % middleware.__name__,
+            '%s.ReverseInnerInStreaming' % middleware.__name__,
+        )
+        response = self.client.get('/second_test/')
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(b''.join(response), b'/second_test/')
+
+    def test_reverse_outer_in_streaming(self):
+        """
+        Test reversing an URL from the *default* URLconf from inside
+        a streaming response.
+        """
+        settings.MIDDLEWARE_CLASSES += (
+            '%s.ChangeURLconfMiddleware' % middleware.__name__,
+            '%s.ReverseOuterInStreaming' % middleware.__name__,
+        )
+        message = "Reverse for 'outer' with arguments '()' and keyword arguments '{}' not found."
+        with self.assertRaisesMessage(NoReverseMatch, message):
+            self.client.get('/second_test/')
+            b''.join(self.client.get('/second_test/'))
+
 class ErrorHandlerResolutionTests(TestCase):
     """Tests for handler404 and handler500"""