Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Make set_locale an around_action to avoid leaking between requests (#10282)

This commit is contained in:
David Taylor 2020-07-22 17:30:26 +01:00 committed by GitHub
parent 723d7e3a61
commit bcb0e62363
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 136 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
app/controllers/application_controller.rb
View File

@ -32,7 +32,7 @@ class ApplicationController < ActionController::Base
before_action :handle_theme
before_action :set_current_user_for_logs
before_action :clear_notifications
before_action :set_locale
around_action :with_resolved_locale
before_action :set_mobile_view
before_action :block_if_readonly_mode
before_action :authorize_mini_profiler
@ -245,6 +245,8 @@ class ApplicationController < ActionController::Base
end
end
message = title = nil
with_resolved_locale(check_current_user: false) do
if opts[:custom_message_translated]
title = message = opts[:custom_message_translated]
elsif opts[:custom_message]
@ -257,16 +259,19 @@ class ApplicationController < ActionController::Base
title = I18n.t("page_not_found.title")
end
end
end
error_page_opts = { title: title, status: status_code, group: opts[:group] }
if show_json_errors
opts = { type: type, status: status_code }
with_resolved_locale(check_current_user: false) do
# Include error in HTML format for topics#show.
if (request.params[:controller] == 'topics' && request.params[:action] == 'show') || (request.params[:controller] == 'categories' && request.params[:action] == 'find_by_slug')
opts[:extras] = { html: build_not_found_page(error_page_opts) }
end
end
render_json_error message, opts
else
@ -277,11 +282,12 @@ class ApplicationController < ActionController::Base
rescue Discourse::InvalidAccess
return render plain: message, status: status_code
end
with_resolved_locale do
error_page_opts[:layout] = opts[:include_ember] ? 'application' : 'no_ember'
render html: build_not_found_page(error_page_opts)
end
end
end
# If a controller requires a plugin, it will raise an exception if that plugin is
# disabled. This allows plugins to be disabled programatically.
@ -325,19 +331,23 @@ class ApplicationController < ActionController::Base
end
end
def set_locale
if !current_user
def with_resolved_locale(check_current_user: true)
if check_current_user && current_user
locale = current_user.effective_locale
else
if SiteSetting.set_locale_from_accept_language_header
locale = locale_from_header
else
locale = SiteSetting.default_locale
end
else
locale = current_user.effective_locale
end
I18n.locale = I18n.locale_available?(locale) ? locale : SiteSettings::DefaultsProvider::DEFAULT_LOCALE
if !I18n.locale_available?(locale)
locale = SiteSettings::DefaultsProvider::DEFAULT_LOCALE
end
I18n.ensure_all_loaded!
I18n.with_locale(locale) { yield }
end
def store_preloaded(key, json)

4
spec/requests/admin/site_texts_controller_spec.rb
View File

@ -533,8 +533,8 @@ RSpec.describe Admin::SiteTextsController do
}
expect(response.status).to eq(200)
expect(Category.find(SiteSetting.staff_category_id).name).to eq(I18n.t("staff_category_name"))
expect(Topic.find(SiteSetting.guidelines_topic_id).title).to eq(I18n.t("guidelines_topic.title"))
expect(Category.find(SiteSetting.staff_category_id).name).to eq(I18n.t("staff_category_name", locale: :de))
expect(Topic.find(SiteSetting.guidelines_topic_id).title).to eq(I18n.t("guidelines_topic.title", locale: :de))
end
end
end

103
spec/requests/application_controller_spec.rb
View File

