[Django] #26005: uri_to_iri() broken
Django
-------------------------------+--------------------
Reporter: Chronial | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (URLs) | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The current implementation of uri_to_iri is incorrect.
In step two of [http://tools.ietf.org/html/rfc3987.html#section-3.2 the
algorithm], it should:
> Convert all percent-encodings ("%" followed by two hexadecimal
digits) to the corresponding octets, except those corresponding to
"%", characters in "reserved", and characters in US-ASCII not
allowed in URIs.
But instead it just runs an unquote
([https://github.com/django/django/blob/dbb0df2a0ec5bee80bee336fc81408efb30b7e47/django/utils/encoding.py#L199
source])
This also makes this statement from
[https://docs.djangoproject.com/en/1.9/ref/unicode/#uri-and-iri-handling
the docs] a lie:
> Both iri_to_uri() and uri_to_iri() functions are idempotent, which means
the following is always true:
> uri_to_iri(uri_to_iri(some_string)) == uri_to_iri(some_string)
But at the moment
{{{
uri_to_iri(uri_to_iri("%2525")) == "%" != "%25" == uri_to_iri("%2525")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26005>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------+------------------------------------
Reporter: Chronial | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* component: Core (URLs) => Utilities
* needs_tests: => 0
* version: 1.9 => 1.8
* needs_docs: => 0
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Comment:
If you could provide a patch soon, we might backport it to 1.8 given this
function was introduced there.
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:1>
Django
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by varunnaganathan):
* status: new => assigned
* owner: nobody => varunnaganathan
Comment:
Hi,i wanted to work on this.Added a small tweak to avoid converting the
percent encoding for '%' i.e. '%25'.basically I'm replacing all instances
of '%25' in the uri string with '%'.I tried a few tests and they seem to
work.Do let me know if that's fine.I'll organize the code and post a PR.
I'm pretty new to contributing to django so I'm sorry if I wasn't suppose
to self assign this bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:2>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by varunnaganathan):
Also I have a doubt regarding the RFC guidelines.When RFC 3987 states that
the percent encoding corresponding to '%' i.e. '%25' must not be
converted to its octet , does that mean that uri_to_iri("%2525") should
return %2525 only as the result?
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:3>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by varunnaganathan):
Created a trial Pull Request
https://github.com/django/django/pull/6085
Please review
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:4>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chronial):
@timgraham I created A PR against 1.8.x:
https://github.com/django/django/pull/6426
This now correctly implements the algorithm from the RFC, except for step
4: The implementation is supposed to re-encode some unicode characters
“that are not appropriate” for IRIs. I added a note about that to the
docstring.
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:5>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chronial):
New PR against master: https://github.com/django/django/pull/6428. I also
didn't foresee what that this fix would affect the testing infrastructure
so I didn't run the whole test suite last time. Now everything should
work.
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:6>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:7>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by phildini):
* needs_better_patch: 0 => 1
Comment:
Setting the 'patch_needs_improvement' bit as there are failing checks and
open questions on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:8>
Django
---------------------------+-------------------------------------------
Reporter: Chronial | Owner: varunnaganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Chronial):
Replying to [comment:8 phildini]:
> Setting the 'patch_needs_improvement' bit as there are failing checks
and open questions on the PR.
What are the open questions?
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:9>
Django
Reporter: Chronial | Owner: varun naganathan
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:10>
Django
---------------------------+---------------------------------------------
Reporter: Chronial | Owner: varun naganathan
Component: Utilities | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"03281d8fe7a32f580a85235659d4fbb143eeb867" 03281d8]:
{{{
#!CommitTicketReference repository=""
revision="03281d8fe7a32f580a85235659d4fbb143eeb867"
Fixed #26005 -- Fixed some percent decoding cases in uri_to_iri().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26005#comment:11>