From 9d4009b0e845e3c2bbc4ed0e6959271183f3ceca Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Tue, 29 Sep 2020 21:42:45 +0200 Subject: [PATCH] FIX: Use correct locale for error messages (#10776) Error messages for exceeded rate limits and invalid parameters always used the English locale instead of the default locale or the current user's locale. --- app/controllers/application_controller.rb | 36 ++++++++------ spec/requests/application_controller_spec.rb | 52 ++++++++++++++++++++ 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f8cc316c5ce..932fb56da52 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -152,13 +152,15 @@ class ApplicationController < ActionController::Base rescue_from RateLimiter::LimitExceeded do |e| retry_time_in_seconds = e&.available_in - render_json_error( - e.description, - type: :rate_limit, - status: 429, - extras: { wait_seconds: retry_time_in_seconds }, - headers: { 'Retry-After': retry_time_in_seconds }, - ) + with_resolved_locale do + render_json_error( + e.description, + type: :rate_limit, + status: 429, + extras: { wait_seconds: retry_time_in_seconds }, + headers: { 'Retry-After': retry_time_in_seconds } + ) + end end rescue_from Discourse::NotLoggedIn do |e| @@ -170,11 +172,15 @@ class ApplicationController < ActionController::Base end rescue_from Discourse::InvalidParameters do |e| - message = I18n.t('invalid_params', message: e.message) + opts = { + custom_message: 'invalid_params', + custom_message_params: { message: e.message } + } + if (request.format && request.format.json?) || request.xhr? || !request.get? - rescue_discourse_actions(:invalid_parameters, 400, include_ember: true, custom_message_translated: message) + rescue_discourse_actions(:invalid_parameters, 400, opts.merge(include_ember: true)) else - rescue_discourse_actions(:not_found, 400, custom_message_translated: message) + rescue_discourse_actions(:not_found, 400, opts) end end @@ -238,10 +244,8 @@ class ApplicationController < ActionController::Base 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] - title = message = I18n.t(opts[:custom_message]) + if opts[:custom_message] + title = message = I18n.t(opts[:custom_message], opts[:custom_message_params] || {}) else message = I18n.t(type) if status_code == 403 @@ -329,8 +333,8 @@ class ApplicationController < ActionController::Base end def with_resolved_locale(check_current_user: true) - if check_current_user && current_user - locale = current_user.effective_locale + if check_current_user && (user = current_user rescue nil) + locale = user.effective_locale else if SiteSetting.set_locale_from_accept_language_header locale = locale_from_header diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 1296935f63f..c5a2aca8787 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -647,6 +647,58 @@ RSpec.describe ApplicationController do expect(response.body).to have_tag("link", with: { rel: "canonical", href: "http://test.localhost/t/#{topic.slug}/#{topic.id}" }) end + context "default locale" do + before do + SiteSetting.default_locale = :fr + sign_in(Fabricate(:user)) + end + + after do + I18n.reload! + end + + context "with rate limits" do + before { RateLimiter.enable } + after { RateLimiter.disable } + + it "serves a LimitExceeded error in the preferred locale" do + SiteSetting.max_likes_per_day = 1 + post1 = Fabricate(:post) + post2 = Fabricate(:post) + override = TranslationOverride.create( + locale: "fr", + translation_key: "rate_limiter.by_type.create_like", + value: "French LimitExceeded error message" + ) + I18n.reload! + + post "/post_actions.json", params: { + id: post1.id, post_action_type_id: PostActionType.types[:like] + } + expect(response.status).to eq(200) + + post "/post_actions.json", params: { + id: post2.id, post_action_type_id: PostActionType.types[:like] + } + expect(response.status).to eq(429) + expect(response.parsed_body["errors"].first).to eq(override.value) + end + end + + it "serves an InvalidParameters error with the default locale" do + override = TranslationOverride.create( + locale: "fr", + translation_key: "invalid_params", + value: "French InvalidParameters error message" + ) + I18n.reload! + + get "/search.json", params: { q: "hello\0hello" } + expect(response.status).to eq(400) + expect(response.parsed_body["errors"].first).to eq(override.value) + end + end + describe "set_locale" do # Using /bootstrap.json because it returns a locale-dependent value def headers(locale)