Fixed #24733 -- Passed the triggering exception to 40x error handlers

Thanks Tim Graham for the review.

django/core/handlers/base.py

@@ -3,6 +3,7 @@
 import logging
 import sys
 import types
+import warnings
 
 from django import http
 from django.conf import settings
@@ -13,6 +14,7 @@
 from django.db import connections, transaction
 from django.http.multipartparser import MultiPartParserError
 from django.utils import six
+from django.utils.deprecation import RemovedInDjango21Warning
 from django.utils.encoding import force_text
 from django.utils.module_loading import import_string
 from django.views import debug
@@ -80,10 +82,21 @@ def make_view_atomic(self, view):
                 view = transaction.atomic(using=db.alias)(view)
         return view
 
-    def get_exception_response(self, request, resolver, status_code):
+    def get_exception_response(self, request, resolver, status_code, exception):
         try:
             callback, param_dict = resolver.resolve_error_handler(status_code)
-            response = callback(request, **param_dict)
+            # 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.1",
+                    RemovedInDjango21Warning, stacklevel=2
+                )
+                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())
@@ -171,25 +184,25 @@ def get_response(self, request):
             if settings.DEBUG:
                 response = debug.technical_404_response(request, exc)
             else:
-                response = self.get_exception_response(request, resolver, 404)
+                response = self.get_exception_response(request, resolver, 404, exc)
 
-        except PermissionDenied:
+        except PermissionDenied as exc:
             logger.warning(
                 'Forbidden (Permission denied): %s', request.path,
                 extra={
                     'status_code': 403,
                     'request': request
                 })
-            response = self.get_exception_response(request, resolver, 403)
+            response = self.get_exception_response(request, resolver, 403, exc)
 
-        except MultiPartParserError:
+        except MultiPartParserError as exc:
             logger.warning(
                 'Bad request (Unable to parse request body): %s', request.path,
                 extra={
                     'status_code': 400,
                     'request': request
                 })
-            response = self.get_exception_response(request, resolver, 400)
+            response = self.get_exception_response(request, resolver, 400, exc)
 
         except SuspiciousOperation as exc:
             # The request logger receives events for any problematic request
@@ -205,7 +218,7 @@ def get_response(self, request):
             if settings.DEBUG:
                 return debug.technical_500_response(request, *sys.exc_info(), status_code=400)
 
-            response = self.get_exception_response(request, resolver, 400)
+            response = self.get_exception_response(request, resolver, 400, exc)
 
         except SystemExit:
             # Allow sys.exit() to actually exit. See tickets #1023 and #4701

django/views/defaults.py

@@ -1,22 +1,40 @@
 from django import http
 from django.template import Context, Engine, TemplateDoesNotExist, loader
+from django.utils import six
+from django.utils.encoding import force_text
 from django.views.decorators.csrf import requires_csrf_token
 
 
 # This can be called when CsrfViewMiddleware.process_view has not run,
 # therefore need @requires_csrf_token in case the template needs
 # {% csrf_token %}.
 @requires_csrf_token
-def page_not_found(request, template_name='404.html'):
+def page_not_found(request, exception, template_name='404.html'):
     """
     Default 404 handler.
 
     Templates: :template:`404.html`
     Context:
         request_path
             The path of the requested URL (e.g., '/app/pages/bad_page/')
+        exception
+            The message from the exception which triggered the 404 (if one was
+            supplied), or the exception class name
     """
-    context = {'request_path': request.path}
+    exception_repr = exception.__class__.__name__
+    # Try to get an "interesting" exception message, if any (and not the ugly
+    # Resolver404 dictionary)
+    try:
+        message = exception.args[0]
+    except (AttributeError, IndexError):
+        pass
+    else:
+        if isinstance(message, six.text_type):
+            exception_repr = message
+    context = {
+        'request_path': request.path,
+        'exception': exception_repr,
+    }
     try:
         template = loader.get_template(template_name)
         body = template.render(context, request)
@@ -46,7 +64,7 @@ def server_error(request, template_name='500.html'):
 
 
 @requires_csrf_token
