Fixed #22384 -- Deprecated reversing URLs by dotted path.

django/contrib/flatpages/urls.py

@@ -2,5 +2,5 @@ from django.conf.urls import url
 from django.contrib.flatpages import views
 
 urlpatterns = [
-    url(r'^(?P<url>.*)$', views.flatpage),
+    url(r'^(?P<url>.*)$', views.flatpage, name='django.contrib.flatpages.views.flatpage'),
 ]

django/contrib/messages/tests/base.py

@@ -158,10 +158,9 @@ class BaseTests(object):
         data = {
             'messages': ['Test message %d' % x for x in range(5)],
         }
-        show_url = reverse('django.contrib.messages.tests.urls.show')
+        show_url = reverse('show_message')
         for level in ('debug', 'info', 'success', 'warning', 'error'):
-            add_url = reverse('django.contrib.messages.tests.urls.add',
+            add_url = reverse('add_message', args=(level,))
-                              args=(level,))
             response = self.client.post(add_url, data, follow=True)
             self.assertRedirects(response, show_url)
             self.assertTrue('messages' in response.context)
@@ -175,10 +174,9 @@ class BaseTests(object):
         data = {
             'messages': ['Test message %d' % x for x in range(5)],
         }
-        show_url = reverse('django.contrib.messages.tests.urls.show_template_response')
+        show_url = reverse('show_template_response')
         for level in self.levels.keys():
-            add_url = reverse('django.contrib.messages.tests.urls.add_template_response',
+            add_url = reverse('add_template_response', args=(level,))
-                              args=(level,))
             response = self.client.post(add_url, data, follow=True)
             self.assertRedirects(response, show_url)
             self.assertTrue('messages' in response.context)
@@ -191,7 +189,7 @@ class BaseTests(object):
                 self.assertNotContains(response, msg)
 
     def test_context_processor_message_levels(self):
-        show_url = reverse('django.contrib.messages.tests.urls.show_template_response')
+        show_url = reverse('show_template_response')
         response = self.client.get(show_url)
 
         self.assertTrue('DEFAULT_MESSAGE_LEVELS' in response.context)
@@ -206,12 +204,11 @@ class BaseTests(object):
         data = {
             'messages': ['Test message %d' % x for x in range(5)],
         }
-        show_url = reverse('django.contrib.messages.tests.urls.show')
+        show_url = reverse('show_message')
         messages = []
         for level in ('debug', 'info', 'success', 'warning', 'error'):
             messages.extend([Message(self.levels[level], msg) for msg in data['messages']])
-            add_url = reverse('django.contrib.messages.tests.urls.add',
+            add_url = reverse('add_message', args=(level,))
-                              args=(level,))
             self.client.post(add_url, data)
         response = self.client.get(show_url)
         self.assertTrue('messages' in response.context)
@@ -233,10 +230,9 @@ class BaseTests(object):
         data = {
             'messages': ['Test message %d' % x for x in range(5)],
         }
-        reverse('django.contrib.messages.tests.urls.show')
+        reverse('show_message')
         for level in ('debug', 'info', 'success', 'warning', 'error'):
-            add_url = reverse('django.contrib.messages.tests.urls.add',
+            add_url = reverse('add_message', args=(level,))
-                              args=(level,))
             self.assertRaises(MessageFailure, self.client.post, add_url,
                               data, follow=True)
 
@@ -254,10 +250,9 @@ class BaseTests(object):
             'messages': ['Test message %d' % x for x in range(5)],
             'fail_silently': True,
         }
-        show_url = reverse('django.contrib.messages.tests.urls.show')
+        show_url = reverse('show_message')
         for level in ('debug', 'info', 'success', 'warning', 'error'):
-            add_url = reverse('django.contrib.messages.tests.urls.add',
+            add_url = reverse('add_message', args=(level,))
-                              args=(level,))
             response = self.client.post(add_url, data, follow=True)
             self.assertRedirects(response, show_url)
             self.assertFalse('messages' in response.context)

django/contrib/messages/tests/urls.py

