Opened 12 years ago

Closed 12 years ago

Last modified 5 years ago

#18776 closed Bug (fixed)

urlparse do not support reverse_lazy as url arg

Reported by: Claude Paroz Owned by: nobody
Component: Python 3 Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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:
if not isinstance(url, str): str.decode(encoding, errors) .
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.

Change History (11)

comment:1 by Claude Paroz, 12 years ago

Here is some experimental code that is specifically adding the decode attribute to __proxy__ instances resulting from reverse_lazy.
(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)

comment:2 by Claude Paroz, 12 years ago

Note also that the failure on Python 3 can be reproduced with the test urlpatterns_reverse.ReverseLazyTest.test_user_permission_with_lazy_reverse

comment:3 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

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.

comment:4 by Claude Paroz, 12 years ago

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.)

comment:6 by Aymeric Augustin, 12 years ago

I commented on the pull request for that patch: https://github.com/django/django/pull/281

comment:7 by Aymeric Augustin, 12 years ago

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...

comment:8 by Aymeric Augustin <aymeric.augustin@…>, 12 years ago

Resolution: fixed
Status: newclosed

In [de3ad8bb2d14ccf878121b96e6e0359dd8b1f9d5]:

[py3] Avoided passing a lazy string to urlparse.

This causes an exception under Python 3.

Fixed #18776.

comment:9 by Peter Schmidt, 9 years ago

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

comment:10 by bugmenot, 5 years ago

This bug has regressed into Django 2.

comment:11 by Claude Paroz, 5 years ago

If you found a regression, please open a new ticket and show us some code which reproduces the failure.

Note: See TracTickets for help on using tickets.
Back to Top