From 69de988f9237fe978c5fca363ca2d2622f8e2ccd Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 14 Jun 2016 00:41:58 -0700 Subject: [PATCH] [1.10.x] Refs #26601 -- Improved backwards-compatibility of DEP 5 middleware exception handling. Backport of 7d1b69dbe7f72ac04d2513f0468fe2146231b286 from master --- django/contrib/flatpages/middleware.py | 14 +- django/contrib/redirects/middleware.py | 9 +- django/core/handlers/base.py | 77 +++----- django/core/handlers/exception.py | 135 +++++++++++++ django/middleware/exception.py | 78 -------- django/middleware/locale.py | 15 +- django/utils/deprecation.py | 8 +- docs/topics/http/middleware.txt | 210 ++++++++++++--------- tests/middleware_exceptions/middleware.py | 54 +++++- tests/middleware_exceptions/test_legacy.py | 89 +-------- tests/middleware_exceptions/tests.py | 133 +++++++++++++ 11 files changed, 476 insertions(+), 346 deletions(-) create mode 100644 django/core/handlers/exception.py delete mode 100644 django/middleware/exception.py create mode 100644 tests/middleware_exceptions/tests.py diff --git a/django/contrib/flatpages/middleware.py b/django/contrib/flatpages/middleware.py index 4c6760535d2..9c6a7273ccc 100644 --- a/django/contrib/flatpages/middleware.py +++ b/django/contrib/flatpages/middleware.py @@ -1,20 +1,10 @@ from django.conf import settings from django.contrib.flatpages.views import flatpage from django.http import Http404 -from django.middleware.exception import ExceptionMiddleware +from django.utils.deprecation import MiddlewareMixin -class FlatpageFallbackMiddleware(ExceptionMiddleware): - - def __init__(self, get_response=None): - # This override makes get_response optional during the - # MIDDLEWARE_CLASSES deprecation. - super(FlatpageFallbackMiddleware, self).__init__(get_response) - - def __call__(self, request): - response = super(FlatpageFallbackMiddleware, self).__call__(request) - return self.process_response(request, response) - +class FlatpageFallbackMiddleware(MiddlewareMixin): def process_response(self, request, response): if response.status_code != 404: return response # No need to check for a flatpage for non-404 responses. diff --git a/django/contrib/redirects/middleware.py b/django/contrib/redirects/middleware.py index e3ee982a8b2..f5c9f855884 100644 --- a/django/contrib/redirects/middleware.py +++ b/django/contrib/redirects/middleware.py @@ -6,11 +6,10 @@ from django.conf import settings from django.contrib.redirects.models import Redirect from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ImproperlyConfigured -from django.middleware.exception import ExceptionMiddleware +from django.utils.deprecation import MiddlewareMixin -class RedirectFallbackMiddleware(ExceptionMiddleware): - +class RedirectFallbackMiddleware(MiddlewareMixin): # Defined as class-level attributes to be subclassing-friendly. response_gone_class = http.HttpResponseGone response_redirect_class = http.HttpResponsePermanentRedirect @@ -23,10 +22,6 @@ class RedirectFallbackMiddleware(ExceptionMiddleware): ) super(RedirectFallbackMiddleware, self).__init__(get_response) - def __call__(self, request): - response = super(RedirectFallbackMiddleware, self).__call__(request) - return self.process_response(request, response) - def process_response(self, request, response): # No need to check for a redirect for non-404 responses. if response.status_code != 404: diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index fbe7a8b8a0b..55802f6a652 100644 --- a/django/core/handlers/base.py +++ b/django/core/handlers/base.py @@ -9,12 +9,15 @@ from django.conf import settings from django.core import signals from django.core.exceptions import ImproperlyConfigured, MiddlewareNotUsed from django.db import connections, transaction -from django.middleware.exception import ExceptionMiddleware from django.urls import get_resolver, get_urlconf, set_urlconf from django.utils import six from django.utils.deprecation import RemovedInDjango20Warning from django.utils.module_loading import import_string -from django.views import debug + +from .exception import ( + convert_exception_to_response, get_exception_response, + handle_uncaught_exception, +) logger = logging.getLogger('django.request') @@ -48,7 +51,7 @@ class BaseHandler(object): "deprecated. Update your middleware and use settings.MIDDLEWARE " "instead.", RemovedInDjango20Warning ) - handler = self._legacy_get_response + handler = convert_exception_to_response(self._legacy_get_response) for middleware_path in settings.MIDDLEWARE_CLASSES: mw_class = import_string(middleware_path) try: @@ -72,7 +75,7 @@ class BaseHandler(object): if hasattr(mw_instance, 'process_exception'): self._exception_middleware.insert(0, mw_instance.process_exception) else: - handler = self._get_response + handler = convert_exception_to_response(self._get_response) for middleware_path in reversed(settings.MIDDLEWARE): middleware = import_string(middleware_path) try: @@ -94,10 +97,10 @@ class BaseHandler(object): self._view_middleware.insert(0, mw_instance.process_view) if hasattr(mw_instance, 'process_template_response'): self._template_response_middleware.append(mw_instance.process_template_response) + if hasattr(mw_instance, 'process_exception'): + self._exception_middleware.append(mw_instance.process_exception) - handler = mw_instance - - handler = ExceptionMiddleware(handler, self) + handler = convert_exception_to_response(mw_instance) # We only assign to this when initialization is complete as it is used # as a flag for initialization being complete. @@ -111,25 +114,7 @@ class BaseHandler(object): return view def get_exception_response(self, request, resolver, status_code, exception): - try: - callback, param_dict = resolver.resolve_error_handler(status_code) - # Unfortunately, inspect.getargspec result is not trustable enough - # depending on the callback wrapping in decorators (frequent for handlers). - # Falling back on try/except: - try: - response = callback(request, **dict(param_dict, exception=exception)) - except TypeError: - warnings.warn( - "Error handlers should accept an exception parameter. Update " - "your code as this parameter will be required in Django 2.0", - RemovedInDjango20Warning, stacklevel=2 - ) - response = callback(request, **param_dict) - except Exception: - signals.got_request_exception.send(sender=self.__class__, request=request) - response = self.handle_uncaught_exception(request, resolver, sys.exc_info()) - - return response + return get_exception_response(request, resolver, status_code, exception, self.__class__) def get_response(self, request): """Return an HttpResponse object for the given HttpRequest.""" @@ -138,6 +123,8 @@ class BaseHandler(object): response = self._middleware_chain(request) + # This block is only needed for legacy MIDDLEWARE_CLASSES; if + # MIDDLEWARE is used, self._response_middleware will be empty. try: # Apply response middleware, regardless of the response for middleware_method in self._response_middleware: @@ -168,6 +155,11 @@ class BaseHandler(object): return response def _get_response(self, request): + """ + Resolve and call the view, then apply view, exception, and + template_response middleware. This method is everything that happens + inside the request/response middleware. + """ response = None if hasattr(request, 'urlconf'): @@ -237,35 +229,14 @@ class BaseHandler(object): raise def handle_uncaught_exception(self, request, resolver, exc_info): - """ - Processing for any otherwise uncaught exceptions (those that will - generate HTTP 500 responses). Can be overridden by subclasses who want - customised 500 handling. - - Be *very* careful when overriding this because the error could be - caused by anything, so assuming something like the database is always - available would be an error. - """ - if settings.DEBUG_PROPAGATE_EXCEPTIONS: - raise - - logger.error( - 'Internal Server Error: %s', request.path, - exc_info=exc_info, - extra={'status_code': 500, 'request': request}, - ) - - if settings.DEBUG: - return debug.technical_500_response(request, *exc_info) - - # If Http500 handler is not installed, re-raise last exception - if resolver.urlconf_module is None: - six.reraise(*exc_info) - # Return an HttpResponse that displays a friendly error message. - callback, param_dict = resolver.resolve_error_handler(500) - return callback(request, **param_dict) + """Allow subclasses to override uncaught exception handling.""" + return handle_uncaught_exception(request, resolver, exc_info) def _legacy_get_response(self, request): + """ + Apply process_request() middleware and call the main _get_response(), + if needed. Used only for legacy MIDDLEWARE_CLASSES. + """ response = None # Apply request middleware for middleware_method in self._request_middleware: diff --git a/django/core/handlers/exception.py b/django/core/handlers/exception.py new file mode 100644 index 00000000000..9edf457ad1e --- /dev/null +++ b/django/core/handlers/exception.py @@ -0,0 +1,135 @@ +from __future__ import unicode_literals + +import logging +import sys +import warnings +from functools import wraps + +from django.conf import settings +from django.core import signals +from django.core.exceptions import PermissionDenied, SuspiciousOperation +from django.http import Http404 +from django.http.multipartparser import MultiPartParserError +from django.urls import get_resolver, get_urlconf +from django.utils import six +from django.utils.decorators import available_attrs +from django.utils.deprecation import RemovedInDjango20Warning +from django.utils.encoding import force_text +from django.views import debug + +logger = logging.getLogger('django.request') + + +def convert_exception_to_response(get_response): + """ + Wrap the given get_response callable in exception-to-response conversion. + + All exceptions will be converted. All known 4xx exceptions (Http404, + PermissionDenied, MultiPartParserError, SuspiciousOperation) will be + converted to the appropriate response, and all other exceptions will be + converted to 500 responses. + + This decorator is automatically applied to all middleware to ensure that + no middleware leaks an exception and that the next middleware in the stack + can rely on getting a response instead of an exception. + """ + @wraps(get_response, assigned=available_attrs(get_response)) + def inner(request): + try: + response = get_response(request) + except Exception as exc: + response = response_for_exception(request, exc) + return response + return inner + + +def response_for_exception(request, exc): + if isinstance(exc, Http404): + if settings.DEBUG: + response = debug.technical_404_response(request, exc) + else: + response = get_exception_response(request, get_resolver(get_urlconf()), 404, exc) + + elif isinstance(exc, PermissionDenied): + logger.warning( + 'Forbidden (Permission denied): %s', request.path, + extra={'status_code': 403, 'request': request}, + ) + response = get_exception_response(request, get_resolver(get_urlconf()), 403, exc) + + elif isinstance(exc, MultiPartParserError): + logger.warning( + 'Bad request (Unable to parse request body): %s', request.path, + extra={'status_code': 400, 'request': request}, + ) + response = get_exception_response(request, get_resolver(get_urlconf()), 400, exc) + + elif isinstance(exc, SuspiciousOperation): + # The request logger receives events for any problematic request + # The security logger receives events for all SuspiciousOperations + security_logger = logging.getLogger('django.security.%s' % exc.__class__.__name__) + security_logger.error( + force_text(exc), + extra={'status_code': 400, 'request': request}, + ) + if settings.DEBUG: + response = debug.technical_500_response(request, *sys.exc_info(), status_code=400) + else: + response = get_exception_response(request, get_resolver(get_urlconf()), 400, exc) + + elif isinstance(exc, SystemExit): + # Allow sys.exit() to actually exit. See tickets #1023 and #4701 + raise + + else: + signals.got_request_exception.send(sender=None, request=request) + response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info()) + + return response + + +def get_exception_response(request, resolver, status_code, exception, sender=None): + try: + callback, param_dict = resolver.resolve_error_handler(status_code) + # Unfortunately, inspect.getargspec result is not trustable enough + # depending on the callback wrapping in decorators (frequent for handlers). + # Falling back on try/except: + try: + response = callback(request, **dict(param_dict, exception=exception)) + except TypeError: + warnings.warn( + "Error handlers should accept an exception parameter. Update " + "your code as this parameter will be required in Django 2.0", + RemovedInDjango20Warning, stacklevel=2 + ) + response = callback(request, **param_dict) + except Exception: + signals.got_request_exception.send(sender=sender, request=request) + response = handle_uncaught_exception(request, resolver, sys.exc_info()) + + return response + + +def handle_uncaught_exception(request, resolver, exc_info): + """ + Processing for any otherwise uncaught exceptions (those that will + generate HTTP 500 responses). + """ + if settings.DEBUG_PROPAGATE_EXCEPTIONS: + raise + + logger.error( + 'Internal Server Error: %s', request.path, + exc_info=exc_info, + extra={'status_code': 500, 'request': request}, + ) + + if settings.DEBUG: + return debug.technical_500_response(request, *exc_info) + + # If Http500 handler is not installed, reraise the last exception. + if resolver.urlconf_module is None: + six.reraise(*exc_info) + # Return an HttpResponse that displays a friendly error message. + callback, param_dict = resolver.resolve_error_handler(500) + return callback(request, **param_dict) diff --git a/django/middleware/exception.py b/django/middleware/exception.py deleted file mode 100644 index 437a82cf265..00000000000 --- a/django/middleware/exception.py +++ /dev/null @@ -1,78 +0,0 @@ -from __future__ import unicode_literals - -import logging -import sys - -from django.conf import settings -from django.core import signals -from django.core.exceptions import PermissionDenied, SuspiciousOperation -from django.http import Http404 -from django.http.multipartparser import MultiPartParserError -from django.urls import get_resolver, get_urlconf -from django.utils.encoding import force_text -from django.views import debug - -logger = logging.getLogger('django.request') - - -class ExceptionMiddleware(object): - """ - Convert selected exceptions to HTTP responses. - - For example, convert Http404 to a 404 response either through handler404 - or through the debug view if settings.DEBUG=True. To ensure that - exceptions raised by other middleware are converted to the appropriate - response, this middleware is always automatically applied as the outermost - middleware. - """ - def __init__(self, get_response, handler=None): - from django.core.handlers.base import BaseHandler - self.get_response = get_response - self.handler = handler or BaseHandler() - - def __call__(self, request): - try: - response = self.get_response(request) - except Http404 as exc: - if settings.DEBUG: - response = debug.technical_404_response(request, exc) - else: - response = self.handler.get_exception_response(request, get_resolver(get_urlconf()), 404, exc) - - except PermissionDenied as exc: - logger.warning( - 'Forbidden (Permission denied): %s', request.path, - extra={'status_code': 403, 'request': request}, - ) - response = self.handler.get_exception_response(request, get_resolver(get_urlconf()), 403, exc) - - except MultiPartParserError as exc: - logger.warning( - 'Bad request (Unable to parse request body): %s', request.path, - extra={'status_code': 400, 'request': request}, - ) - response = self.handler.get_exception_response(request, get_resolver(get_urlconf()), 400, exc) - - except SuspiciousOperation as exc: - # The request logger receives events for any problematic request - # The security logger receives events for all SuspiciousOperations - security_logger = logging.getLogger('django.security.%s' % exc.__class__.__name__) - security_logger.error( - force_text(exc), - extra={'status_code': 400, 'request': request}, - ) - if settings.DEBUG: - return debug.technical_500_response(request, *sys.exc_info(), status_code=400) - - response = self.handler.get_exception_response(request, get_resolver(get_urlconf()), 400, exc) - - except SystemExit: - # Allow sys.exit() to actually exit. See tickets #1023 and #4701 - raise - - except Exception: # Handle everything else. - # Get the exception info now, in case another exception is thrown later. - signals.got_request_exception.send(sender=self.handler.__class__, request=request) - response = self.handler.handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info()) - - return response diff --git a/django/middleware/locale.py b/django/middleware/locale.py index 1a95dbc8b21..65a3893e9ae 100644 --- a/django/middleware/locale.py +++ b/django/middleware/locale.py @@ -3,13 +3,13 @@ from django.conf import settings from django.conf.urls.i18n import is_language_prefix_patterns_used from django.http import HttpResponseRedirect -from django.middleware.exception import ExceptionMiddleware from django.urls import get_script_prefix, is_valid_path from django.utils import translation from django.utils.cache import patch_vary_headers +from django.utils.deprecation import MiddlewareMixin -class LocaleMiddleware(ExceptionMiddleware): +class LocaleMiddleware(MiddlewareMixin): """ This is a very simple middleware that parses a request and decides what translation object to install in the current @@ -19,17 +19,6 @@ class LocaleMiddleware(ExceptionMiddleware): """ response_redirect_class = HttpResponseRedirect - def __init__(self, get_response=None): - # This override makes get_response optional during the - # MIDDLEWARE_CLASSES deprecation. - super(LocaleMiddleware, self).__init__(get_response) - - def __call__(self, request): - response = self.process_request(request) - if not response: - response = super(LocaleMiddleware, self).__call__(request) - return self.process_response(request, response) - def process_request(self, request): urlconf = getattr(request, 'urlconf', settings.ROOT_URLCONF) i18n_patterns_used, prefixed_default_language = is_language_prefix_patterns_used(urlconf) diff --git a/django/utils/deprecation.py b/django/utils/deprecation.py index 7627426080b..351bd1d27d8 100644 --- a/django/utils/deprecation.py +++ b/django/utils/deprecation.py @@ -121,13 +121,7 @@ class MiddlewareMixin(object): if hasattr(self, 'process_request'): response = self.process_request(request) if not response: - try: - response = self.get_response(request) - except Exception as e: - if hasattr(self, 'process_exception'): - return self.process_exception(request, e) - else: - raise + response = self.get_response(request) if hasattr(self, 'process_response'): response = self.process_response(request, response) return response diff --git a/docs/topics/http/middleware.txt b/docs/topics/http/middleware.txt index 1f5414e95bc..1f792560272 100644 --- a/docs/topics/http/middleware.txt +++ b/docs/topics/http/middleware.txt @@ -40,14 +40,9 @@ A middleware can be written as a function that looks like this:: def middleware(request): # Code to be executed for each request before - # the view is called. + # the view (and later middleware) are called. - try: - response = get_response(request) - except Exception as e: - # Code to handle an exception that wasn't caught - # further up the chain, if desired. - ... + response = get_response(request) # Code to be executed for each request/response after # the view is called. @@ -56,7 +51,7 @@ A middleware can be written as a function that looks like this:: return middleware -Or it can be written as a class with a ``__call__()`` method, like this:: +Or it can be written as a class whose instances are callable, like this:: class SimpleMiddleware(object): def __init__(self, get_response): @@ -65,24 +60,15 @@ Or it can be written as a class with a ``__call__()`` method, like this:: def __call__(self, request): # Code to be executed for each request before - # the view is called. + # the view (and later middleware) are called. - try: - response = self.get_response(request) - except Exception as e: - # Code to handle an exception that wasn't caught - # further up the chain, if desired. - ... + response = self.get_response(request) # Code to be executed for each request/response after # the view is called. return response -In both examples, the ``try``/``except`` isn't required if the middleware -doesn't need to handle any exceptions. If it is included, it should probably -catch something more specific than ``Exception``. - The ``get_response`` callable provided by Django might be the actual view (if this is the last listed middleware) or it might be the next middleware in the chain. The current middleware doesn't need to know or care what exactly it is, @@ -92,30 +78,32 @@ The above is a slight simplification -- the ``get_response`` callable for the last middleware in the chain won't be the actual view but rather a wrapper method from the handler which takes care of applying :ref:`view middleware `, calling the view with appropriate URL arguments, and -applying :ref:`template-response ` middleware. +applying :ref:`template-response ` and +:ref:`exception ` middleware. Middleware can live anywhere on your Python path. ``__init__(get_response)`` -------------------------- -Middleware classes must accept a ``get_response`` argument. You can also +Middleware factories must accept a ``get_response`` argument. You can also initialize some global state for the middleware. Keep in mind a couple of caveats: * Django initializes your middleware with only the ``get_response`` argument, so you can't define ``__init__()`` as requiring any other arguments. -* Unlike the ``__call__()`` method which get called once per request, +* Unlike the ``__call__()`` method which is called once per request, ``__init__()`` is called only *once*, when the Web server starts. .. versionchanged:: 1.10 - In older versions, ``__init__`` was not called until the Web server + In older versions, ``__init__()`` wasn't called until the Web server responded to its first request. - If you want to allow your middleware to be used in Django 1.9 and earlier, - make ``get_response`` an optional argument (``get_response=None``). + In older versions, ``__init__()`` didn't accept any arguments. To allow + your middleware to be used in Django 1.9 and earlier, make ``get_response`` + an optional argument (``get_response=None``). Marking middleware as unused ---------------------------- @@ -133,9 +121,9 @@ To activate a middleware component, add it to the :setting:`MIDDLEWARE` list in your Django settings. In :setting:`MIDDLEWARE`, each middleware component is represented by a string: -the full Python path to the middleware's class or function name. For example, -here's the default value created by :djadmin:`django-admin startproject -`:: +the full Python path to the middleware factory's class or function name. For +example, here's the default value created by :djadmin:`django-admin +startproject `:: MIDDLEWARE = [ 'django.middleware.security.SecurityMiddleware', @@ -159,25 +147,29 @@ authenticated user in the session; therefore, it must run after :ref:`middleware-ordering` for some common hints about ordering of Django middleware classes. -Hooks and application order -=========================== +Middleware order and layering +============================= -During the request phase, before calling the view, Django applies middleware -in the order it's defined in :setting:`MIDDLEWARE`, top-down. You can think of -it like an onion: each middleware class is a "layer" that wraps the view. +During the request phase, before calling the view, Django applies middleware in +the order it's defined in :setting:`MIDDLEWARE`, top-down. -Middleware see only the changes made by middleware that run before it. A -middleware (and the view) is skipped entirely if a preceding middleware -short-circuits by returning a response without ever calling ``get_response``. -That response will only pass through the middleware that have already run. +You can think of it like an onion: each middleware class is a "layer" that +wraps the view, which is in the core of the onion. If the request passes +through all the layers of the onion (each one calls ``get_response`` to pass +the request in to the next layer), all the way to the view at the core, the +response will then pass through every layer (in reverse order) on the way back +out. -Similarly, a middleware that sees the request on the way in and doesn't return -a response is guaranteed that it will always see the response on the way back -out. If the middleware also wants to see any uncaught exception on the way out, -it can wrap its call to ``get_response()`` in a ``try``/``except``. +If one of the layers decides to short-circuit and return a response without +ever calling its ``get_response``, none of the layers of the onion inside that +layer (including the view) will see the request or the response. The response +will only return through the same layers that the request passed in through. -Besides the middleware pattern described earlier, you can add two other methods -to class-based middleware: +Other middleware hooks +====================== + +Besides the basic request/response middleware pattern described earlier, you +can add three other special methods to class-based middleware: .. _view-middleware: @@ -217,6 +209,28 @@ bother calling the appropriate view; it'll apply response middleware to that :func:`~django.views.decorators.csrf.csrf_protect` decorators which allow views to explicitly control at what point the CSRF validation should occur. +.. _exception-middleware: + +``process_exception()`` +----------------------- + +.. method:: process_exception(request, exception) + +``request`` is an :class:`~django.http.HttpRequest` object. ``exception`` is an +``Exception`` object raised by the view function. + +Django calls ``process_exception()`` when a view raises an exception. +``process_exception()`` should return either ``None`` or an +:class:`~django.http.HttpResponse` object. If it returns an +:class:`~django.http.HttpResponse` object, the template response and response +middleware will be applied and the resulting response returned to the +browser. Otherwise, :ref:`default exception handling ` kicks in. + +Again, middleware are run in reverse order during the response phase, which +includes ``process_exception``. If an exception middleware returns a response, +the ``process_exception`` methods of the middleware classes above that +middleware won't be called at all. + .. _template-response-middleware: ``process_template_response()`` @@ -268,31 +282,24 @@ must test for streaming responses and adjust their behavior accordingly:: for chunk in content: yield alter_content(chunk) -.. _exception-middleware: +Exception handling +================== -Exception middleware -==================== +Django automatically converts exceptions raised by the view or by middleware +into an appropriate HTTP response with an error status code. :ref:`Certain +exceptions ` are converted to 4xx status codes, while an unknown +exception is converted to a 500 status code. -A middleware that does some custom exception handling might looks like this:: - - class ExceptionMiddleware(object): - def __init__(self, get_response): - self.get_response = get_response - - def __call__(self, request): - try: - response = self.get_response(request) - except Exception as e: - # Do something with the exception and possibly reraise it - # unless you wish to silence it. - ... - return response - -Middleware that wants to do something for all exception responses, an HTTP 404 -for example, need to both catch the appropriate exception (e.g. ``Http404``) -and look for regular responses with the status code of interest. You can -subclass :class:`~django.middleware.exception.ExceptionMiddleware` if you want -to transform exceptions into the appropriate response. +This conversion takes place before and after each middleware (you can think of +it as the thin film in between each layer of the onion), so that every +middleware can always rely on getting some kind of HTTP response back from +calling its ``get_response`` callable. Middleware don't need to worry about +wrapping their call to ``get_response`` in a ``try/except`` and handling an +exception that might have been raised by a later middleware or the view. Even +if the very next middleware in the chain raises an +:class:`~django.http.Http404` exception, for example, your middleware won't see +that exception; instead it will get an :class:`~django.http.HttpResponse` +object with a :attr:`~django.http.HttpResponse.status_code` of 404. .. _upgrading-middleware: @@ -302,30 +309,57 @@ Upgrading pre-Django 1.10-style middleware .. class:: django.utils.deprecation.MiddlewareMixin :module: -Django provides ``django.utils.deprecation.MiddlewareMixin`` to ease providing -the existing built-in middleware in both new-style and old-style forms and to -ease similar conversions of third-party middleware. +Django provides ``django.utils.deprecation.MiddlewareMixin`` to ease creating +middleware classes that are compatible with both :setting:`MIDDLEWARE` and the +old :setting:`MIDDLEWARE_CLASSES`. -In most cases, this mixin will be sufficient to convert a middleware with -sufficient backwards-compatibility; the new short-circuiting semantics will be -harmless or even beneficial to the existing middleware. +The mixin provides an ``__init__()`` method that accepts an optional +``get_response`` argument and stores it in ``self.get_response``. -In a few cases, a middleware class may need more invasive changes to adjust to -the new semantics. +The ``__call__()`` method: -For example, in the current request-handling logic, the handler transforms any -exception that passes through all ``process_exception`` middleware uncaught -into a response with appropriate status code (e.g. 404, 403, 400, or 500), and -then passes that response through the full chain of ``process_response`` -middleware. +#. Calls ``self.process_request(request)`` (if defined). +#. Calls ``self.get_response(request)`` to get the response from later + middleware and the view. +#. Calls ``self.process_response(request, response)`` (if defined). +#. Returns the response. -In new-style middleware, a given middleware only gets one shot at a given -response or uncaught exception "on the way out," and will see either a returned -response or an uncaught exception, but not both. +If used with :setting:`MIDDLEWARE_CLASSES`, the ``__call__()`` method will +never be used; Django calls ``process_request()`` and ``process_response()`` +directly. -This means that certain middleware which want to do something with all 404 -responses (for example, the ``RedirectFallbackMiddleware`` and -``FlatpageFallbackMiddleware`` in ``django.contrib.redirects`` and -``django.contrib.flatpages``) now need to watch out for both a 404 response -and an uncaught ``Http404`` exception. They do this by subclassing -:class:`~django.middleware.exception.ExceptionMiddleware`. +In most cases, inheriting from this mixin will be sufficient to make an +old-style middleware compatible with the new system with sufficient +backwards-compatibility. The new short-circuiting semantics will be harmless or +even beneficial to the existing middleware. In a few cases, a middleware class +may need some changes to adjust to the new semantics. + +These are the behavioral differences between using :setting:`MIDDLEWARE` and +:setting:`MIDDLEWARE_CLASSES`: + +1. Under :setting:`MIDDLEWARE_CLASSES`, every middleware will always have its + ``process_response`` method called, even if an earlier middleware + short-circuited by returning a response from its ``process_request`` + method. Under :setting:`MIDDLEWARE`, middleware behaves more like an onion: + the layers that a response goes through on the way out are the same layers + that saw the request on the way in. If a middleware short-circuits, only + that middleware and the ones before it in :setting:`MIDDLEWARE` will see the + response. + +2. Under :setting:`MIDDLEWARE_CLASSES`, ``process_exception`` is applied to + exceptions raised from a middleware ``process_request`` method. Under + :setting:`MIDDLEWARE`, ``process_exception`` applies only to exceptions + raised from the view (or from the ``render`` method of a + :class:`~django.template.response.TemplateResponse`). Exceptions raised from + a middleware are converted to the appropriate HTTP response and then passed + to the next middleware. + +3. Under :setting:`MIDDLEWARE_CLASSES`, if a ``process_response`` method raises + an exception, the ``process_response`` methods of all earlier middleware are + skipped and a ``500 Internal Server Error`` HTTP response is always + returned (even if the exception raised was e.g. an + :class:`~django.http.Http404`). Under :setting:`MIDDLEWARE`, an exception + raised from a middleware will immediately be converted to the appropriate + HTTP response, and then the next middleware in line will see that + response. Middleware are never skipped due to a middleware raising an + exception. diff --git a/tests/middleware_exceptions/middleware.py b/tests/middleware_exceptions/middleware.py index 6871d9556ea..690660fb53a 100644 --- a/tests/middleware_exceptions/middleware.py +++ b/tests/middleware_exceptions/middleware.py @@ -1,10 +1,58 @@ from __future__ import unicode_literals -from django.http import HttpResponse -from django.utils.deprecation import MiddlewareMixin +from django.http import Http404, HttpResponse +from django.template import engines + +log = [] -class ProcessExceptionMiddleware(MiddlewareMixin): +class BaseMiddleware(object): + def __init__(self, get_response): + self.get_response = get_response + def __call__(self, request): + return self.get_response(request) + + +class ProcessExceptionMiddleware(BaseMiddleware): def process_exception(self, request, exception): return HttpResponse('Exception caught') + + +class ProcessExceptionLogMiddleware(BaseMiddleware): + def process_exception(self, request, exception): + log.append('process-exception') + + +class ProcessExceptionExcMiddleware(BaseMiddleware): + def process_exception(self, request, exception): + raise Exception('from process-exception') + + +class ProcessViewMiddleware(BaseMiddleware): + def process_view(self, request, view_func, view_args, view_kwargs): + return HttpResponse('Processed view %s' % view_func.__name__) + + +class ProcessViewNoneMiddleware(BaseMiddleware): + def process_view(self, request, view_func, view_args, view_kwargs): + log.append('processed view %s' % view_func.__name__) + return None + + +class TemplateResponseMiddleware(BaseMiddleware): + def process_template_response(self, request, response): + response.template_name = engines['django'].from_string('template-response middleware') + return response + + +class LogMiddleware(BaseMiddleware): + def __call__(self, request): + response = self.get_response(request) + log.append((response.status_code, response.content)) + return response + + +class NotFoundMiddleware(BaseMiddleware): + def __call__(self, request): + raise Http404('not found') diff --git a/tests/middleware_exceptions/test_legacy.py b/tests/middleware_exceptions/test_legacy.py index 6aab3782d88..d59caa38824 100644 --- a/tests/middleware_exceptions/test_legacy.py +++ b/tests/middleware_exceptions/test_legacy.py @@ -1,15 +1,15 @@ import sys -from django.conf import settings -from django.core.exceptions import MiddlewareNotUsed from django.core.signals import got_request_exception from django.http import HttpResponse from django.template import engines from django.template.response import TemplateResponse -from django.test import RequestFactory, SimpleTestCase, override_settings -from django.test.utils import ignore_warnings, patch_logger +from django.test import SimpleTestCase, override_settings +from django.test.utils import ignore_warnings from django.utils.deprecation import MiddlewareMixin, RemovedInDjango20Warning +from .tests import MiddlewareNotUsedTests + class TestException(Exception): pass @@ -512,14 +512,6 @@ class MiddlewareTests(BaseMiddlewareExceptionTest): # Check that the right middleware methods have been invoked self.assert_middleware_usage(middleware, True, True, True, True, False) - @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.ProcessExceptionMiddleware']) - def test_exception_in_render_passed_to_process_exception(self): - # Repopulate the list of middlewares since it's already been populated - # by setUp() before the MIDDLEWARE setting got overridden. - self.client.handler.load_middleware() - response = self.client.get('/middleware_exceptions/exception_in_render/') - self.assertEqual(response.content, b'Exception caught') - class BadMiddlewareTests(BaseMiddlewareExceptionTest): @@ -869,79 +861,6 @@ class BadMiddlewareTests(BaseMiddlewareExceptionTest): self.assert_middleware_usage(middleware, True, True, True, True, False) self.assert_middleware_usage(post_middleware, True, True, True, True, False) -_missing = object() - - -@override_settings(ROOT_URLCONF='middleware_exceptions.urls') -class RootUrlconfTests(SimpleTestCase): - - @override_settings(ROOT_URLCONF=None) - def test_missing_root_urlconf(self): - # Removing ROOT_URLCONF is safe, as override_settings will restore - # the previously defined settings. - del settings.ROOT_URLCONF - with self.assertRaises(AttributeError): - self.client.get("/middleware_exceptions/view/") - - -class MyMiddleware(object): - - def __init__(self, get_response=None): - raise MiddlewareNotUsed - - def process_request(self, request): - pass - - -class MyMiddlewareWithExceptionMessage(object): - - def __init__(self, get_response=None): - raise MiddlewareNotUsed('spam eggs') - - def process_request(self, request): - pass - - -@override_settings( - DEBUG=True, - ROOT_URLCONF='middleware_exceptions.urls', - MIDDLEWARE=['django.middleware.common.CommonMiddleware'], -) -class MiddlewareNotUsedTests(SimpleTestCase): - - rf = RequestFactory() - - def test_raise_exception(self): - request = self.rf.get('middleware_exceptions/view/') - with self.assertRaises(MiddlewareNotUsed): - MyMiddleware().process_request(request) - - @override_settings(MIDDLEWARE=['middleware_exceptions.test_legacy.MyMiddleware']) - def test_log(self): - with patch_logger('django.request', 'debug') as calls: - self.client.get('/middleware_exceptions/view/') - self.assertEqual(len(calls), 1) - self.assertEqual( - calls[0], - "MiddlewareNotUsed: 'middleware_exceptions.test_legacy.MyMiddleware'" - ) - - @override_settings(MIDDLEWARE=['middleware_exceptions.test_legacy.MyMiddlewareWithExceptionMessage']) - def test_log_custom_message(self): - with patch_logger('django.request', 'debug') as calls: - self.client.get('/middleware_exceptions/view/') - self.assertEqual(len(calls), 1) - self.assertEqual( - calls[0], - "MiddlewareNotUsed('middleware_exceptions.test_legacy.MyMiddlewareWithExceptionMessage'): spam eggs" - ) - - @override_settings(DEBUG=False) - def test_do_not_log_when_debug_is_false(self): - with patch_logger('django.request', 'debug') as calls: - self.client.get('/middleware_exceptions/view/') - self.assertEqual(len(calls), 0) - @ignore_warnings(category=RemovedInDjango20Warning) @override_settings( diff --git a/tests/middleware_exceptions/tests.py b/tests/middleware_exceptions/tests.py new file mode 100644 index 00000000000..f6a7e24e591 --- /dev/null +++ b/tests/middleware_exceptions/tests.py @@ -0,0 +1,133 @@ +from django.conf import settings +from django.core.exceptions import MiddlewareNotUsed +from django.test import RequestFactory, SimpleTestCase, override_settings +from django.test.utils import patch_logger + +from . import middleware as mw + + +@override_settings(ROOT_URLCONF='middleware_exceptions.urls') +class MiddlewareTests(SimpleTestCase): + def tearDown(self): + mw.log = [] + + @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.ProcessViewNoneMiddleware']) + def test_process_view_return_none(self): + response = self.client.get('/middleware_exceptions/view/') + self.assertEqual(mw.log, ['processed view normal_view']) + self.assertEqual(response.content, b'OK') + + @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.ProcessViewMiddleware']) + def test_process_view_return_response(self): + response = self.client.get('/middleware_exceptions/view/') + self.assertEqual(response.content, b'Processed view normal_view') + + @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.TemplateResponseMiddleware']) + def test_process_template_response(self): + response = self.client.get('/middleware_exceptions/template_response/') + self.assertEqual(response.content, b'template-response middleware') + + @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.LogMiddleware']) + def test_view_exception_converted_before_middleware(self): + response = self.client.get('/middleware_exceptions/permission_denied/') + self.assertEqual(mw.log, [(response.status_code, response.content)]) + self.assertEqual(response.status_code, 403) + + @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.ProcessExceptionMiddleware']) + def test_view_exception_handled_by_process_exception(self): + response = self.client.get('/middleware_exceptions/error/') + self.assertEqual(response.content, b'Exception caught') + + @override_settings(MIDDLEWARE=[ + 'middleware_exceptions.middleware.ProcessExceptionLogMiddleware', + 'middleware_exceptions.middleware.ProcessExceptionMiddleware', + ]) + def test_response_from_process_exception_short_circuits_remainder(self): + response = self.client.get('/middleware_exceptions/error/') + self.assertEqual(mw.log, []) + self.assertEqual(response.content, b'Exception caught') + + @override_settings(MIDDLEWARE=[ + 'middleware_exceptions.middleware.LogMiddleware', + 'middleware_exceptions.middleware.NotFoundMiddleware', + ]) + def test_exception_in_middleware_converted_before_prior_middleware(self): + response = self.client.get('/middleware_exceptions/view/') + self.assertEqual(mw.log, [(404, response.content)]) + self.assertEqual(response.status_code, 404) + + @override_settings(MIDDLEWARE=['middleware_exceptions.middleware.ProcessExceptionMiddleware']) + def test_exception_in_render_passed_to_process_exception(self): + response = self.client.get('/middleware_exceptions/exception_in_render/') + self.assertEqual(response.content, b'Exception caught') + + +@override_settings(ROOT_URLCONF='middleware_exceptions.urls') +class RootUrlconfTests(SimpleTestCase): + + @override_settings(ROOT_URLCONF=None) + def test_missing_root_urlconf(self): + # Removing ROOT_URLCONF is safe, as override_settings will restore + # the previously defined settings. + del settings.ROOT_URLCONF + with self.assertRaises(AttributeError): + self.client.get("/middleware_exceptions/view/") + + +class MyMiddleware(object): + + def __init__(self, get_response=None): + raise MiddlewareNotUsed + + def process_request(self, request): + pass + + +class MyMiddlewareWithExceptionMessage(object): + + def __init__(self, get_response=None): + raise MiddlewareNotUsed('spam eggs') + + def process_request(self, request): + pass + + +@override_settings( + DEBUG=True, + ROOT_URLCONF='middleware_exceptions.urls', + MIDDLEWARE=['django.middleware.common.CommonMiddleware'], +) +class MiddlewareNotUsedTests(SimpleTestCase): + + rf = RequestFactory() + + def test_raise_exception(self): + request = self.rf.get('middleware_exceptions/view/') + with self.assertRaises(MiddlewareNotUsed): + MyMiddleware().process_request(request) + + @override_settings(MIDDLEWARE=['middleware_exceptions.tests.MyMiddleware']) + def test_log(self): + with patch_logger('django.request', 'debug') as calls: + self.client.get('/middleware_exceptions/view/') + self.assertEqual(len(calls), 1) + self.assertEqual( + calls[0], + "MiddlewareNotUsed: 'middleware_exceptions.tests.MyMiddleware'" + ) + + @override_settings(MIDDLEWARE=['middleware_exceptions.tests.MyMiddlewareWithExceptionMessage']) + def test_log_custom_message(self): + with patch_logger('django.request', 'debug') as calls: + self.client.get('/middleware_exceptions/view/') + self.assertEqual(len(calls), 1) + self.assertEqual( + calls[0], + "MiddlewareNotUsed('middleware_exceptions.tests.MyMiddlewareWithExceptionMessage'): spam eggs" + ) + + @override_settings(DEBUG=False) + def test_do_not_log_when_debug_is_false(self): + with patch_logger('django.request', 'debug') as calls: + self.client.get('/middleware_exceptions/view/') + self.assertEqual(len(calls), 0)