@@ -33,7 +33,7 @@ def add(request, message_type):
         else:
             getattr(messages, message_type)(request, msg)
 
-    show_url = reverse('django.contrib.messages.tests.urls.show')
+    show_url = reverse('show_message')
     return HttpResponseRedirect(show_url)
 
 
@@ -42,7 +42,7 @@ def add_template_response(request, message_type):
     for msg in request.POST.getlist('messages'):
         getattr(messages, message_type)(request, msg)
 
-    show_url = reverse('django.contrib.messages.tests.urls.show_template_response')
+    show_url = reverse('show_template_response')
     return HttpResponseRedirect(show_url)
 
 
@@ -69,9 +69,9 @@ class ContactFormViewWithMsg(SuccessMessageMixin, FormView):
 
 
 urlpatterns = [
-    url('^add/(debug|info|success|warning|error)/$', add),
+    url('^add/(debug|info|success|warning|error)/$', add, name='add_message'),
     url('^add/msg/$', ContactFormViewWithMsg.as_view(), name='add_success_msg'),
-    url('^show/$', show),
+    url('^show/$', show, name='show_message'),
-    url('^template_response/add/(debug|info|success|warning|error)/$', add_template_response),
+    url('^template_response/add/(debug|info|success|warning|error)/$', add_template_response, name='add_template_response'),
-    url('^template_response/show/$', show_template_response),
+    url('^template_response/show/$', show_template_response, name='show_template_response'),
 ]

django/contrib/sitemaps/tests/test_https.py

@@ -1,8 +1,10 @@
 from __future__ import unicode_literals
 
 from datetime import date
+import warnings
 
 from django.test import override_settings
+from django.utils.deprecation import RemovedInDjango20Warning
 
 from .base import SitemapTestsBase
 