@ -628,4 +628,107 @@ RSpec.describe ApplicationController do
get "/t/#{topic.slug}/#{topic.id}"
expect(response.body).to have_tag("link", with: { rel: "canonical", href: "http://test.localhost/t/#{topic.slug}/#{topic.id}" })
end
describe "set_locale" do
# Using /bootstrap.json because it returns a locale-dependent value
def headers(locale)
{ HTTP_ACCEPT_LANGUAGE: locale }
end
context "allow_user_locale disabled" do
context "accept-language header differs from default locale" do
before do
SiteSetting.allow_user_locale = false
SiteSetting.default_locale = "en"
end
context "with an anonymous user" do
it "uses the default locale" do
get "/bootstrap.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("en.js")
end
end
context "with a logged in user" do
it "it uses the default locale" do
user = Fabricate(:user, locale: :fr)
sign_in(user)
get "/bootstrap.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("en.js")
end
end
end
end
context "set_locale_from_accept_language_header enabled" do
context "accept-language header differs from default locale" do
before do
SiteSetting.allow_user_locale = true
SiteSetting.set_locale_from_accept_language_header = true
SiteSetting.default_locale = "en"
end
context "with an anonymous user" do
it "uses the locale from the headers" do
get "/bootstrap.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("fr.js")
end
it "doesn't leak after requests" do
get "/bootstrap.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("fr.js")
expect(I18n.locale.to_s).to eq(SiteSettings::DefaultsProvider::DEFAULT_LOCALE)
end
end
context "with a logged in user" do
before do
user = Fabricate(:user, locale: :fr)
sign_in(user)
end
it "uses the user's preferred locale" do
get "/bootstrap.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("fr.js")
end
it "serves a 404 page in the preferred locale" do
get "/missingroute", headers: headers("fr")
expect(response.status).to eq(404)
expected_title = I18n.t("page_not_found.title", locale: :fr)
expect(response.body).to include(CGI.escapeHTML(expected_title))
end
end
end
context "the preferred locale includes a region" do
it "returns the locale and region separated by an underscore" do
SiteSetting.allow_user_locale = true
SiteSetting.set_locale_from_accept_language_header = true
SiteSetting.default_locale = "en"
get "/bootstrap.json", headers: headers("zh-CN")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("zh_CN.js")
end
end
context 'accept-language header is not set' do
it 'uses the site default locale' do
SiteSetting.allow_user_locale = true
SiteSetting.default_locale = 'en'
get "/bootstrap.json", headers: headers("")
expect(response.status).to eq(200)
expect(response.parsed_body['bootstrap']['locale_script']).to end_with("en.js")
end
end
end
end
end

88
spec/requests/topics_controller_spec.rb
View File

@ -1993,94 +1993,6 @@ RSpec.describe TopicsController do
end
end
describe "set_locale" do
def headers(locale)
{ HTTP_ACCEPT_LANGUAGE: locale }
end
context "allow_user_locale disabled" do
context "accept-language header differs from default locale" do
before do
SiteSetting.allow_user_locale = false
SiteSetting.default_locale = "en"
end
context "with an anonymous user" do
it "uses the default locale" do
get "/t/#{topic.id}.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(I18n.locale).to eq(:en)
end
end
context "with a logged in user" do
it "it uses the default locale" do
user = Fabricate(:user, locale: :fr)
sign_in(user)
get "/t/#{topic.id}.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(I18n.locale).to eq(:en)
end
end
end
end
context "set_locale_from_accept_language_header enabled" do
context "accept-language header differs from default locale" do
before do
SiteSetting.allow_user_locale = true
SiteSetting.set_locale_from_accept_language_header = true
SiteSetting.default_locale = "en"
end
context "with an anonymous user" do
it "uses the locale from the headers" do
get "/t/#{topic.id}.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(I18n.locale).to eq(:fr)
end
end
context "with a logged in user" do
it "uses the user's preferred locale" do
user = Fabricate(:user, locale: :fr)
sign_in(user)
get "/t/#{topic.id}.json", headers: headers("fr")
expect(response.status).to eq(200)
expect(I18n.locale).to eq(:fr)
end
end
end
context "the preferred locale includes a region" do
it "returns the locale and region separated by an underscore" do
SiteSetting.allow_user_locale = true
SiteSetting.set_locale_from_accept_language_header = true
SiteSetting.default_locale = "en"
get "/t/#{topic.id}.json", headers: headers("zh-CN")
expect(response.status).to eq(200)
expect(I18n.locale).to eq(:zh_CN)
end
end
context 'accept-language header is not set' do
it 'uses the site default locale' do
SiteSetting.allow_user_locale = true
SiteSetting.default_locale = 'en'
get "/t/#{topic.id}.json", headers: headers("")
expect(response.status).to eq(200)
expect(I18n.locale).to eq(:en)
end
end
end
end
describe "read only header" do
it "returns no read only header by default" do
get "/t/#{topic.id}.json"