Re: [Django] #13260: urlresolvers.reverse() generates invalid URLs when an argument contains % character
Django
character
-------------------------------------+-------------------------------------
Reporter: semenov | Owner: stumbles
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Design
Has patch: 1 | decision needed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* component: Core (Other) => Core (URLs)
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:16>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
character
Reporter: semenov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------
Changes (by aaugustin):
* owner: stumbles => aaugustin
* stage: Design decision needed => Accepted
Comment:
Judging by
[http://hg.python.org/cpython/file/6aab72424063/Lib/wsgiref/simple_server.py#l82
the reference implementation], WSGI urlunquotes the path before putting it
in `environ['PATH_INFO']` where Django reads it.
That's where things get complicated :)
1) To round-trip properly through reverse / resolve, arguments must not be
urlquoted by reverse — there's nothing to urlunquote them in this
scenario.
2) To round-trip properly through reverse / render in template / click in
browser / resolve, arguments must be urlquoted by reverse.
----
The
[https://docs.djangoproject.com/en/dev/ref/urlresolvers/#django.core.urlresolvers.reverse
docs for reverse] say that "the string returned by `reverse()` is already
urlquoted", which obviously isn't true for at least some special
characters.
This refers to the fact that `reverse()` calls `iri_to_uri()`, but that
function is primarily concerned with escaping non-ASCII characters. It's
also idempotent, which means it won't escape any character that's legal in
an URL.
----
I recommmend to change `reverse()` to urlquote its arguments, for the
following reasons:
- (2) above is the common case
- The current behavior doesn't match the docs
- The current behavior may result in invalid URLs (that still work in
practic -- URL handling code is notoriously robust to malformed inputs!)
- Contributors who looked at this ticket until now were mostly in favor of
considering this a bug
It will be worth a note in the "backwards incompatible changes" because it
seems very likely that some developers are working around the current,
buggy behavior by urlquoting arguments to reverse, and they would get
double-quoting.
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:17>
Django
character
-----------------------------+-------------------------------------
Reporter: semenov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------
Comment (by aaugustin):
I've created a little experiment to showcase the bug:
{{{
# Put this in a file called experiment13260.py and run it with:
# django-admin.py runserver --settings=experiment13260
from django.conf.urls import patterns, url
from django.http import HttpResponse
from django.template import Context, Template
DEBUG = True
ROOT_URLCONF = 'experiment13260'
SECRET_KEY = 'whatever'
TEMPLATE = Template("""<html>
<head>
<title>Experiment with Django ticket #13260</title>
</head>
<body>
<p>Put something with special characters in the URL!</p>
<p>Argument passed to the view: <b>{{ arg }}</b></p>
<p>Reversed URL: <b><a href="{% url 'view' arg %}">{% url 'view' arg
%}</a></b></p>
</body>
</html>""")
urlpatterns = patterns('',
url(r'^(.*)/?$', lambda req, arg:
HttpResponse(TEMPLATE.render(Context({'arg': arg}))), name='view'),
)
}}}
If you go to http://localhost:8000/%252525/, every time you click the
"Reversed URL" link, you lose a 25 in the URL.
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:18>
Django
character
-----------------------------+-------------------------------------
Reporter: semenov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------
Comment (by aaugustin):
Pull request: https://github.com/django/django/pull/917
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:19>
Django
character
-----------------------------+-------------------------------------
Reporter: semenov | Owner: aaugustin
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"31b5275235bac150a54059db0288a19b9e0516c7"]:
{{{
#!CommitTicketReference repository=""
revision="31b5275235bac150a54059db0288a19b9e0516c7"
Fixed #13260 -- Quoted arguments interpolated in URLs in reverse.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:20>
Django
character
-----------------------------+-------------------------------------
Reporter: semenov | Owner: aaugustin
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"dfc092622e5e55081f9a76fddea752494c4505ba"]:
{{{
#!CommitTicketReference repository=""
revision="dfc092622e5e55081f9a76fddea752494c4505ba"
Fixed #21529 -- Noted that {% url %} encodes its output (refs #13260).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:21>
Django
character
-----------------------------+-------------------------------------
Reporter: semenov | Owner: aaugustin
Type: Bug | Status: closed
Component: Core (URLs) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"a4c32d70c2bcf9731b6d6ff3370d2260ab4812af"]:
{{{
#!CommitTicketReference repository=""
revision="a4c32d70c2bcf9731b6d6ff3370d2260ab4812af"
[1.6.x] Fixed #21529 -- Noted that {% url %} encodes its output (refs
#13260).
Backport of dfc092622e from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13260#comment:22>