Fixed #13260 -- Quoted arguments interpolated in URLs in reverse.

django/core/urlresolvers.py

@@ -375,6 +375,9 @@ class RegexURLResolver(LocaleRegexProvider):
     def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
         if args and kwargs:
             raise ValueError("Don't mix *args and **kwargs in call to reverse()!")
+        text_args = [force_text(v) for v in args]
+        text_kwargs = dict((k, force_text(v)) for (k, v) in kwargs.items())
+
         try:
             lookup_view = get_callable(lookup_view, True)
         except (ImportError, AttributeError) as e:
@@ -387,8 +390,7 @@ class RegexURLResolver(LocaleRegexProvider):
                 if args:
                     if len(args) != len(params) + len(prefix_args):
                         continue
-                    unicode_args = [force_text(val) for val in args]
+                    candidate_subs = dict(zip(prefix_args + params, text_args))
-                    candidate = (prefix_norm.replace('%', '%%') + result) % dict(zip(prefix_args + params, unicode_args))
                 else:
                     if set(kwargs.keys()) | set(defaults.keys()) != set(params) | set(defaults.keys()) | set(prefix_args):
                         continue
@@ -399,10 +401,16 @@ class RegexURLResolver(LocaleRegexProvider):
                             break
                     if not matches:
                         continue
-                    unicode_kwargs = dict([(k, force_text(v)) for (k, v) in kwargs.items()])
+                    candidate_subs = text_kwargs
-                    candidate = (prefix_norm.replace('%', '%%') + result) % unicode_kwargs
+                # WSGI provides decoded URLs, without %xx escapes, and the URL
-                if re.search('^%s%s' % (prefix_norm, pattern), candidate, re.UNICODE):
+                # resolver operates on such URLs. First substitute arguments
-                    return candidate
+                # without quoting to build a decoded URL and look for a match.
+                # Then, if we have a match, redo the substitution with quoted
+                # arguments in order to return a properly encoded URL.
+                candidate_pat = prefix_norm.replace('%', '%%') + result
+                if re.search('^%s%s' % (prefix_norm, pattern), candidate_pat % candidate_subs, re.UNICODE):
+                    candidate_subs = dict((k, urlquote(v)) for (k, v) in candidate_subs.items())
+                    return candidate_pat % candidate_subs
         # lookup_view can be URL label, or dotted path, or callable, Any of
         # these can be passed in at the top, but callables are not friendly in
         # error messages.

docs/releases/1.6.txt

