p15693087
/
django
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Thanks Tim Graham for the review.
This commit is contained in:
Claude Paroz 2015-04-21 21:54:00 +02:00
parent bd53db5eab
commit 70779d9c1c
9 changed files with 103 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
django/core/handlers/base.py
View File

@ -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

31
django/views/defaults.py
View File

@ -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('<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())
@ -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('<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)})
)

3
docs/internals/deprecation.txt
View File

@ -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

39
docs/ref/views.txt
View File

@ -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.

7
docs/releases/1.9.txt
View File

@ -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

3
tests/view_tests/tests/test_debug.py
View File

@ -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/')

14
tests/view_tests/tests/test_defaults.py
View File

@ -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"

3
tests/view_tests/urls.py
View File

@ -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

2
tests/view_tests/views.py
View File

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