[Django] #28123: django.utils.html.smart_urlquote() is incorrectly parsing the query string
Django
------------------------------------------+--------------------------------
Reporter: Denis Pechenev | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords: smart_urlquote
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+--------------------------------
Query string like
'search_text=%D0%B4%D0%B6%D0%B8%D0%BD%D1%81%D0%BE%D0%B2%D1%8B%D0%B5+%D0%BA%D1%83%D1%80%D1%82%D0%BA%D0%B8'
is already encoded. But smart_urlquote() encodes it again because of
incorrect parsing in parse_qsl(). Value should be encoded with ASCII
before parsing.
So there should be something like:
{{{
query_parts = [(unquote(force_str(q[0])), unquote(force_str(q[1])))
for q in parse_qsl(query.encode('ascii'),
keep_blank_values=True)]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28123>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Denis Pechenev | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Denis Pechenev:
Old description:
> Query string like
> 'search_text=%D0%B4%D0%B6%D0%B8%D0%BD%D1%81%D0%BE%D0%B2%D1%8B%D0%B5+%D0%BA%D1%83%D1%80%D1%82%D0%BA%D0%B8'
> is already encoded. But smart_urlquote() encodes it again because of
> incorrect parsing in parse_qsl(). Value should be encoded with ASCII
> before parsing.
>
> So there should be something like:
>
> {{{
> query_parts = [(unquote(force_str(q[0])), unquote(force_str(q[1])))
> for q in parse_qsl(query.encode('ascii'),
> keep_blank_values=True)]
> }}}
New description:
Query string like
'search_text=%D0%B4%D0%B6%D0%B8%D0%BD%D1%81%D0%BE%D0%B2%D1%8B%D0%B5+%D0%BA%D1%83%D1%80%D1%82%D0%BA%D0%B8'
is already encoded. But smart_urlquote() encodes it again because of
incorrect parsing in parse_qsl(). Value should be encoded with ASCII
before parsing.
So there should be something like:
{{{
query_parts = [(unquote(force_str(q[0])), unquote(force_str(q[1])))
for q in parse_qsl(query.encode('ascii'),
keep_blank_values=True)]
}}}
https://github.com/django/django/blob/master/django/utils/html.py#L216
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:1>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Uncategorized | Version: 1.10
Severity: Normal | Resolution:
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Denis Pechenev):
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:2>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Uncategorized | Version: 1.10
Severity: Normal | Resolution: invalid
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Denis Pechenev):
* status: new => closed
* resolution: => invalid
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:3>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Uncategorized | Version: 1.10
Severity: Normal | Resolution:
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Denis Pechenev):
* status: closed => new
* resolution: invalid =>
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:4>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Uncategorized | Version: 1.10
Severity: Normal | Resolution:
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Igor Gumenyuk):
The is very annoying bug in `smart_urlquote` since `urlize` (for example)
calls it internally and passes unicode as an argument.
And there is no way to wrap argument in `str()` (sic!) or at least
`.encode('ascii')` without forking `urlize`
Here's an example to show the issue:
{{{
from django.utils.html import smart_urlquote, urlize
s1 =
'http://example.com/?search_text=%D0%BF%D1%80%D0%B8%D0%B2%D0%B5%D1%82'
#'http://example.com/?search_text=привет
s2 =
u'http://example.com/?search_text=%D0%BF%D1%80%D0%B8%D0%B2%D0%B5%D1%82'
In [15]: smart_urlquote(s1)
Out[15]:
u'http://example.com/?search_text=%D0%BF%D1%80%D0%B8%D0%B2%D0%B5%D1%82'
In [16]: smart_urlquote(s2)
Out[16]:
u'http://example.com/women?search_text=%C3%90%C2%BF%C3%91%C2%80%C3%90%C2%B8%C3%90%C2%B2%C3%90%C2%B5%C3%91%C2%82'
}}}
With unicode string resulting URL gets double urlquoted.
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:5>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Uncategorized | Version: 1.10
Severity: Normal | Resolution:
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
As far as I can tell, Python 3 isn't affected. Since the master branch
doesn't support Python 2 and I don't think this issue qualifies for a
backport based on our [https://docs.djangoproject.com/en/dev/internals
/release-process/#supported-versions supported versions policy], I think
we can close it as wontfix.
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:6>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution: wontfix
Keywords: smart_urlquote | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* component: Uncategorized => Utilities
* resolution: => wontfix
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:7>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Igor Gumenyuk):
* status: closed => new
* resolution: wontfix =>
Comment:
While python3 is great option, I don't think you're right. This bug has
also exists in 1.10, 1.11 (LTS) versions which support python2.7.
Moreover it qualifies as data loss/corruption bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:8>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
How does it cause data loss?
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:9>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Denis Pechenev):
Replying to [comment:9 Tim Graham]:
> How does it cause data loss?
The bug doesn't cause data loss, but it causes data corruption after
applying urlize() filter for strings which contain urls. So urls become
broken and there is no way to fix that without forking urlize() or
updating application to support python3 or applying some custom code. We
have Django 1.10.7 and python 2.7. According the
[https://docs.djangoproject.com/en/1.10/releases/1.10/#python-
compatibility Documentation] those versions are compatible.
Could you explain how to fix the bug without updating python to third
version? Thank you in advance.
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:10>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
Since this is a regression in Django 1.8 due to
4b8a1d2c0d1a8c5107f3aef01597db78d2a2a5ce, we could accept a patch for
Django 1.11.
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:11>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution:
Keywords: smart_urlquote | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* keywords: smart_urlquote => smart_urlquote python2
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:12>
Django
Reporter: Denis Pechenev | Owner: nobody
Component: Utilities | Version: 1.10
Severity: Normal | Resolution: wontfix
Keywords: smart_urlquote | Triage Stage: Accepted
python2 |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => wontfix
Comment:
It seems a patch for this issue isn't forthcoming.
--
Ticket URL: <https://code.djangoproject.com/ticket/28123#comment:13>