Warnings regarding 404s logged for URLs missing trailing slashes

[JK Laiho]

I recently deployed a project into production using Django 1.9.2 and started getting strange logged warning messages from Sentry for 404's. Looking into it, this occurred in django.core.handers.base.BaseHandler.get_response and was related to people visiting URLs without trailing slashes.

I compared the behaviour against an earlier, similarly configured project still running Django 1.7.x and this didn't occur there.

Digging deeper, it seems that commit ​434d309e to fix 24720 inside CommonMiddleware was causing this. In lines 56-66 (58-68 in 1.9.2), the path is only checked for a missing slash if the prerequisites for PREPEND_WWW processing are met, since they are indented beneath it. This doesn't really make sense, since the two settings are not interdependent.

As a result, an Http404 is raised after request middleware processing in BaseHandler.get_response, at which point a warning is logged—for every single request for a path without a trailing slash.

APPEND_SLASH still takes effect eventually, but only when CommonMiddleware is called again, this time for process_response, whereupon the normal redirect gets done. Thanks to this, everything appears to function normally for the end user, but unnecessary 404 warnings end up getting logged. (Though if APPEND_SLASH was False, you'd probably want them logged.)

It seems to me that CommonMiddleware.process_request needs a bit of reworking to run the checks for PREPEND_WWW and APPEND_SLASH independently, and to determine the need for a redirect based on whether at least one of these is necessary, still fulfilling the purpose of 24720. I've provisionally marked the ticket as "easy pickings", as it seems to be that way from my admittedly limited research into this.

[JK Laiho]

A further note: I'm naturally using URL reversing judiciously in all of my templates so that the URLs that people actually click on have trailing slashes already in place, but for some reason some people still type some URLs by hand, leaving out the trailing slashes.

[Emett Speer]

I have created a fix: ​https://github.com/the-kid89/django/tree/ticket_26293

[Emett Speer]

Pull request: ​https://github.com/django/django/pull/6227

[JK Laiho]

I haven't tested your changes, but by my reading of the code, it looks incorrect. After your unindentation, process_request now always returns a HttpResponsePermanentRedirect, as long as the forbidden UA check at the top passes. It should only do that when either the slash has been appended or "www" has been prepended.

[Emett Speer]

You are correct. My patch just breaks more things. My guess for the logging is because it first has to 404 before it tries APPEND_SLASH. I will have to look into this much further then I thought I would.

Replying to jklaiho:

I haven't tested your changes, but by my reading of the code, it looks incorrect. After your unindentation, process_request now always returns a HttpResponsePermanentRedirect, as long as the forbidden UA check at the top passes. It should only do that when either the slash has been appended or "www" has been prepended.

[Sam]

PR Submitted including regression test. Tests pass and docs build without issue.

patch: ​https://github.com/ieatkittens/django/tree/ticket_26293

[Tim Graham <timograham@…>]

In 9390da7:

Fixed #26293 -- Fixed CommonMiddleware to process PREPEND_WWW and APPEND_SLASH independently.

[Tim Graham <timograham@…>]

In ccc367fd:

[1.9.x] Fixed #26293 -- Fixed CommonMiddleware to process PREPEND_WWW and APPEND_SLASH independently.

Backport of 9390da7fb6e251eaa9a785692f987296cb14523f from master