diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index a4edf876bf..cad2ba2a60 100644 --- a/django/core/handlers/base.py +++ b/django/core/handlers/base.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import logging import sys import types +import warnings from django import http from django.conf import settings @@ -13,6 +14,7 @@ from django.core.exceptions import ( 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 @@ class BaseHandler(object): 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 @@ class BaseHandler(object): 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 @@ class BaseHandler(object): 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 diff --git a/django/views/defaults.py b/django/views/defaults.py index 35d42830a4..d4651e6665 100644 --- a/django/views/defaults.py +++ b/django/views/defaults.py @@ -1,5 +1,7 @@ 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 @@ -7,7 +9,7 @@ from django.views.decorators.csrf import requires_csrf_token # 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. @@ -15,8 +17,24 @@ def page_not_found(request, template_name='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,6 +75,7 @@ def bad_request(request, template_name='400.html'): template = loader.get_template(template_name) except TemplateDoesNotExist: return http.HttpResponseBadRequest('

Bad Request (400)

', content_type='text/html') + # No exception content is passed to the template, to not disclose any sensitive information. return http.HttpResponseBadRequest(template.render()) @@ -64,7 +83,7 @@ def bad_request(request, template_name='400.html'): # 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('

403 Forbidden

', content_type='text/html') - return http.HttpResponseForbidden(template.render(request=request)) + return http.HttpResponseForbidden( + template.render(request=request, context={'exception': force_text(exception)}) + ) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 987dc2b580..6580d2796c 100644 --- a/docs/internals/deprecation.txt +++ b/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 diff --git a/docs/ref/views.txt b/docs/ref/views.txt index d15e4446f2..133a79814b 100644 --- a/docs/ref/views.txt +++ b/docs/ref/views.txt @@ -61,7 +61,7 @@ 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 @@ -69,8 +69,10 @@ special view devoted to handling 404 errors. By default, it's the view 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,7 +118,7 @@ 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 @@ -118,7 +126,9 @@ 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. diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 8be0e334b8..25868dafd3 100644 --- a/docs/releases/1.9.txt +++ b/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 `, 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 diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 17c4f6e359..a98b776f78 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -81,7 +81,7 @@ class DebugViewTests(LoggingCaptureMixin, TestCase): '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 }}).', }), ], }, @@ -89,6 +89,7 @@ class DebugViewTests(LoggingCaptureMixin, TestCase): 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/') diff --git a/tests/view_tests/tests/test_defaults.py b/tests/view_tests/tests/test_defaults.py index 0f3f4fe014..a729276887 100644 --- a/tests/view_tests/tests/test_defaults.py +++ b/tests/view_tests/tests/test_defaults.py @@ -79,7 +79,8 @@ class DefaultsTests(TestCase): '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 @@ class DefaultsTests(TestCase): 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" diff --git a/tests/view_tests/urls.py b/tests/view_tests/urls.py index 1d34b1f934..4215a4b26a 100644 --- a/tests/view_tests/urls.py +++ b/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 @@ urlpatterns = [ 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 diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py index c4d025c349..7996193b2e 100644 --- a/tests/view_tests/views.py +++ b/tests/view_tests/views.py @@ -52,7 +52,7 @@ def raises400(request): def raises403(request): - raise PermissionDenied + raise PermissionDenied("Insufficient Permissions") def raises404(request):