Add CommonMiddleware.response_redirect_class to customize the redirect class

[Matt Robenolt]

Redirecting with a permanent redirect (HttpResponsePermanentRedirect) can lead to 404s and other unexpected behavior when things move around.

For example:

In settings.py, APPEND_SLASHES = True (the default behavior)

In urls.py url(r'foo/$', ..._

A user visits example.com/foo and gets redirected to example.com/foo/ as expected.

Later, we decide that we don't like slashes in our urls, and change to APPEND_SLASHES = False and change our url route to url(r'foo$', ...

A user visits example.com/foo again (which is now the correct url) and their browser redirects them to example.com/foo/ which is now a 404.

This behavior happens specifically because Django has told it that this redirect is 100% going to happen, so the browser caches it and doesn't ask the server again. In my opinion, it's a bad idea that Django makes this assumption because it has no insight into what future plans are and whatnot.

This behavior has also existed since at least 2007, so I'm not sure how much effort needs to go into changing this moving forward for 1.8+.

[Daniele Procida]

I can confirm the behaviour, though I'm not sure what the correct behaviour should be.

[Tim Graham]

See #21587 which suggests to default RedirectView to permanent=False. There are backwards compatibility concerns so at least a deprecation would be needed. Making the redirect class a class attribute so it's more easily customizable would be a good start.

[Matt Robenolt]

How do you see this happening inside CommonMiddleware since it doesn't utilize RedirectView or anything user configurable at the moment? We can have two different middlewares, but that's really gross imo. This is just some magical behavior that users have little insight into.

[Matt Robenolt]

I can see introducing a new setting to settings.py.

APPEND_SLASHES_REDIRECT_PERMANENT = True which defaults to true, and we switch to False in 1.9 or whatever makes sense in the deprecation cycle.

Thoughts on that?

[Tim Graham]

An example of making it configurable is LocaleMiddleware.response_redirect_class.

[Matt Robenolt]

Ahh, ok, I like that.

Is it then suggested to subclass the middleware and override?

While on this topic... should the APPEND_SLASHES behavior be considered to be stripped out into it's own middleware? Or is there value in having it crammed inside CommonMiddleware?

I ask because if we introduce CommonMiddleware.response_redirect_class, it feels ambiguous. Though there's only one redirect path in CommonMiddleware, it just doesn't feel as concrete as LocaleMiddleware.

[Matt Robenolt]

Oh, I guess this would affect the PREPEND_WWW setting as well.

[Matt Robenolt]

I updated the patch to introduce CommonMiddleware.response_redirect_class so CommonMiddleware can be subclassed with the default being HttpResponsePermanentRedirect still for backwards compatibility. I'm ok with this solution and going through deprecation process to change default to HttpResponseRedirect.

[Tim Graham]

I am not convinced about changing the default redirect as I expect the majority of users with APPEND_SLASHES and PREPEND_WWW would want the SEO benefits of a 301 redirect and are not going to change their URLs after their site is deployed. This likely needs a wider discussion on django-developers.

In the meantime, I think we can move forward with adding CommonMiddleware.response_redirect_class as a separate ticket.

[Berker Peksag]

​https://github.com/django/django/pull/3466

[Tim Graham <timograham@…>]

In df0523debcc2d0984f1bc11d323f04227d4b388b:

Fixed #23531 -- Added CommonMiddleware.response_redirect_class.