[Django] #18776: urlparse do not support reverse_lazy as url arg
Django
--------------------------------------+------------------------
Reporter: claudep | Owner: nobody
Type: Bug | Status: new
Component: Python 3 | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+------------------------
With Python 2, we could pass a reverse_lazy result ({{{__proxy__}}}
instance) to urlparse and the string conversion was done without problem.
With Python 3, this is failing, because urlparse is feeding url argument
to a _coerce_args function which basically run:[[BR]]
{{{ if not isinstance(url, str): str.decode(encoding, errors) }}}.[[BR]]
And this is failing with an {{{AttributeError: '__proxy__' object has no
attribute 'decode'}}}.
I think it is valuable not to have to explicitely force_text(url) each
time we want to pass it to urlparse.
--
Ticket URL: <https://code.djangoproject.com/ticket/18776>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Here is some experimental code that is specifically adding the decode
attribute to `__proxy__` instances resulting from reverse_lazy.[[BR]]
(Warning: this might be a horrible hack, don't yell at me!)
{{{
from functools import wraps
def reverse_lazy(*args, **kwargs):
@wraps(reverse)
def wrapper(*args, **kw):
proxy = lazy(reverse, str)(*args, **kw)
# Needed in Python 3 to pass the _coerce_args function in urlparse
proxy.decode = lambda x, y: str(x)
return proxy
return wrapper(*args, **kwargs)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:1>
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Note also that the failure on Python 3 can be reproduced with the test
`urlpatterns_reverse.ReverseLazyTest.test_user_permission_with_lazy_reverse`
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:2>
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* stage: Unreviewed => Accepted
Comment:
Since `reverse_lazy` is just defined at `reverse_lazy = lazy(reverse,
str)` I strongly believe this should be fixed in `lazy`, not only in
`reverse_lazy`.
Otherwise any other use of `lazy` would still be vulnerable to the same
problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:3>
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Mmmh... it's even worse than that, because even if we solve the issue at
lazy level, the _coerce_args function will still break with a
`TypeError("Cannot mix str and non-str arguments")` if we pass a non-empty
scheme argument. But who wrote this _coerce_args function!!
So now either we find a way to make `isinstance(<__proxy__ instance>, str)
== True`, or we have to find some workaround (custom urlparse in
utils.http, document the limitation, etc.)
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:4>
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
An easy solution was :
https://github.com/pelletier/django/commit/22987ddf66ed2af39b8df920e3e8501af8782819
But force_text is also good.
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:5>
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I commented on the pull request for that patch:
https://github.com/django/django/pull/281
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:6>
Django
Type: Bug | Status: new
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Given Claude's decent but unsuccessful efforts toward a "correct" fix, I
think we should just go for the simplest solution and just break the lazy
layer before calling urlparse.
If the same problem crops up somewhere else, we can always revisit this
decision...
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:7>
Django
Reporter: claudep | Owner: nobody
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [de3ad8bb2d14ccf878121b96e6e0359dd8b1f9d5]:
{{{
#!CommitTicketReference repository=""
revision="de3ad8bb2d14ccf878121b96e6e0359dd8b1f9d5"
[py3] Avoided passing a lazy string to urlparse.
This causes an exception under Python 3.
Fixed #18776.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:8>
Django
Reporter: claudep | Owner: nobody
Type: Bug | Status: closed
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by pzrq):
Re: https://code.djangoproject.com/ticket/18776#comment:4
In case anyone is interested (came up through reviewing this from #24097),
the original author can be found here:
https://hg.python.org/cpython/diff/e44410e5928e/Lib/urllib/parse.py
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:9>
Django
Reporter: Claude Paroz | Owner: nobody
Type: Bug | Status: closed
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by bugmenot):
This bug has regressed into Django 2.
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:10>
Django
Reporter: Claude Paroz | Owner: nobody
Type: Bug | Status: closed
Component: Python 3 | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
If you found a regression, please open a new ticket and show us some code
which reproduces the failure.
--
Ticket URL: <https://code.djangoproject.com/ticket/18776#comment:11>