Opened 6 years ago

Last modified 6 years ago

#22425 new New feature

provide ability to abort URL resolution early

Reported by: Chris Jerdonek Owned by: nobody
Component: Core (URLs) Version: master
Severity: Normal Keywords:
Cc: chris.jerdonek@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


This is a feature request to provide a way to abort early from URL resolution if a regex doesn't match. I was able to figure out a way to do this myself using some wrapper constructs, but it seemed unnecessarily tricky and also relied on Django internals (and so may be brittle).

To illustrate the use case, consider a typical URLconf:

urlpatterns = patterns('',
    url(r'^articles/', include(article_urls)),
    url(r'^blogs/', include(blog_urls)),

While the above is common, it also seems common that one would want URL resolution to result in a 404 if an URL matches ^articles/ but does not match the first url() pattern above. This would be the case, for example, when all article URLs should be in the article_urls URLconf. However, Django would process such an URL by unnecessarily continuing to examine every pattern in urlpatterns that follows (^blogs/, etc). It would be good to have a way to achieve this 404 functionality in a performant and DRY way.

For example, the following would not be as performant or DRY as it could be because you have to duplicate the regex and because Django's URL resolver would unnecessarily need to match against ^articles/ twice:

urlpatterns = patterns('',
    url(r'^articles/', include(article_urls)),
    url(r'^articles/', page_not_found),
    url(r'^blogs/', include(blog_urls)),

Modifying account_urls and public_urls etc by including page_not_found is also not a good solution because it is not DRY (the page_not_found addition needs to be repeated) and because those URLs might be in third-party libraries.

Change History (5)

comment:1 Changed 6 years ago by Chris Jerdonek

Cc: chris.jerdonek@… added

comment:2 Changed 6 years ago by Chris Jerdonek

Another reason for implementing this in Django is that it would be easier to have the nice 404 debug page that explains why the page is a 404. The solution I came up with does not display the debug 404 because the page_not_found view occurs explicitly in the URLconf.

comment:3 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

Accepting the idea as an interesting suggestion. However, by way of design, I'd suggest a slightly different construction:

urlpatterns = patterns('',
    url(r'^articles/', include(article_urls, terminal_prefix=True)),
    url(r'^blogs/', include(blog_urls)),

That is - "if this prefix didn't match, a full URL, but the prefix matched, then abort" (terminal_prefix is a bikeshed name - happy for an alternative here).

The reason I'm suggesting this - I can't see a use case where you would include page_not_found as a url pattern *without* repeating a prefix of an included pattern. On top of that, using the include means that you don't need to re-run the prefix match; the "include()" is already matching the prefix, so it's a matter of aborting if the included pattern didn't match anything.

One thing to keep in mind during design/testing -- the interaction of this feature with the flatpages middleware (or any other response-processing middleware).

comment:4 Changed 6 years ago by Chris Jerdonek

Thanks. Yes, enhancing include() in some way like that is also what I was thinking.

Here's another scenario for consideration that is related but different. This came up in something I'm working on. Consider an URLconf of URLs that should only be accessible when logged in:

urlpatterns = patterns('',
    url(r'^private/', include(logged_in_urls)),

In this case, if a "private" URL is requested that doesn't match, I would like the following custom view to take effect: If the user isn't logged in, the user is redirected to the login page with next properly set. Otherwise, the user sees a 404.

This makes me wonder if the change should be more general than a simple boolean, something like: include(logged_in_urls, catch_all_view=login_or_404_view).

comment:5 Changed 6 years ago by Chris Jerdonek

Okay, so I found a simple way to implement this and that doesn't rely on Django internals, so maybe this feature request is no longer necessary:

def include_all(arg, namespace=None, app_name=None, catch_all_view=None):
    """Return an URLconf for inclusion that matches all URLs."""
    if catch_all_view is None:
        catch_all_view = page_not_found
    return include(patterns('',
        url('', include(arg, namespace=namespace, app_name=app_name)),
        url('', catch_all_view),

(It's possible the code above needs to be tweaked in some way, but I believe the general idea is sound.)

From my perspective, the only outstanding issues related to this are (1) making the debug 404 page show up in development instead of the real one when page_not_found is used explicitly in an URLconf, and (2) making the debug 404 page show more useful info when it renders (e.g. the URL pattern that matched and the view that executed). I can open separate issues for these as they are orthogonal to the current one.

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