Opened 3 years ago

Closed 19 months ago

#18373 closed Cleanup/optimization (fixed)

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

Reported by: anonymous Owned by: gnosek
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 Changed 3 years ago by willhardy

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

    from django.core import urlresolvers
    urlresolvers.resolve("/this/can/not/match/anything/")

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.

Last edited 3 years ago by willhardy (previous) (diff)

comment:2 Changed 3 years ago by aaugustin

  • Component changed from Uncategorized to Core (URLs)
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

comment:3 Changed 3 years ago by abki

  • Cc amirouche.boubekki@… added

comment:4 Changed 3 years ago by gnosek

  • Owner changed from nobody to gnosek
  • Status changed from new to assigned

comment:5 Changed 3 years ago by gnosek

  • Has patch set

comment:7 Changed 2 years ago by aaugustin

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

comment:8 Changed 19 months ago by timo

  • Patch needs improvement set

The patch no longer merges cleanly.

comment:9 Changed 19 months ago by gnosek

  • Patch needs improvement unset

comment:10 Changed 19 months ago by Honza Král <honza.kral@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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