@@ -38,6 +40,14 @@ class HTTPSDetectionSitemapTests(SitemapTestsBase):
 
     def test_sitemap_index_with_https_request(self):
         "A sitemap index requested in HTTPS is rendered with HTTPS links"
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore", category=RemovedInDjango20Warning)
+            # The URL for views.sitemap in tests/urls/http.py has been updated
+            # with a name but since reversing by Python path is tried first
+            # before reversing by name and works since we're giving
+            # name='django.contrib.sitemaps.views.sitemap', we need to silence
+            # the erroneous warning until reversing by dotted path is removed.
+            # The test will work without modification when it's removed.
             response = self.client.get('/simple/index.xml', **self.extra)
         expected_content = """<?xml version="1.0" encoding="UTF-8"?>
 <sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">

django/contrib/sitemaps/tests/urls/http.py

@@ -72,7 +72,7 @@ urlpatterns = [
     url(r'^simple/custom-index\.xml$', views.index,
         {'sitemaps': simple_sitemaps, 'template_name': 'custom_sitemap_index.xml'}),
     url(r'^simple/sitemap-(?P<section>.+)\.xml$', views.sitemap,
-        {'sitemaps': simple_sitemaps}),
+        {'sitemaps': simple_sitemaps}, name='django.contrib.sitemaps.views.sitemap'),
     url(r'^simple/sitemap\.xml$', views.sitemap, {'sitemaps': simple_sitemaps}),
     url(r'^simple/custom-sitemap\.xml$', views.sitemap,
         {'sitemaps': simple_sitemaps, 'template_name': 'custom_sitemap.xml'}),

django/core/urlresolvers.py

@@ -12,10 +12,12 @@ import functools
 from importlib import import_module
 import re
 from threading import local
+import warnings
 
 from django.http import Http404
 from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist
 from django.utils.datastructures import MultiValueDict
+from django.utils.deprecation import RemovedInDjango20Warning
 from django.utils.encoding import force_str, force_text, iri_to_uri
 from django.utils.functional import lazy
 from django.utils.http import urlquote
@@ -424,11 +426,18 @@ class RegexURLResolver(LocaleRegexProvider):
         if not self._populated:
             self._populate()
 
+        original_lookup = lookup_view
         try:
             if lookup_view in self._callback_strs:
                 lookup_view = get_callable(lookup_view, True)
         except (ImportError, AttributeError) as e:
             raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e))
+        else:
+            if not callable(original_lookup) and callable(lookup_view):
+                warnings.warn(
+                    'Reversing by dotted path is deprecated (%s).' % original_lookup,
+                    RemovedInDjango20Warning, stacklevel=3
+                )
         possibilities = self.reverse_dict.getlist(lookup_view)
 
         prefix_norm, prefix_args = normalize(urlquote(_prefix))[0]

docs/internals/deprecation.txt

@@ -32,6 +32,9 @@ about each item can often be found in the release notes of two versions prior.
   and migrations will become compulsory for all apps. This includes automatic
   loading of fixtures and support for initial SQL data.
 
+* The ability to :func:`~django.core.urlresolvers.reverse` URLs using a dotted
+  Python path will be removed.
+
 .. _deprecation-removed-in-1.9:
 
 1.9

docs/ref/contrib/sitemaps.txt

@@ -54,7 +54,8 @@ Initialization
 To activate sitemap generation on your Django site, add this line to your
 :doc:`URLconf </topics/http/urls>`::
 
-   (r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap', {'sitemaps': sitemaps})
+   url(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap',
+       {'sitemaps': sitemaps}, name='django.contrib.sitemaps.views.sitemap')
 
 This tells Django to build a sitemap when a client accesses :file:`/sitemap.xml`.
 
@@ -284,7 +285,8 @@ Here's an example of a :doc:`URLconf </topics/http/urls>` using both::
         # ...
 
         # the sitemap
-        url(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap', {'sitemaps': sitemaps}),
+        url(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap',
+            {'sitemaps': sitemaps}, name='django.contrib.sitemaps.views.sitemap'),
     ]
 
 .. _URLconf: ../url_dispatch/
@@ -325,7 +327,8 @@ the sitemap. For example::
         url(r'^about/$', 'views.about', name='about'),
         url(r'^license/$', 'views.license', name='license'),
         # ...
-        url(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap', {'sitemaps': sitemaps})
+        url(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap',
+            {'sitemaps': sitemaps}, name='django.contrib.sitemaps.views.sitemap')
     ]
 
 

docs/ref/templates/builtins.txt

@@ -982,7 +982,7 @@ resulting path will be encoded using :func:`~django.utils.encoding.iri_to_uri`.
 This is a way to output links without violating the DRY principle by having to
 hard-code URLs in your templates::
 
-    {% url 'path.to.some_view' v1 v2 %}
+    {% url 'some-url-name' v1 v2 %}
 
 The first argument is a path to a view function in the format
 ``package.package.module.function``. It can be a quoted literal or any other
@@ -991,7 +991,7 @@ should be space-separated values that will be used as arguments in the URL.
 The example above shows passing positional arguments. Alternatively you may
 use keyword syntax::
 
-    {% url 'path.to.some_view' arg1=v1 arg2=v2 %}
+    {% url 'some-url-name' arg1=v1 arg2=v2 %}
 
 Do not mix both positional and keyword syntax in a single call. All arguments
 required by the URLconf should be present.
@@ -1002,7 +1002,7 @@ takes a client ID (here, ``client()`` is a method inside the views file
 
 .. code-block:: python
 
-    ('^client/([0-9]+)/$', 'app_views.client')
+    ('^client/([0-9]+)/$', 'app_views.client', name='app-views-client')
 
 If this app's URLconf is included into the project's URLconf under a path
 such as this:
@@ -1013,7 +1013,7 @@ such as this:
 
 ...then, in a template, you can create a link to this view like this::
 
-    {% url 'app_views.client' client.id %}
+    {% url 'app-views-client' client.id %}
 
 The template tag will output the string ``/clients/client/123/``.
 
@@ -1028,7 +1028,7 @@ cause your site to display an error page.
 If you'd like to retrieve a URL without displaying it, you can use a slightly
 different call::
 
-    {% url 'path.to.view' arg arg2 as the_url %}
+    {% url 'some-url-name' arg arg2 as the_url %}
 
     <a href="{{ the_url }}">I'm linking to {{ the_url }}</a>
 
@@ -1038,7 +1038,7 @@ The scope of the variable created by the  ``as var`` syntax is the
 This ``{% url ... as var %}`` syntax will *not* cause an error if the view is
 missing. In practice you'll use this to link to views that are optional::
 
-    {% url 'path.to.view' as the_url %}
+    {% url 'some-url-name' as the_url %}
     {% if the_url %}
       <a href="{{ the_url }}">Link to optional stuff</a>
     {% endif %}
@@ -1051,6 +1051,13 @@ This will follow the normal :ref:`namespaced URL resolution strategy
 <topics-http-reversing-url-namespaces>`, including using any hints provided
 by the context as to the current application.
 
+.. deprecated:: 1.8
+
+    The dotted Python path syntax is deprecated and will be removed in
+    Django 2.0::
+
+        {% url 'path.to.some_view' v1 v2 %}
+
 .. warning::
 
     Don't forget to put quotes around the function path or pattern name,

docs/ref/urlresolvers.txt

@@ -16,13 +16,12 @@ your code, Django provides the following function:
 :ref:`URL pattern name <naming-url-patterns>`, or the callable view object.
 For example, given the following ``url``::
 
-    url(r'^archive/$', 'news.views.archive', name='news_archive')
+    from news import views
+
+    url(r'^archive/$', views.archive, name='news_archive')
 
 you can use any of the following to reverse the URL::
 
-    # using the Python path
-    reverse('news.views.archive')
-
     # using the named URL
     reverse('news_archive')
 
@@ -63,6 +62,10 @@ namespaces into URLs on specific application instances, according to the
 The ``urlconf`` argument is the URLconf module containing the url patterns to
 use for reversing. By default, the root URLconf for the current thread is used.
 
+.. deprecated:: 1.8
+
+    The ability to reverse using the Python path, e.g.
+    ``reverse('news.views.archive')``, has been deprecated.
 
 .. admonition:: Make sure your views are all correct.
 

docs/releases/1.8.txt

@@ -323,3 +323,21 @@ Using an incorrect count of unpacked values in the :ttag:`for` template tag
 
 Using an incorrect count of unpacked values in :ttag:`for` tag will raise an
 exception rather than fail silently in Django 2.0.
+
+Passing a dotted path to :func:`~django.core.urlresolvers.reverse()` and :ttag:`url`
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Reversing URLs by Python path is an expensive operation as it causes the
+path being reversed to be imported. This behavior has also resulted in a
+`security issue`_. Use :ref:`named URL patterns <naming-url-patterns>`
+for reversing instead.
+
+If you are using :mod:`django.contrib.sitemaps`, add the ``name`` argument to
+the ``url`` that references :func:`django.contrib.sitemaps.views.sitemap`:
+
+    url(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap',
+        {'sitemaps': sitemaps}, name='django.contrib.sitemaps.views.sitemap')
+
+to ensure compatibility when reversing by Python path is removed in Django 2.0.
+
+.. _security issue: https://www.djangoproject.com/weblog/2014/apr/21/security/#s-issue-unexpected-code-execution-using-reverse

docs/topics/http/urls.txt

@@ -555,7 +555,7 @@ Consider again this URLconf entry::
 
     urlpatterns = [
         #...
-        url(r'^articles/([0-9]{4})/$', 'news.views.year_archive'),
+        url(r'^articles/([0-9]{4})/$', 'news.views.year_archive', name='news-year-archive'),
         #...
     ]
 
@@ -566,11 +566,11 @@ You can obtain these in template code by using:
 
 .. code-block:: html+django
 
-    <a href="{% url 'news.views.year_archive' 2012 %}">2012 Archive</a>
+    <a href="{% url 'news-year-archive' 2012 %}">2012 Archive</a>
     {# Or with the year in a template context variable: #}
     <ul>
     {% for yearvar in year_list %}
-    <li><a href="{% url 'news.views.year_archive' yearvar %}">{{ yearvar }} Archive</a></li>
+    <li><a href="{% url 'news-year-archive' yearvar %}">{{ yearvar }} Archive</a></li>
     {% endfor %}
     </ul>
 
@@ -583,7 +583,7 @@ Or in Python code::
         # ...
         year = 2006
         # ...
-        return HttpResponseRedirect(reverse('news.views.year_archive', args=(year,)))
+        return HttpResponseRedirect(reverse('news-year-archive', args=(year,)))
 
 If, for some reason, it was decided that the URLs where content for yearly
 article archives are published at should be changed then you would only need to
@@ -599,65 +599,19 @@ URLs. Read the next section to know about the solution Django provides for this.
 Naming URL patterns
 ===================
 
-It's fairly common to use the same view function in multiple URL patterns in
+In order to perform URL reversing, you'll need to use **named URL patterns**
-your URLconf. For example, these two URL patterns both point to the ``archive``
+as done in the examples above. The string used for the URL name can contain any
-view::
+characters you like. You are not restricted to valid Python names.
 
-    from django.conf.urls import url
+When you name your URL patterns, make sure you use names that are unlikely
-    from mysite.views import archive
+to clash with any other application's choice of names. If you call your URL
+pattern ``comment``, and another application does the same thing, there's
+no guarantee which URL will be inserted into your template when you use
+this name.
 
-    urlpatterns = [
+Putting a prefix on your URL names, perhaps derived from the application
-        url(r'^archive/([0-9]{4})/$', archive),
+name, will decrease the chances of collision. We recommend something like
-        url(r'^archive-summary/([0-9]{4})/$', archive, {'summary': True}),
+``myapp-comment`` instead of ``comment``.
-    ]
-
-This is completely valid, but it leads to problems when you try to do reverse
-URL matching (through the :func:`~django.core.urlresolvers.reverse` function
-or the :ttag:`url` template tag). Continuing this example, if you wanted to
-retrieve the URL for the ``archive`` view, Django's reverse URL matcher would
-get confused, because *two* URL patterns point at that view.
-
-To solve this problem, Django supports **named URL patterns**. That is, you can
-give a name to a URL pattern in order to distinguish it from other patterns
-using the same view and parameters. Then, you can use this name in reverse URL
-matching.
-
-Here's the above example, rewritten to use named URL patterns::
-
-    from django.conf.urls import url
-    from mysite.views import archive
-
-    urlpatterns = [
-        url(r'^archive/([0-9]{4})/$', archive, name="full-archive"),
-        url(r'^archive-summary/([0-9]{4})/$', archive, {'summary': True}, name="arch-summary"),
-    ]
-
-With these names in place (``full-archive`` and ``arch-summary``), you can
-target each pattern individually by using its name:
-
-.. code-block:: html+django
-
-    {% url 'arch-summary' 1945 %}
-    {% url 'full-archive' 2007 %}
-
-Even though both URL patterns refer to the ``archive`` view here, using the
-``name`` parameter to :func:`django.conf.urls.url` allows you to tell them
-apart in templates.
-
-The string used for the URL name can contain any characters you like. You are
-not restricted to valid Python names.
-
-.. note::
-
-    When you name your URL patterns, make sure you use names that are unlikely
-    to clash with any other application's choice of names. If you call your URL
-    pattern ``comment``, and another application does the same thing, there's
-    no guarantee which URL will be inserted into your template when you use
-    this name.
-
-    Putting a prefix on your URL names, perhaps derived from the application
-    name, will decrease the chances of collision. We recommend something like
-    ``myapp-comment`` instead of ``comment``.
 
 .. _topics-http-defining-url-namespaces:
 

tests/resolve_url/tests.py

@@ -1,9 +1,11 @@
 from __future__ import unicode_literals
+import warnings
 
 from django.core.urlresolvers import NoReverseMatch
 from django.contrib.auth.views import logout
 from django.shortcuts import resolve_url
 from django.test import TestCase, override_settings
+from django.utils.deprecation import RemovedInDjango20Warning
 
 from .models import UnimportantThing
 
@@ -60,6 +62,8 @@ class ResolveUrlTests(TestCase):
         Tests that passing a view function to ``resolve_url`` will result in
         the URL path mapping to that view.
         """
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore", category=RemovedInDjango20Warning)
             resolved_url = resolve_url('django.contrib.auth.views.logout')
             self.assertEqual('/accounts/logout/', resolved_url)
 

