Allow disabling of auto-404-redirection in LocaleMiddleware

[Alex Vandiver]

This is related to the last two comments on #17734. Specifically, if an application decides to return an explicit 404, there is no way to prevent the LocaleMiddleware from overriding this and trying the language redirect.

In those comments, it was about catch-all URL patterns. I'm running into something related, but slightly different -- we ​serve 404's for the `/` endpoint if the subdomain isn't valid, which the LocaleMiddleware unhelpfully redirects to (e.g.) /en/ which isn't any less of a 404.

Would folks be amenable to a patch which disabled the auto-404-redirect functionality in the middleware with a flag of some sort?

[Carlton Gibson]

Hi Alex.

Thanks for the report. Perhaps need an extra coffee to think through if it's going to be worth the complexity but... — is this not a middleware ordering issue? i.e. If you put your HostDomainMiddleware before LocaleMiddleware would the response returned in process_request not get sent back to the client before LocaleMiddleware ever got the chance to look at it?

[Carlton Gibson]

Looks like you've just introduced this issue here ​https://github.com/zulip/zulip/commit/536bd3188e9428993fd712ed2f0df7c160b6ad60 🤔

[Alex Vandiver]

This is certainly tied to middleware ordering -- but I don't think that swapping the order up will fix it.

Replying to Carlton Gibson:

Looks like you've just introduced this issue here ​https://github.com/zulip/zulip/commit/536bd3188e9428993fd712ed2f0df7c160b6ad60 🤔

That's the commit which introduces the need for it, and and works around its lack; you can see it cites this ticket in so doing.

As noted in that commit message, the LocaleMiddleware needs to be around the middleware that throws the 404, in order for the 404 page to be able to be properly localized. If the LocaleMiddleware is inside it, then the 404 page actually gets the last localization that the thread served, in the previous request! You can see this live if you bounce reload on ​https://nonexistent.zulipchat.com/ for a bit.

[Carlton Gibson]

Yes, I see it. TBH, I don't think a change here is worth the complexity. It's pretty niche and you can easily subclass LocaleMiddleware to skip the process_response redirect, by setting an attribute on the response in your HostDomainMiddleware and checking for it there.

[Alex Vandiver]

Fair enough. The only slight ugliness with subclassing is that one needs to repeat ​the logic that adds `Content-Language` and `Vary` headers -- skipping all of the process_response on 404s wouldn't produce the right headers. Which actually means a ​reasonable amount of code duplication.