mirror of
https://github.com/discourse/discourse.git
synced 2025-03-22 19:27:56 +08:00
FIX: stop logging every time invalid params are sent
Previously we were logging warning for invalid encoded params, this can cause a log flood
This commit is contained in:
parent
da39a310c3
commit
5b630f3188
app/controllers
lib/middleware
spec/requests
@ -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
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user