@@ -281,6 +281,16 @@ warning is generated by :djadmin:`makemessages` when it finds them. E.g.:
     {{ title }}{# Translators: Extracted and associated with 'Welcome' below #}
     <h1>{% trans "Welcome" %}</h1>
 
+Quoting in :func:`~django.core.urlresolvers.reverse`
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When reversing URLs, Django didn't apply :func:`~django.utils.http.urlquote`
+to arguments before interpolating them in URL patterns. This bug is fixed in
+Django 1.6. If you worked around this bug by applying URL quoting before
+passing arguments to :func:`~django.core.urlresolvers.reverse`, this may
+result in double-quoting. If this happens, simply remove the URL quoting from
+your code.
+
 Storage of IP addresses in the comments app
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

tests/admin_views/tests.py

@@ -32,7 +32,7 @@ from django.utils import formats, translation, unittest
 from django.utils.cache import get_max_age
 from django.utils.encoding import iri_to_uri, force_bytes
 from django.utils.html import escape
-from django.utils.http import urlencode
+from django.utils.http import urlencode, urlquote
 from django.utils._os import upath
 from django.utils import six
 from django.test.utils import override_settings
@@ -1450,8 +1450,8 @@ class AdminViewStringPrimaryKeyTest(TestCase):
         "Link to the changeform of the object in changelist should use reverse() and be quoted -- #18072"
         prefix = '/test_admin/admin/admin_views/modelwithstringprimarykey/'
         response = self.client.get(prefix)
-        # this URL now comes through reverse(), thus iri_to_uri encoding
+        # this URL now comes through reverse(), thus url quoting and iri_to_uri encoding
-        pk_final_url = escape(iri_to_uri(quote(self.pk)))
+        pk_final_url = escape(iri_to_uri(urlquote(quote(self.pk))))
         should_contain = """<th><a href="%s%s/">%s</a></th>""" % (prefix, pk_final_url, escape(self.pk))
         self.assertContains(response, should_contain)
 
@@ -1484,8 +1484,8 @@ class AdminViewStringPrimaryKeyTest(TestCase):
     def test_deleteconfirmation_link(self):
         "The link from the delete confirmation page referring back to the changeform of the object should be quoted"
         response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/' % quote(self.pk))
-        # this URL now comes through reverse(), thus iri_to_uri encoding
+        # this URL now comes through reverse(), thus url quoting and iri_to_uri encoding
-        should_contain = """/%s/">%s</a>""" % (escape(iri_to_uri(quote(self.pk))), escape(self.pk))
+        should_contain = """/%s/">%s</a>""" % (escape(iri_to_uri(urlquote(quote(self.pk)))), escape(self.pk))
         self.assertContains(response, should_contain)
 
     def test_url_conflicts_with_add(self):

tests/template_tests/tests.py

@@ -1611,12 +1611,12 @@ class Templates(TestCase):
             'url08': ('{% url "метка_оператора" v %}', {'v': 'Ω'}, '/url_tag/%D0%AE%D0%BD%D0%B8%D0%BA%D0%BE%D0%B4/%CE%A9/'),
             'url09': ('{% url "метка_оператора_2" tag=v %}', {'v': 'Ω'}, '/url_tag/%D0%AE%D0%BD%D0%B8%D0%BA%D0%BE%D0%B4/%CE%A9/'),
             'url10': ('{% url "template_tests.views.client_action" id=client.id action="two words" %}', {'client': {'id': 1}}, '/url_tag/client/1/two%20words/'),
-            'url11': ('{% url "template_tests.views.client_action" id=client.id action="==" %}', {'client': {'id': 1}}, '/url_tag/client/1/==/'),
+            'url11': ('{% url "template_tests.views.client_action" id=client.id action="==" %}', {'client': {'id': 1}}, '/url_tag/client/1/%3D%3D/'),
-            'url12': ('{% url "template_tests.views.client_action" id=client.id action="," %}', {'client': {'id': 1}}, '/url_tag/client/1/,/'),
+            'url12': ('{% url "template_tests.views.client_action" id=client.id action="," %}', {'client': {'id': 1}}, '/url_tag/client/1/%2C/'),
             'url13': ('{% url "template_tests.views.client_action" id=client.id action=arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'),
             'url14': ('{% url "template_tests.views.client_action" client.id arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'),
             'url15': ('{% url "template_tests.views.client_action" 12 "test" %}', {}, '/url_tag/client/12/test/'),
-            'url18': ('{% url "template_tests.views.client" "1,2" %}', {}, '/url_tag/client/1,2/'),
+            'url18': ('{% url "template_tests.views.client" "1,2" %}', {}, '/url_tag/client/1%2C2/'),
 
             'url19': ('{% url named_url client.id %}', {'named_url': 'template_tests.views.client', 'client': {'id': 1}}, '/url_tag/client/1/'),
             'url20': ('{% url url_name_in_var client.id %}', {'url_name_in_var': 'named.client', 'client': {'id': 1}}, '/url_tag/named-client/1/'),

tests/urlpatterns_reverse/tests.py

@@ -97,7 +97,11 @@ test_data = (
     ('product', '/product/chocolate+($2.00)/', [], {'price': '2.00', 'product': 'chocolate'}),
     ('headlines', '/headlines/2007.5.21/', [], dict(year=2007, month=5, day=21)),
     ('windows', r'/windows_path/C:%5CDocuments%20and%20Settings%5Cspam/', [], dict(drive_name='C', path=r'Documents and Settings\spam')),
-    ('special', r'/special_chars/+%5C$*/', [r'+\$*'], {}),
+    ('special', r'/special_chars/%2B%5C%24%2A/', [r'+\$*'], {}),
+    ('special', r'/special_chars/some%20resource/', [r'some resource'], {}),
+    ('special', r'/special_chars/10%25%20complete/', [r'10% complete'], {}),
+    ('special', r'/special_chars/some%20resource/', [], {'chars': r'some resource'}),
+    ('special', r'/special_chars/10%25%20complete/', [], {'chars': r'10% complete'}),
     ('special', NoReverseMatch, [''], {}),
     ('mixed', '/john/0/', [], {'name': 'john'}),
     ('repeats', '/repeats/a/', [], {}),

tests/urlpatterns_reverse/urls.py

@@ -40,7 +40,7 @@ urlpatterns = patterns('',
             name="headlines"),
     url(r'^windows_path/(?P<drive_name>[A-Z]):\\(?P<path>.+)/$', empty_view,
             name="windows"),
-    url(r'^special_chars/(.+)/$', empty_view, name="special"),
+    url(r'^special_chars/(?P<chars>.+)/$', empty_view, name="special"),
     url(r'^(?P<name>.+)/\d+/$', empty_view, name="mixed"),
     url(r'^repeats/a{1,2}/$', empty_view, name="repeats"),
     url(r'^repeats/a{2,4}/$', empty_view, name="repeats2"),