Browse files

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

  • Loading branch information...
1 parent 4485b2a commit 31b5275235bac150a54059db0288a19b9e0516c7 @aaugustin aaugustin committed Mar 18, 2013
@@ -375,6 +375,9 @@ def reverse(self, lookup_view, *args, **kwargs):
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())
lookup_view = get_callable(lookup_view, True)
except (ImportError, AttributeError) as e:
@@ -387,8 +390,7 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
if args:
if len(args) != len(params) + len(prefix_args):
- unicode_args = [force_text(val) for val in args]
- candidate = (prefix_norm.replace('%', '%%') + result) % dict(zip(prefix_args + params, unicode_args))
+ candidate_subs = dict(zip(prefix_args + params, text_args))
if set(kwargs.keys()) | set(defaults.keys()) != set(params) | set(defaults.keys()) | set(prefix_args):
@@ -399,10 +401,16 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
if not matches:
- unicode_kwargs = dict([(k, force_text(v)) for (k, v) in kwargs.items()])
- candidate = (prefix_norm.replace('%', '%%') + result) % unicode_kwargs
- if'^%s%s' % (prefix_norm, pattern), candidate, re.UNICODE):
- return candidate
+ candidate_subs = text_kwargs
+ # WSGI provides decoded URLs, without %xx escapes, and the URL
rogerdahl Sep 17, 2016

@aaugustin I'm having an issue with URLs not being properly decoded before being passed into my views when running under WSGI, while things work fine under the dev server. Looking into it, I found this comment, which starts with, WSGI provides decoded URLs, without %xx escapes. This is the opposite of my understanding, so I'm wondering if it's related to the issue I'm having.

In my Django app, I have an endpoint where one of the URL path segments is itself a complete URL. This embedded URL has to be percent-encoded, or it would cause the outer URL to become malformed, and Django wouldn't be able to parse it.

So it seems like Django needs the percent-encoded URL for parsing and should decode path segments and query parameters before passing them to view functions. Is that correct and, if so, how does it fit with the comment in the code?

Any feedback much appreciated.

aaugustin Sep 17, 2016 Member

Can you ask on the django-developers mailing list?

I don't remember the minutiae of every change I made to Django three years ago and I'm not sure I'll find time to investigate your comment in the next months.

rogerdahl Sep 17, 2016

Well, this wasn't really about a change in the code but about a fairly general statement you made about how WSGI works. Thanks for the reply though. I'll keep investigating.

aaugustin Sep 18, 2016 Member

The best way to know how WSGI works is to test with a plain WSGI app what gunicorn, mod_wsgi and uwsgi do. They may or may not do the same thing, which may or may not be what PEP 3333 says :-/

Then you can add Django to the mix and see if it does something incorrect.

+ # resolver operates on such URLs. First substitute arguments
+ # 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'^%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.
@@ -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
@@ -32,7 +32,7 @@
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 @@ def test_changelist_to_changeform_link(self):
"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
- pk_final_url = escape(iri_to_uri(quote(
+ # this URL now comes through reverse(), thus url quoting and iri_to_uri encoding
+ pk_final_url = escape(iri_to_uri(urlquote(quote(
should_contain = """<th><a href="%s%s/">%s</a></th>""" % (prefix, pk_final_url, escape(
self.assertContains(response, should_contain)
@@ -1484,8 +1484,8 @@ def test_recentactions_without_content_type(self):
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(
- # this URL now comes through reverse(), thus iri_to_uri encoding
- should_contain = """/%s/">%s</a>""" % (escape(iri_to_uri(quote(, escape(
+ # this URL now comes through reverse(), thus url quoting and iri_to_uri encoding
+ should_contain = """/%s/">%s</a>""" % (escape(iri_to_uri(urlquote(quote(, escape(
self.assertContains(response, should_contain)
def test_url_conflicts_with_add(self):
@@ -1611,12 +1611,12 @@ def get_template_tests(self):
'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" action="two words" %}', {'client': {'id': 1}}, '/url_tag/client/1/two%20words/'),
- 'url11': ('{% url "template_tests.views.client_action" action="==" %}', {'client': {'id': 1}}, '/url_tag/client/1/==/'),
- 'url12': ('{% url "template_tests.views.client_action" action="," %}', {'client': {'id': 1}}, '/url_tag/client/1/,/'),
+ 'url11': ('{% url "template_tests.views.client_action" action="==" %}', {'client': {'id': 1}}, '/url_tag/client/1/%3D%3D/'),
+ 'url12': ('{% url "template_tests.views.client_action" action="," %}', {'client': {'id': 1}}, '/url_tag/client/1/%2C/'),
'url13': ('{% url "template_tests.views.client_action" action=arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'),
'url14': ('{% url "template_tests.views.client_action" 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 %}', {'named_url': 'template_tests.views.client', 'client': {'id': 1}}, '/url_tag/client/1/'),
'url20': ('{% url url_name_in_var %}', {'url_name_in_var': 'named.client', 'client': {'id': 1}}, '/url_tag/named-client/1/'),
@@ -97,7 +97,11 @@
('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/', [], {}),
@@ -40,7 +40,7 @@
url(r'^windows_path/(?P<drive_name>[A-Z]):\\(?P<path>.+)/$', empty_view,
- 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"),

0 comments on commit 31b5275

Please sign in to comment.