tests/template_tests/tests.py

@@ -529,8 +529,7 @@ class TemplateTests(TestCase):
 
             # Warm the URL reversing cache. This ensures we don't pay the cost
             # warming the cache during one of the tests.
-            urlresolvers.reverse('template_tests.views.client_action',
+            urlresolvers.reverse('named.client', args=(0,))
-                                 kwargs={'id': 0, 'action': "update"})
 
             for name, vals in tests:
                 if isinstance(vals[2], tuple):
@@ -575,6 +574,7 @@ class TemplateTests(TestCase):
                                     try:
                                         with warnings.catch_warnings():
                                             # Ignore deprecations of using the wrong number of variables with the 'for' tag.
+                                            # and warnings for {% url %} reversing by dotted path
                                             warnings.filterwarnings("ignore", category=RemovedInDjango20Warning, module="django.template.defaulttags")
                                             output = self.render(test_template, vals)
                                     except ShouldNotExecuteException:

tests/urlpatterns_reverse/erroneous_urls.py

@@ -1,18 +1,26 @@
 from django.conf.urls import url
 
+from . import views
+
 urlpatterns = [
     # View has erroneous import
-    url(r'erroneous_inner/$', 'urlpatterns_reverse.views.erroneous_view'),
+    url(r'erroneous_inner/$', views.erroneous_view),
     # Module has erroneous import
+    # Remove in Django 2.0 along with erroneous_views_module as this is only
+    # an issue with string in urlpatterns
     url(r'erroneous_outer/$', 'urlpatterns_reverse.erroneous_views_module.erroneous_view'),
     # Module is an unqualified string
     url(r'erroneous_unqualified/$', 'unqualified_view'),
     # View does not exist
+    # Remove in Django 2.0 along with erroneous_views_module as this is only
+    # an issue with string in urlpatterns
     url(r'missing_inner/$', 'urlpatterns_reverse.views.missing_view'),
     # View is not callable
+    # Remove in Django 2.0 along with erroneous_views_module as this is only
+    # an issue with string in urlpatterns
     url(r'uncallable/$', 'urlpatterns_reverse.views.uncallable'),
     # Module does not exist
     url(r'missing_outer/$', 'urlpatterns_reverse.missing_module.missing_view'),
     # Regex contains an error (refs #6170)
-    url(r'(regex_error/$', 'regressiontestes.urlpatterns_reverse.views.empty_view'),
+    url(r'(regex_error/$', views.empty_view),
 ]

