#30043: AdminURLFieldWidget incorrectly unquotes URLs e.g. containing %2F
-------------------------------------+-------------------------------------
Reporter: Brenton Partridge | Owner: Brady
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: admin, urlfield, | Triage Stage: Accepted
smart_urlquote, url, quote |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Right, yes, that was it. The issue is that there's no (easy) way to know
if a string has already been quoted, which re-reading is exactly what
Florian said above:
> The main issue here is that we do not know if the URL supplied is
already sufficiently quoted or not.
I think the use-case will be where you have a URL, with a quoted-URL as a
query parameter (say). As understand it, you do want `%2F`s in there, so
you'd use `quote_plus`, as used by default by `urlencode`:
{{{
>>> from urllib.parse import quote, quote_plus, urlencode
>>> quote('https://djangoproject.com/')
'https%3A//djangoproject.com/'
>>> quote_plus('https://djangoproject.com/testing/')
'https%3A%2F%2Fdjangoproject.com%2Ftesting%2F'
>>> urlencode({'q':'https://djangoproject.com/testing/'})
'q=https%3A%2F%2Fdjangoproject.com%2Ftesting%2F'
}}}
But, if it's part of the path, and not the query parameters, you don't
want `quote_plus`. Taking Florian's example:
{{{
>>> quote('/äöü')
'/%C3%A4%C3%B6%C3%BC'
>>> quote_plus('/äöü')
'%2F%C3%A4%C3%B6%C3%BC'
}}}
The 2nd, as part of the path, would be wrong.
The good thing about `iri_to_uri()` is that it won't ''double-percent-
escape'', whereas `quote()` keeps on changing the input (re-quoting) each
time, so if a URL does **look** quoted you can pass it through and get the
right results. **But edge cases...???**
`smart_urlquote()`, perhaps why it has the name,
[https://github.com/django/django/blob/315357ad25a6590e7f4564ec2e56a22132b09001/django/utils/html.py#L224-L233
**already** handles the the query string parameters separately, using
`urlencode`]. (And looking at the issue linked in the comment there, that
was the exact discussion from #22267...) — **SO** short of concrete test
cases (i.e. full example URLs where the behaviour is wrong) I can't see
that we can progress.
On that basis, I'm going to close as `needsinfo`. Happy to re-open if full
test cases come up.
Thank you for your effort and time looking into this Brady! Welcome
aboard! 🎉
--
Ticket URL: <https://code.djangoproject.com/ticket/30043#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/068.5286721087945093c035a69be6c72d3d%40djangoproject.com.
