PREPEND_WWW and APPEND_SLASH settings don't work with flatpages middleware

#6167 handles APPEND_SLASH (someone didn't search before adding a ticket :P). Rewording summary to just cover PREPEND_WWW

Ah, I didn't see #6167, but I don't think it's exactly the same: this ticket concerns the use of FlatpageFallbackMiddleware whereas #6167 seems to concern the use of the flatpage view (at least from looking at the patch).

It's actually a more general problem, in that if a middleware returns a successful response code (even when no view is mapped to the current URL), then ideally we should honour the APPEND_SLASH and PREPEND_WWW settings and issue a redirect.

Correct, they are different issues (hence why I didn't close).

To make APPEND_SLASH always append a slash (on a successful response code) goes against the new behaviour. It should only try the slash if it 404s. Sure, fix PREPEND_WWW like that though (even though I think it's a stupid setting that should belong to a web server, not the Django framework).

Ah yes, what I should have said is that in CommonMiddleware's process_response() we should append a slash if it 404s and APPEND_SLASH is set.

The problem is that if you use FlatpageFallbackMiddleware, redirects do not happen even if they did before the change in [6852], as the change relies on being able to resolve the URL using the URLconf.

For example, if the URL "/foo" isn't found by FlatpageFallbackMiddleware, we should still redirect if APPEND_SLASH is set.

Patch for CommonMiddleware? to fix handling of APPEND_SLASH and PREPEND_WWW

Changed the summary back since this ticket still covers the APPEND_SLASH problem.

This patch also fixes #6167.

From a brief overview, I'm not 100% sure this fixes #6167 (it will always resolve the non-slash if you're using flatpages as the fallback in urlconf rather than using the middleware).

Go ahead and steal the tests in the other ticket, make sure this ticket fixes every case and then close #6167 as superceeded by this one. (You'll probably need some more tests for the things you're fixing)

Updated patch with unit tests.

Updated patch with slightly improved unit tests (more descriptive docstring comments).

The unit tests catch the problem reported by this ticket, as well as checking some cases in #6167, which this ticket supercedes.

Jason: In your patch, file django/middleware/common.py, line 108,

if resolve and new_url[0] == new_url[0] and ...

?

(In [8218]) Fixed #6213: flatpage view now correctly redirects if settings.APPEND_SLASH. Thanks, crankycoder.

Replying to lukeplant:

Jason: In your patch, file django/middleware/common.py, line 108, {{{ #!python if resolve and new_url[0] == new_url[0] and ... }}}

Oops, that should be:

if resolve and new_url[0] == old_url[0] and ...

This issue has still not been fixed: if settings.PREPEND_WWW is True and I have a flatpage mapped to '/', http://jasondavies.com/ doesn't redirect to http://www.jasondavies.com/

This is because in the code at the moment, a redirect only happens if the target URL can be resolved using urlresolvers.resolve() , which obviously doesn't work when FlatpageFallbackMiddleware is used. My patch fixes this problem by checking for a successful response code from any preceding middleware, and issuing a redirect if necessary as per the old behaviour.

Updated patch

The current solution also leads to an infinite "302 Found" redirect if APPEND_SLASH is True and a flatpage exists at "/".

Should I file a new bug for this?

Replying to jacob:

(In [8218]) Fixed #6213: flatpage view now correctly redirects if settings.APPEND_SLASH. Thanks, crankycoder.

Changeset [8218] also causes flatpages to trigger redirect on all URLs that don't end with a slash and return 404 response, regardless whether any page exists.

For example, if you have flatpage '/about/', flagpages are going to redirect '/about' -> '/about/', but also '/robots.txt' to '/robots.txt/' (if you don't have the file handled in urlconf), etc.