Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed #22223 -- Prevented over-escaping URLs in reverse() #2859

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

claudep commented Jun 26, 2014

And follow more closely the class of characters defined in the
RFC 3986.
Thanks Erik van Zijst for the report and the initial patch.

@claudep claudep Fixed #22223 -- Prevented over-escaping URLs in reverse()
And follow more closely the class of characters defined in the
RFC 3986.
Thanks Erik van Zijst for the report and the initial patch.
93e23e0

@timgraham timgraham commented on the diff Jun 30, 2014

django/contrib/admin/utils.py
@@ -57,7 +57,7 @@ def quote(s):
res = list(s)
for i in range(len(res)):
c = res[i]
- if c in """:/_#?;@&=+$,"<>%\\""":
+ if c in """:/_#?;@&=+$,"[]<>%\\""":
@timgraham

timgraham Jun 30, 2014

Owner

Can you add a comment about how this list of characters is generated?

@claudep

claudep Jul 2, 2014

Member

Do you have more stupid questions like this :-) ?
Frankly, it's still a bit mysterious for me. RFC3986_GENDELIMS is completely in, then part of RFC3986_SUBDELIMS (missing: !'()*) + some other chars : _"<>%\

@timgraham timgraham commented on the diff Jun 30, 2014

tests/admin_views/tests.py
@@ -1748,7 +1748,7 @@ def test_changelist_to_changeform_link(self):
prefix = '/test_admin/admin/admin_views/modelwithstringprimarykey/'
response = self.client.get(prefix)
# this URL now comes through reverse(), thus url quoting and iri_to_uri encoding
- pk_final_url = escape(iri_to_uri(urlquote(quote(self.pk))))
@timgraham

timgraham Jun 30, 2014

Owner

F401 'urlquote' imported but unused

Owner

timgraham commented Jun 30, 2014

Seems like we should probably backport to 1.6 since it's a regression that broke stuff. Are you confident enough about it? If so, please add to 1.6.6 release notes.

Works for me.

Member

claudep commented Jul 2, 2014

It's very difficult to tell if this is really safe to backport to 1.6...

Owner

timgraham commented Jul 5, 2014

I think we should just commit it to master for now then.

@claudep claudep closed this Jul 9, 2014

@claudep claudep deleted the claudep:22223 branch Jul 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment