Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#21043 closed Bug (fixed)

resolve doesn't handle lazily evaluated reverses

Reported by: Keryn Knight <django@…> Owned by:
Component: Core (URLs) Version: dev
Severity: Normal Keywords: urlresolvers resolve reverse
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

the following code generates an exception, which isn't documented as a caveat of using reverse_lazy (or being given a lazy proxy when you might've expected to receive a string, because the code is external and outside your control):

>>> from django.core.urlresolvers import reverse_lazy, resolve
>>> proxy = reverse_lazy('admin:index')
>>> resolve(proxy)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/django/core/urlresolvers.py", line 453, in resolve
    return get_resolver(urlconf).resolve(path)
  File "/django/core/urlresolvers.py", line 315, in resolve
    match = self.regex.search(path)
TypeError: expected string or buffer

The fix is dirt simple in userland, because you can just call str/unicode/force_unicode on the proxy to evaluate it into a path. The problem lies in the path given to resolve being directly handed off to the re module, without being checked for validity.

>>> from django.core.urlresolvers import reverse_lazy, resolve
>>> proxy = reverse_lazy('admin:index')
>>> from django.utils.encoding import force_unicode
>>> resolve(force_unicode(proxy))
ResolverMatch(func=<function index at 0x10ae0b230>, args=(), kwargs={}, url_name='index', app_name='admin', namespace='admin')

Both the above assume a project with django.contrib.admin in INSTALLED_APPS, because it's the easiest way of ensuring there's views to resolve :)

I think that the issue should be documented; it's not something that's common, and if you encounter it, one would hope you've got a reasonable idea about where the problem lies, and it's probably safer to document it than promote forced evaluation into the resolve method of RegexURLResolver Marking as easy pickings on that basis.

Change History (7)

comment:1 by DanRJohnson, 11 years ago

Owner: changed from nobody to DanRJohnson
Status: newassigned

comment:2 by DanRJohnson, 11 years ago

Component: DocumentationCore (URLs)
Needs tests: set
Triage Stage: UnreviewedAccepted

I can confirm this issue.

However, I think that resolve should be fixed to work correctly with the proxy object returned by reverse_lazy. I think we should fix the issue with resolve not evaluating the proxy object instead of documenting a work around.

comment:3 by DanRJohnson, 11 years ago

Has patch: set
Type: Cleanup/optimizationBug

I've created a patch and created a pull request.

https://github.com/django/django/pull/1570

comment:4 by DanRJohnson, 11 years ago

Owner: DanRJohnson removed
Status: assignednew

comment:5 by DanRJohnson, 11 years ago

Needs tests: unset

comment:6 by grue, 11 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top