tests/urlpatterns_reverse/tests.py

@@ -6,6 +6,7 @@ from __future__ import unicode_literals
 
 import sys
 import unittest
+import warnings
 
 from django.contrib.auth.models import User
 from django.conf import settings
@@ -17,6 +18,7 @@ from django.http import HttpRequest, HttpResponseRedirect, HttpResponsePermanent
 from django.shortcuts import redirect
 from django.test import TestCase, override_settings
 from django.utils import six
+from django.utils.deprecation import RemovedInDjango20Warning
 
 from admin_scripts.tests import AdminScriptTestCase
 
@@ -173,6 +175,8 @@ class NoURLPatternsTests(TestCase):
 class URLPatternReverse(TestCase):
 
     def test_urlpattern_reverse(self):
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore", category=RemovedInDjango20Warning)
             for name, expected, args, kwargs in test_data:
                 try:
                     got = reverse(name, args=args, kwargs=kwargs)
@@ -373,7 +377,9 @@ class ReverseShortcutTests(TestCase):
 
     def test_reverse_by_path_nested(self):
         # Views that are added to urlpatterns using include() should be
-        # reversible by dotted path.
+        # reversible by doted path.
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore", category=RemovedInDjango20Warning)
             self.assertEqual(reverse('urlpatterns_reverse.views.nested_view'), '/includes/nested_path/')
 
     def test_redirect_view_object(self):