-def bad_request(request, template_name='400.html'):
+def bad_request(request, exception, template_name='400.html'):
     """
     400 error handler.
 
@@ -57,14 +75,15 @@ def bad_request(request, template_name='400.html'):
         template = loader.get_template(template_name)
     except TemplateDoesNotExist:
         return http.HttpResponseBadRequest('<h1>Bad Request (400)</h1>', content_type='text/html')
+    # No exception content is passed to the template, to not disclose any sensitive information.
     return http.HttpResponseBadRequest(template.render())
 
 
 # This can be called when CsrfViewMiddleware.process_view has not run,
 # therefore need @requires_csrf_token in case the template needs
 # {% csrf_token %}.
 @requires_csrf_token
-def permission_denied(request, template_name='403.html'):
+def permission_denied(request, exception, template_name='403.html'):
     """
     Permission denied (403) handler.
 
@@ -78,4 +97,6 @@ def permission_denied(request, template_name='403.html'):
         template = loader.get_template(template_name)
     except TemplateDoesNotExist:
         return http.HttpResponseForbidden('<h1>403 Forbidden</h1>', content_type='text/html')
-    return http.HttpResponseForbidden(template.render(request=request))
+    return http.HttpResponseForbidden(
+        template.render(request=request, context={'exception': force_text(exception)})
+    )

docs/internals/deprecation.txt

@@ -58,6 +58,9 @@ details on these changes.
 * The ``django.template.loaders.base.Loader.__call__()`` method will be
   removed.
 
+* Support for custom error views with a single positional parameter will be
+  dropped.
+
 .. _deprecation-removed-in-2.0:
 
 2.0

docs/ref/views.txt

@@ -61,16 +61,18 @@ these with your own custom views, see :ref:`customizing-error-views`.
 The 404 (page not found) view
 -----------------------------
 
-.. function:: defaults.page_not_found(request, template_name='404.html')
+.. function:: defaults.page_not_found(request, exception, template_name='404.html')
 
 When you raise :exc:`~django.http.Http404` from within a view, Django loads a
 special view devoted to handling 404 errors. By default, it's the view
 :func:`django.views.defaults.page_not_found`, which either produces a very
 simple "Not Found" message or loads and renders the template ``404.html`` if
 you created it in your root template directory.
 
-The default 404 view will pass one variable to the template: ``request_path``,
-which is the URL that resulted in the error.
+The default 404 view will pass two variables to the template: ``request_path``,
+which is the URL that resulted in the error, and ``exception``, which is a
+useful representation of the exception that triggered the view (e.g. containing
+any message passed to a specific ``Http404`` instance).
 
 Three things to note about 404 views:
 
@@ -85,6 +87,12 @@ Three things to note about 404 views:
   your 404 view will never be used, and your URLconf will be displayed
   instead, with some debug information.
 
+.. versionchanged:: 1.9
+
+    The signature of ``page_not_found()`` changed. The function now accepts a
+    second parameter, the exception that triggered the error. A useful
+    representation of the exception is also passed in the template context.
+
 .. _http_internal_server_error_view:
 
 The 500 (server error) view
@@ -110,15 +118,17 @@ instead, with some debug information.
 The 403 (HTTP Forbidden) view
 -----------------------------
 
-.. function:: defaults.permission_denied(request, template_name='403.html')
+.. function:: defaults.permission_denied(request, exception, template_name='403.html')
 
 In the same vein as the 404 and 500 views, Django has a view to handle 403
 Forbidden errors. If a view results in a 403 exception then Django will, by
 default, call the view ``django.views.defaults.permission_denied``.
 
 This view loads and renders the template ``403.html`` in your root template
 directory, or if this file does not exist, instead serves the text
-"403 Forbidden", as per :rfc:`2616` (the HTTP 1.1 Specification).
+"403 Forbidden", as per :rfc:`2616` (the HTTP 1.1 Specification). The template
+context contains ``exception``, which is the unicode representation of the
+exception that triggered the view.
 
 ``django.views.defaults.permission_denied`` is triggered by a
 :exc:`~django.core.exceptions.PermissionDenied` exception. To deny access in a
@@ -131,12 +141,19 @@ view you can use code like this::
             raise PermissionDenied
         # ...
 
+.. versionchanged:: 1.9
+
+    The signature of ``permission_denied()`` changed in Django 1.9. The function
+    now accepts a second parameter, the exception that triggered the error. The
+    unicode representation of the exception is also passed in the template
+    context.
+
 .. _http_bad_request_view:
 
 The 400 (bad request) view
 --------------------------
 
