Instead of adding another middleware, perhaps we can use an
set_locale or even an
after_action to reset the locale?
@tgxworld yes I would prefer to keep this inside rails if possible. The problem is, what do we ‘reset the locale’ to at the end of a request? The default may vary per-site on a multisite cluster, and we don’t know which site will be requested next.
I guess one option is ‘reset to english’ after every request, but that means all 404 pages will always be served in English, regardless of the site locale.
Shouldn’t we always reset the locale to the default when the app is booted? Setting of the request locale is done per locale. I’m not quite familiar with the code but is there something special about our rails 404 page that prevents us from setting the locale before rendering?
Shouldn’t we always reset the locale to the default when the app is booted?
The problem is that the default locale varies per-site. On boot it is
en, then it is changed during a
before_action (and also when running sidekiq jobs)
is there something special about our rails 404 page that prevents us from setting the locale before rendering?
It seems like when an
ActionController::RoutingError is raised, it does not run any
- Updated to be an around_action
- manually called that action when rendering errors
- moved locale specs from topic_controller_spec to application_controller_spec
- cleaned up some specs which were relying on the leak
The title of this pull request changed from “FIX: Reset locale before each request to avoid leaking on 404 pages” to "FIX: Make set_locale an around_action to avoid leaking between requests
Do we need to check if
true here? I see it being set to
true in the tests but it doesn’t seem to be used in the controller.
We have to set it
true in the tests because
set_locale_from_accept_language_header has a validation for it.
SiteSetting.allow_user_locale is not used in the controller directly, but it’s checked in
Got it thanks for clarifying.
PR looks good to me