From 6dbaea6382a46e516a6d21a2e91e4c2eb2fa6ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 15 Sep 2014 11:07:56 -0300 Subject: [PATCH] Merge pull request #16886 from yuki24/bugfix-bad-request-from-public-exception-4-1-stable [4-1-stable] Fix a bug where malformed query strings lead to 500 --- actionpack/CHANGELOG.md | 7 +++++++ .../lib/action_dispatch/middleware/public_exceptions.rb | 6 +++++- actionpack/test/dispatch/show_exceptions_test.rb | 8 +++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index be1deea53b863..01786c2ceb5e9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix a bug where malformed query strings lead to 500. + + fixes #11502. + + *Yuki Nishijima* + + ## Rails 4.0.10 (September 11, 2014) ## * Return an absolute instead of relative path from an asset url in the case diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb index cbb2d475b1e03..4159dfda178bd 100644 --- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb @@ -9,8 +9,12 @@ def initialize(public_path) def call(env) status = env["PATH_INFO"][1..-1] request = ActionDispatch::Request.new(env) - content_type = request.formats.first body = { :status => status, :error => Rack::Utils::HTTP_STATUS_CODES.fetch(status.to_i, Rack::Utils::HTTP_STATUS_CODES[500]) } + content_type = begin + request.formats.first + rescue ActionController::BadRequest + Mime::HTML + end render(status, content_type, body) end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 38bd234f3729e..0392557f047a0 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -8,7 +8,7 @@ def call(env) case req.path when "/not_found" raise AbstractController::ActionNotFound - when "/bad_params" + when "/bad_params", "/bad_params?x[y]=1&x[y][][w]=2" raise ActionDispatch::ParamsParser::ParseError.new("", StandardError.new) when "/method_not_allowed" raise ActionController::MethodNotAllowed @@ -53,6 +53,12 @@ def call(env) get "/unknown_http_method", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_equal "", body + + # Use #post instead of #get as Rack::Test::Session parses + # a query string before ActionDispatch::Request does it. + post "/bad_params?x[y]=1&x[y][][w]=2", {}, {'action_dispatch.show_exceptions' => true} + assert_response 400 + assert_equal "400 error fixture\n", body end test "localize rescue error page" do