From e7fa89fb58cc7ba468e0167f506dc4fce7ec532a Mon Sep 17 00:00:00 2001 From: Lucas Lois Date: Wed, 5 Oct 2016 16:34:26 -0300 Subject: [PATCH] Fixed #27262 -- Moved URL checks to resolver and pattern classes. Thanks Sjoerd Job Postmus for the report and review. --- django/core/checks/urls.py | 91 ++++---------------------------------- django/urls/resolvers.py | 77 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 83 deletions(-) diff --git a/django/core/checks/urls.py b/django/core/checks/urls.py index 7106478642..a1e9929084 100644 --- a/django/core/checks/urls.py +++ b/django/core/checks/urls.py @@ -3,7 +3,7 @@ from __future__ import unicode_literals from django.conf import settings from django.utils import six -from . import Error, Tags, Warning, register +from . import Error, Tags, register @register(Tags.urls) @@ -19,23 +19,13 @@ def check_resolver(resolver): """ Recursively check the resolver. """ - from django.urls import RegexURLPattern, RegexURLResolver - warnings = [] - for pattern in resolver.url_patterns: - if isinstance(pattern, RegexURLResolver): - warnings.extend(check_include_trailing_dollar(pattern)) - # Check resolver recursively - warnings.extend(check_resolver(pattern)) - elif isinstance(pattern, RegexURLPattern): - warnings.extend(check_pattern_name(pattern)) - else: - # This is not a url() instance - warnings.extend(get_warning_for_invalid_pattern(pattern)) - - if not warnings: - warnings.extend(check_pattern_startswith_slash(pattern)) - - return warnings + check_method = getattr(resolver, 'check', None) + if check_method is not None: + return check_method() + elif not hasattr(resolver, 'resolve'): + return get_warning_for_invalid_pattern(resolver) + else: + return [] def get_warning_for_invalid_pattern(pattern): @@ -61,68 +51,3 @@ def get_warning_for_invalid_pattern(pattern): hint=hint, id="urls.E004", )] - - -def describe_pattern(pattern): - """ - Format the URL pattern for display in warning messages. - """ - description = "'{}'".format(pattern.regex.pattern) - if getattr(pattern, 'name', False): - description += " [name='{}']".format(pattern.name) - return description - - -def check_include_trailing_dollar(pattern): - """ - Check that include is not used with a regex ending with a dollar. - """ - regex_pattern = pattern.regex.pattern - if regex_pattern.endswith('$') and not regex_pattern.endswith(r'\$'): - warning = Warning( - "Your URL pattern {} uses include with a regex ending with a '$'. " - "Remove the dollar from the regex to avoid problems including " - "URLs.".format(describe_pattern(pattern)), - id="urls.W001", - ) - return [warning] - else: - return [] - - -def check_pattern_startswith_slash(pattern): - """ - Check that the pattern does not begin with a forward slash. - """ - regex_pattern = pattern.regex.pattern - if not settings.APPEND_SLASH: - # Skip check as it can be useful to start a URL pattern with a slash - # when APPEND_SLASH=False. - return [] - if regex_pattern.startswith('/') or regex_pattern.startswith('^/'): - warning = Warning( - "Your URL pattern {} has a regex beginning with a '/'. Remove this " - "slash as it is unnecessary. If this pattern is targeted in an " - "include(), ensure the include() pattern has a trailing '/'.".format( - describe_pattern(pattern) - ), - id="urls.W002", - ) - return [warning] - else: - return [] - - -def check_pattern_name(pattern): - """ - Check that the pattern name does not contain a colon. - """ - if pattern.name is not None and ":" in pattern.name: - warning = Warning( - "Your URL pattern {} has a name including a ':'. Remove the colon, to " - "avoid ambiguous namespace references.".format(describe_pattern(pattern)), - id="urls.W003", - ) - return [warning] - else: - return [] diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index cec960dc1b..3c4f18c991 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -13,6 +13,8 @@ import threading from importlib import import_module from django.conf import settings +from django.core.checks import Warning +from django.core.checks.urls import check_resolver from django.core.exceptions import ImproperlyConfigured from django.utils import lru_cache, six from django.utils.datastructures import MultiValueDict @@ -107,6 +109,37 @@ class LocaleRegexProvider(object): self._regex_dict[language_code] = compiled_regex return self._regex_dict[language_code] + def describe(self): + """ + Format the URL pattern for display in warning messages. + """ + description = "'{}'".format(self.regex.pattern) + if getattr(self, 'name', False): + description += " [name='{}']".format(self.name) + return description + + def _check_pattern_startswith_slash(self): + """ + Check that the pattern does not begin with a forward slash. + """ + regex_pattern = self.regex.pattern + if not settings.APPEND_SLASH: + # Skip check as it can be useful to start a URL pattern with a slash + # when APPEND_SLASH=False. + return [] + if (regex_pattern.startswith('/') or regex_pattern.startswith('^/')) and not regex_pattern.endswith('/'): + warning = Warning( + "Your URL pattern {} has a regex beginning with a '/'. Remove this " + "slash as it is unnecessary. If this pattern is targeted in an " + "include(), ensure the include() pattern has a trailing '/'.".format( + self.describe() + ), + id="urls.W002", + ) + return [warning] + else: + return [] + class RegexURLPattern(LocaleRegexProvider): def __init__(self, regex, callback, default_args=None, name=None): @@ -118,6 +151,26 @@ class RegexURLPattern(LocaleRegexProvider): def __repr__(self): return force_str('<%s %s %s>' % (self.__class__.__name__, self.name, self.regex.pattern)) + def check(self): + warnings = self._check_pattern_name() + if not warnings: + warnings = self._check_pattern_startswith_slash() + return warnings + + def _check_pattern_name(self): + """ + Check that the pattern name does not contain a colon. + """ + if self.name is not None and ":" in self.name: + warning = Warning( + "Your URL pattern {} has a name including a ':'. Remove the colon, to " + "avoid ambiguous namespace references.".format(self.describe()), + id="urls.W003", + ) + return [warning] + else: + return [] + def resolve(self, path): match = self.regex.search(path) if match: @@ -181,6 +234,30 @@ class RegexURLResolver(LocaleRegexProvider): self.namespace, self.regex.pattern, ) + def check(self): + warnings = self._check_include_trailing_dollar() + for pattern in self.url_patterns: + warnings.extend(check_resolver(pattern)) + if not warnings: + warnings = self._check_pattern_startswith_slash() + return warnings + + def _check_include_trailing_dollar(self): + """ + Check that include is not used with a regex ending with a dollar. + """ + regex_pattern = self.regex.pattern + if regex_pattern.endswith('$') and not regex_pattern.endswith(r'\$'): + warning = Warning( + "Your URL pattern {} uses include with a regex ending with a '$'. " + "Remove the dollar from the regex to avoid problems including " + "URLs.".format(self.describe()), + id="urls.W001", + ) + return [warning] + else: + return [] + def _populate(self): # Short-circuit if called recursively in this thread to prevent # infinite recursion. Concurrent threads may call this at the same