Opened 12 years ago

Closed 11 years ago

#18373 closed Cleanup/optimization (fixed)

Thoroughly misleading error page when resolve() fails on a different URL

Reported by: anonymous Owned by: Grzegorz Nosek
Component: Core (URLs) Version: 1.3
Severity: Normal Keywords:
Cc: amirouche.boubekki@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I just spent a few hours tracking down a display bug in djangorestframework (https://github.com/tomchristie/django-rest-framework/pull/211).

This would have been *much* simpler if the debugging 404 error page hadn't been presenting this entirely irrelevant piece of information the whole time:

"The current URL, api/servers/, didn't match any of these."

As far as I can tell, the error page is getting that URL from the request's path_info attribute, when the real problem was that djangorestframework was doing "resolve(request.path)".

It would be better if the resolver set the URL it was trying to resolve on the Http404 exception object and the debugging error template retrieved it from there, rather than assuming that every 404 it is asked to display relates to the current page.

Change History (10)

comment:1 by Will Hardy, 12 years ago

To reproduce, take any django view, add the following:

    from django.core import urlresolvers
    urlresolvers.resolve("/abcde/")

and observe the misleading debug page.

This will occur whenever a library calls resolve() with something other than request.path_info and the URL is not matched.

Version 0, edited 12 years ago by Will Hardy (next)

comment:2 by Aymeric Augustin, 12 years ago

Component: UncategorizedCore (URLs)
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

comment:3 by Amirouche, 12 years ago

Cc: amirouche.boubekki@… added

comment:4 by Grzegorz Nosek, 12 years ago

Owner: changed from nobody to Grzegorz Nosek
Status: newassigned

comment:5 by Grzegorz Nosek, 12 years ago

Has patch: set

comment:7 by Aymeric Augustin, 12 years ago

This is related to #14343 but probably worth fixing independently.

comment:8 by Tim Graham, 11 years ago

Patch needs improvement: set

The patch no longer merges cleanly.

comment:9 by Grzegorz Nosek, 11 years ago

Patch needs improvement: unset

comment:10 by Honza Král <honza.kral@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 79558c787ebfd0fe723acb061a375b19a27f18cd:

Fixed #18373 - improved handling of Resolver404s from views

When django.core.urlresolvers.resolve was called from a view, failed
and the exception was propagated and rendered by technical_404_response,
the URL mentioned on the page was the current URL instead of the URL
passed to resolve().

Fixed by using the path attribute from the Resolver404 exception instead
of request.path_info. Also cleaned up the exceptions to use standard
named parameters instead of stuffing a dict in args[0]

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