summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--django/core/checks/__init__.py2
-rw-r--r--django/core/checks/registry.py1
-rw-r--r--django/core/checks/urls.py87
-rw-r--r--django/core/urlresolvers.py2
-rw-r--r--docs/ref/checks.txt16
-rw-r--r--docs/releases/1.9.txt2
-rw-r--r--tests/check_framework/test_urls.py37
-rw-r--r--tests/check_framework/urls_include.py7
-rw-r--r--tests/check_framework/urls_name.py13
-rw-r--r--tests/check_framework/urls_no_warnings.py15
-rw-r--r--tests/check_framework/urls_slash.py13
11 files changed, 194 insertions, 1 deletions
diff --git a/django/core/checks/__init__.py b/django/core/checks/__init__.py
index dec2446ae2..cd21744327 100644
--- a/django/core/checks/__init__.py
+++ b/django/core/checks/__init__.py
@@ -15,6 +15,8 @@ import django.core.checks.security.base # NOQA isort:skip
import django.core.checks.security.csrf # NOQA isort:skip
import django.core.checks.security.sessions # NOQA isort:skip
import django.core.checks.templates # NOQA isort:skip
+import django.core.checks.urls # NOQA isort:skip
+
__all__ = [
'CheckMessage',
diff --git a/django/core/checks/registry.py b/django/core/checks/registry.py
index 96df429dc9..3e3d45fff8 100644
--- a/django/core/checks/registry.py
+++ b/django/core/checks/registry.py
@@ -17,6 +17,7 @@ class Tags(object):
security = 'security'
signals = 'signals'
templates = 'templates'
+ urls = 'urls'
class CheckRegistry(object):
diff --git a/django/core/checks/urls.py b/django/core/checks/urls.py
new file mode 100644
index 0000000000..a0d5b4a825
--- /dev/null
+++ b/django/core/checks/urls.py
@@ -0,0 +1,87 @@
+from __future__ import unicode_literals
+
+from . import Tags, Warning, register
+
+
+@register(Tags.urls)
+def check_url_config(app_configs, **kwargs):
+ from django.core.urlresolvers import get_resolver
+ resolver = get_resolver()
+ return check_resolver(resolver)
+
+
+def check_resolver(resolver):
+ """
+ Recursively check the resolver
+ """
+ from django.core.urlresolvers 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))
+
+ warnings.extend(check_pattern_startswith_slash(pattern))
+
+ return warnings
+
+
+def describe_pattern(pattern):
+ """
+ Formats 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):
+ """
+ Checks 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('\$'):
+ 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):
+ """
+ Checks that the pattern does not begin with a forward slash
+ """
+ regex_pattern = pattern.regex.pattern
+ 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.".format(describe_pattern(pattern)),
+ id="urls.W002",
+ )
+ return [warning]
+ else:
+ return []
+
+
+def check_pattern_name(pattern):
+ """
+ Checks 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/core/urlresolvers.py b/django/core/urlresolvers.py
index e462c343dc..3554878865 100644
--- a/django/core/urlresolvers.py
+++ b/django/core/urlresolvers.py
@@ -146,7 +146,7 @@ def get_callable(lookup_view, can_fail=False):
@lru_cache.lru_cache(maxsize=None)
-def get_resolver(urlconf):
+def get_resolver(urlconf=None):
if urlconf is None:
from django.conf import settings
urlconf = settings.ROOT_URLCONF
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index 39f3a06089..208e3fb81f 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -81,6 +81,7 @@ Django's system checks are organized using the following tags:
* ``security``: Checks security related configuration.
* ``templates``: Checks template related configuration.
* ``caches``: Checks cache related configuration.
+* ``urls``: Checks URL configuration.
Some checks may be registered with multiple tags.
@@ -580,3 +581,18 @@ configured:
* **caches.E001**: You must define a ``'default'`` cache in your
:setting:`CACHES` setting.
+
+URLs
+----
+
+The following checks are performed on your URL configuration:
+
+* **urls.W001**: Your URL pattern ``<pattern>`` uses
+ :func:`~django.conf.urls.include` with a ``regex`` ending with a
+ ``$``. Remove the dollar from the ``regex`` to avoid problems
+ including URLs.
+* **urls.W002**: Your URL pattern ``<pattern>`` has a ``regex``
+ beginning with a ``/``. Remove this slash as it is unnecessary.
+* **urls.W003**: Your URL pattern ``<pattern>`` has a ``name``
+ including a ``:``. Remove the colon, to avoid ambiguous namespace
+ references.
diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt
index 18fa8ef6bf..f1eba56a1e 100644
--- a/docs/releases/1.9.txt
+++ b/docs/releases/1.9.txt
@@ -641,6 +641,8 @@ URLs
of (<list of patterns>, <application namespace>) as the first argument to
:func:`~django.conf.urls.include`.
+* System checks have been added for common URL pattern mistakes.
+
Validators
^^^^^^^^^^
diff --git a/tests/check_framework/test_urls.py b/tests/check_framework/test_urls.py
new file mode 100644
index 0000000000..2a05146e77
--- /dev/null
+++ b/tests/check_framework/test_urls.py
@@ -0,0 +1,37 @@
+from django.core.checks.urls import check_url_config
+from django.test import SimpleTestCase
+from django.test.utils import override_settings
+
+
+class CheckUrlsTest(SimpleTestCase):
+ @override_settings(ROOT_URLCONF='check_framework.urls_no_warnings')
+ def test_include_no_warnings(self):
+ result = check_url_config(None)
+ self.assertEqual(result, [])
+
+ @override_settings(ROOT_URLCONF='check_framework.urls_include')
+ def test_include_with_dollar(self):
+ result = check_url_config(None)
+ self.assertEqual(len(result), 1)
+ warning = result[0]
+ self.assertEqual(warning.id, 'urls.W001')
+ expected_msg = "Your URL pattern '^include-with-dollar$' uses include with a regex ending with a '$'."
+ self.assertIn(expected_msg, warning.msg)
+
+ @override_settings(ROOT_URLCONF='check_framework.urls_slash')
+ def test_url_beginning_with_slash(self):
+ result = check_url_config(None)
+ self.assertEqual(len(result), 1)
+ warning = result[0]
+ self.assertEqual(warning.id, 'urls.W002')
+ expected_msg = "Your URL pattern '/starting-with-slash/$' has a regex beginning with a '/'"
+ self.assertIn(expected_msg, warning.msg)
+
+ @override_settings(ROOT_URLCONF='check_framework.urls_name')
+ def test_url_pattern_name_with_colon(self):
+ result = check_url_config(None)
+ self.assertEqual(len(result), 1)
+ warning = result[0]
+ self.assertEqual(warning.id, 'urls.W003')
+ expected_msg = "Your URL pattern '^$' [name='name_with:colon'] has a name including a ':'."
+ self.assertIn(expected_msg, warning.msg)
diff --git a/tests/check_framework/urls_include.py b/tests/check_framework/urls_include.py
new file mode 100644
index 0000000000..5bb94c9688
--- /dev/null
+++ b/tests/check_framework/urls_include.py
@@ -0,0 +1,7 @@
+from django.conf.urls import include, url
+
+urlpatterns = [
+ url(r'^', include([
+ url(r'^include-with-dollar$', include([])),
+ ])),
+]
diff --git a/tests/check_framework/urls_name.py b/tests/check_framework/urls_name.py
new file mode 100644
index 0000000000..9662532851
--- /dev/null
+++ b/tests/check_framework/urls_name.py
@@ -0,0 +1,13 @@
+from django.conf.urls import include, url
+from django.http import HttpResponse
+
+
+def view(request):
+ return HttpResponse('')
+
+
+urlpatterns = [
+ url('^', include([
+ url(r'^$', view, name='name_with:colon'),
+ ])),
+]
diff --git a/tests/check_framework/urls_no_warnings.py b/tests/check_framework/urls_no_warnings.py
new file mode 100644
index 0000000000..657306f1ef
--- /dev/null
+++ b/tests/check_framework/urls_no_warnings.py
@@ -0,0 +1,15 @@
+from django.conf.urls import include, url
+from django.http import HttpResponse
+
+
+def view(request):
+ return HttpResponse('')
+
+
+urlpatterns = [
+ url(r'^foo/', view, name='foo'),
+ # This dollar is ok as it is escaped
+ url(r'^\$', include([
+ url(r'^bar/$', view, name='bar'),
+ ])),
+]
diff --git a/tests/check_framework/urls_slash.py b/tests/check_framework/urls_slash.py
new file mode 100644
index 0000000000..474f6293d5
--- /dev/null
+++ b/tests/check_framework/urls_slash.py
@@ -0,0 +1,13 @@
+from django.conf.urls import include, url
+from django.http import HttpResponse
+
+
+def view(request):
+ return HttpResponse('')
+
+
+urlpatterns = [
+ url('^', include([
+ url(r'/starting-with-slash/$', view),
+ ])),
+]