Fixed #29642 -- Added check for arguments of custom error handler views.

This commit is contained in:
Adam Johnson 2018-09-07 08:33:19 +01:00 committed by Tim Graham
parent d483a5f0dc
commit 245c36d7b6
5 changed files with 73 additions and 5 deletions

View File

@ -6,13 +6,14 @@ a string) and returns a ResolverMatch object which provides access to all
attributes of the resolved URL match.
"""
import functools
import inspect
import re
import threading
from importlib import import_module
from urllib.parse import quote
from django.conf import settings
from django.core.checks import Warning
from django.core.checks import Error, Warning
from django.core.checks.urls import check_resolver
from django.core.exceptions import ImproperlyConfigured
from django.utils.datastructures import MultiValueDict
@ -392,10 +393,33 @@ class URLResolver:
)
def check(self):
warnings = []
messages = []
for pattern in self.url_patterns:
warnings.extend(check_resolver(pattern))
return warnings or self.pattern.check()
messages.extend(check_resolver(pattern))
messages.extend(self._check_custom_error_handlers())
return messages or self.pattern.check()
def _check_custom_error_handlers(self):
messages = []
# All handlers take (request, exception) arguments except handler500
# which takes (request).
for status_code, num_parameters in [(400, 2), (403, 2), (404, 2), (500, 1)]:
handler, param_dict = self.resolve_error_handler(status_code)
signature = inspect.signature(handler)
args = [None] * num_parameters
try:
signature.bind(*args)
except TypeError:
msg = (
"The custom handler{status_code} view '{path}' does not "
"take the correct number of arguments ({args})."
).format(
status_code=status_code,
path=handler.__module__ + '.' + handler.__qualname__,
args='request, exception' if num_parameters == 2 else 'request',
)
messages.append(Error(msg, id='urls.E007'))
return messages
def _populate(self):
# Short-circuit if called recursively in this thread to prevent

View File

@ -457,6 +457,8 @@ The following checks are performed on your URL configuration:
able to reverse all URLs in this namespace.
* **urls.E006**: The :setting:`MEDIA_URL`/ :setting:`STATIC_URL` setting must
end with a slash.
* **urls.E007**: The custom ``handlerXXX`` view ``'path.to.view'`` does not
take the correct number of arguments (…).
``contrib`` app checks
======================

View File

@ -1,5 +1,5 @@
from django.conf import settings
from django.core.checks.messages import Warning
from django.core.checks.messages import Error, Warning
from django.core.checks.urls import (
E006, check_url_config, check_url_namespaces_unique, check_url_settings,
get_warning_for_invalid_pattern,
@ -165,6 +165,28 @@ class UpdatedToPathTests(SimpleTestCase):
self.assertIn(expected_msg, warning.msg)
class CheckCustomErrorHandlersTests(SimpleTestCase):
@override_settings(ROOT_URLCONF='check_framework.urls.bad_error_handlers')
def test_bad_handlers(self):
result = check_url_config(None)
self.assertEqual(len(result), 4)
for code, num_params, error in zip([400, 403, 404, 500], [2, 2, 2, 1], result):
with self.subTest('handler{}'.format(code)):
self.assertEqual(error, Error(
"The custom handler{} view "
"'check_framework.urls.bad_error_handlers.bad_handler' "
"does not take the correct number of arguments (request{})."
.format(code, ', exception' if num_params == 2 else ''),
id='urls.E007',
))
@override_settings(ROOT_URLCONF='check_framework.urls.good_error_handlers')
def test_good_handlers(self):
result = check_url_config(None)
self.assertEqual(result, [])
class CheckURLSettingsTests(SimpleTestCase):
@override_settings(STATIC_URL='a/', MEDIA_URL='b/')

View File

@ -0,0 +1,10 @@
urlpatterns = []
handler400 = __name__ + '.bad_handler'
handler403 = __name__ + '.bad_handler'
handler404 = __name__ + '.bad_handler'
handler500 = __name__ + '.bad_handler'
def bad_handler():
pass

View File

@ -0,0 +1,10 @@
urlpatterns = []
handler400 = __name__ + '.good_handler'
handler403 = __name__ + '.good_handler'
handler404 = __name__ + '.good_handler'
handler500 = __name__ + '.good_handler'
def good_handler(request, exception=None, foo='bar'):
pass