From 521765f63d724f0a1becb614530170f5d00fc6a9 Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Thu, 7 Mar 2013 06:33:51 +0700 Subject: [PATCH] Fixed #19541 -- Fixed BaseHandler to enable reversing URLs in response middlewares and streamed responses with respect to per-request urlconf. --- django/core/handlers/base.py | 183 ++++++++++++------------ django/test/testcases.py | 4 +- tests/urlpatterns_reverse/middleware.py | 23 +++ tests/urlpatterns_reverse/tests.py | 53 +++++++ 4 files changed, 170 insertions(+), 93 deletions(-) diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index 5327ce58911..900ea8e6b77 100644 --- a/django/core/handlers/base.py +++ b/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 diff --git a/django/test/testcases.py b/django/test/testcases.py index 66026dd23ea..b8e59b0a6a8 100644 --- a/django/test/testcases.py +++ b/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() diff --git a/tests/urlpatterns_reverse/middleware.py b/tests/urlpatterns_reverse/middleware.py index 03749e9570c..fbf577786e2 100644 --- a/tests/urlpatterns_reverse/middleware.py +++ b/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()) diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 4a45e63cb0e..24f96fac7ca 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/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"""