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
rails · Sep 16, 2014 · 6dbaea6 · 6dbaea6
1 parent 093f3e9
commit 6dbaea6
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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