-.. function:: defaults.bad_request(request, template_name='400.html')
+.. function:: defaults.bad_request(request, exception, template_name='400.html')
 
 When a :exc:`~django.core.exceptions.SuspiciousOperation` is raised in Django,
 it may be handled by a component of Django (for example resetting the session
@@ -145,6 +162,14 @@ data). If not specifically handled, Django will consider the current request a
 
 ``django.views.defaults.bad_request``, is otherwise very similar to the
 ``server_error`` view, but returns with the status code 400 indicating that
-the error condition was the result of a client operation.
+the error condition was the result of a client operation. By default, nothing
+related to the exception that triggered the view is passed to the template
+context, as the exception message might contain sensitive information like
+filesystem paths.
 
 ``bad_request`` views are also only used when :setting:`DEBUG` is ``False``.
+
+.. versionchanged:: 1.9
+
+    The signature of ``bad_request()`` changed in Django 1.9. The function
+    now accepts a second parameter, the exception that triggered the error.

docs/releases/1.9.txt

@@ -269,6 +269,9 @@ Requests and Responses
 
 * The debug view now shows details of chained exceptions on Python 3.
 
+* The default 40x error views now accept a second positional parameter, the
+  exception that triggered the view.
+
 Tests
 ^^^^^
 
@@ -520,6 +523,10 @@ Miscellaneous
   Therefore, the ``@skipIfCustomUser`` decorator is no longer needed to
   decorate tests in ``django.contrib.auth``.
 
+* If you customized some :ref:`error handlers <error-views>`, the view
+  signatures with only one request parameter are deprecated. The views should
+  now also accept a second ``exception`` positional parameter.
+
 .. removed-features-1.9:
 
 Features removed in 1.9

tests/view_tests/tests/test_debug.py

@@ -81,14 +81,15 @@ def test_403(self):
         'OPTIONS': {
             'loaders': [
                 ('django.template.loaders.locmem.Loader', {
-                    '403.html': 'This is a test template for a 403 error.',
+                    '403.html': 'This is a test template for a 403 error ({{ exception }}).',
                 }),
             ],
         },
     }])
     def test_403_template(self):
         response = self.client.get('/raises403/')
         self.assertContains(response, 'test template', status_code=403)
+        self.assertContains(response, '(Insufficient Permissions).', status_code=403)
 
     def test_404(self):
         response = self.client.get('/raises404/')

tests/view_tests/tests/test_defaults.py

@@ -79,7 +79,8 @@ def test_server_error(self):
         'OPTIONS': {
             'loaders': [
                 ('django.template.loaders.locmem.Loader', {
-                    '404.html': 'This is a test template for a 404 error.',
+                    '404.html': 'This is a test template for a 404 error '
+                                '(path: {{ request_path }}, exception: {{ exception }}).',
                     '500.html': 'This is a test template for a 500 error.',
                 }),
             ],
@@ -90,10 +91,13 @@ def test_custom_templates(self):
         Test that 404.html and 500.html templates are picked by their respective
         handler.
         """
-        for code, url in ((404, '/non_existing_url/'), (500, '/server_error/')):
-            response = self.client.get(url)
-            self.assertContains(response, "test template for a %d error" % code,
-                status_code=code)
+        response = self.client.get('/server_error/')
+        self.assertContains(response, "test template for a 500 error", status_code=500)
+        response = self.client.get('/no_such_url/')
+        self.assertContains(response, 'path: /no_such_url/', status_code=404)
+        self.assertContains(response, 'exception: Resolver404', status_code=404)
+        response = self.client.get('/technical404/')
+        self.assertContains(response, 'exception: Testing technical 404.', status_code=404)
 
     def test_get_absolute_url_attributes(self):
         "A model can set attributes on the get_absolute_url method"

tests/view_tests/urls.py

@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+from functools import partial
 from os import path
 
 from django.conf.urls import include, url
@@ -57,7 +58,7 @@
     url(r'^$', views.index_page),
 
     # Default views
-    url(r'^non_existing_url/', defaults.page_not_found),
+    url(r'^non_existing_url/', partial(defaults.page_not_found, exception=None)),
     url(r'^server_error/', defaults.server_error),
 
     # a view that raises an exception for the debug view

tests/view_tests/views.py

@@ -52,7 +52,7 @@ def raises400(request):
 
 
 def raises403(request):
-    raise PermissionDenied
+    raise PermissionDenied("Insufficient Permissions")
 
 
 def raises404(request):