diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e24baddfff7..ff5abffe881 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -252,7 +252,11 @@ class ApplicationController < ActionController::Base if show_json_errors # HACK: do not use render_json_error for topics#show if request.params[:controller] == 'topics' && request.params[:action] == 'show' - return render status: status_code, layout: false, plain: (status_code == 404 || status_code == 410) ? build_not_found_page(status_code) : message + return render( + status: status_code, + layout: false, + plain: (status_code == 404 || status_code == 410) ? build_not_found_page(status_code) : message + ) end render_json_error message, type: type, status: status_code diff --git a/lib/middleware/discourse_public_exceptions.rb b/lib/middleware/discourse_public_exceptions.rb index 4770b427934..de34d61cb3e 100644 --- a/lib/middleware/discourse_public_exceptions.rb +++ b/lib/middleware/discourse_public_exceptions.rb @@ -16,6 +16,10 @@ module Middleware exception = env["action_dispatch.exception"] response = ActionDispatch::Response.new + # Special handling for invalid params, in this case we can not re-dispatch + # the Request object has a "broken" .params which can not be accessed + exception = nil if Rack::QueryParser::InvalidParameterError === exception + if exception begin fake_controller = ApplicationController.new diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index fb64f91b0b5..03c5488f119 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -15,6 +15,33 @@ RSpec.describe ApplicationController do end end + describe 'invalid request params' do + before do + @old_logger = Rails.logger + @logs = StringIO.new + Rails.logger = Logger.new(@logs) + end + + after do + Rails.logger = @old_logger + end + + it 'should not raise a 500 (nor should it log a warning) for bad params' do + bad_str = "d\xDE".force_encoding('utf-8') + expect(bad_str.valid_encoding?).to eq(false) + + get "/latest.json", params: { test: bad_str } + + expect(response.status).to eq(400) + expect(@logs.string).not_to include('exception app middleware') + + expect(JSON.parse(response.body)).to eq( + "status" => 400, + "error" => "Bad Request" + ) + end + end + describe 'build_not_found_page' do describe 'topic not found' do