Merge pull request #36196 from st0012/fix-29947

Hide malformed parameters from error page

actionpack/CHANGELOG.md

+*   Fix a bug where DebugExceptions throws an error when malformed query parameters are provided
+
+    *Yuki Nishijima*, *Stan Lo*
+
 ## Rails 6.0.0.rc1 (April 24, 2019) ##
 
 *   Make system tests take a failed screenshot in a `before_teardown` hook

actionpack/lib/action_dispatch/middleware/debug_view.rb

@@ -56,5 +56,13 @@ def render(*)
     def protect_against_forgery?
       false
     end
+
+    def params_valid?
+      begin
+        @request.parameters
+      rescue ActionController::BadRequest
+        false
+      end
+    end
   end
 end

actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb

@@ -6,7 +6,9 @@
 <% end %>
 
 <h2 style="margin-top: 30px">Request</h2>
-<p><b>Parameters</b>:</p> <pre><%= debug_params(@request.filtered_parameters) %></pre>
+<% if params_valid? %>
+  <p><b>Parameters</b>:</p> <pre><%= debug_params(@request.filtered_parameters) %></pre>
+<% end %>
 
 <div class="details">
   <div class="summary"><a href="#" onclick="return toggleSessionDump()">Toggle session dump</a></div>

actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb

 <%
-  clean_params = @request.filtered_parameters.clone
+  clean_params = params_valid? ? @request.filtered_parameters.clone : {}
   clean_params.delete("action")
   clean_params.delete("controller")
 

actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb

 <header>
   <h1>
     <%= @exception.class.to_s %>
-    <% if @request.parameters['controller'] %>
+    <% if params_valid? && @request.parameters['controller'] %>
       in <%= @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%= @request.parameters['action'] %><% end %>
     <% end %>
   </h1>

actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.text.erb

 <%= @exception.class.to_s %><%
-  if @request.parameters['controller']
+  if params_valid? && @request.parameters['controller']
 %> in <%= @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%= @request.parameters['action'] %><% end %>
 <% end %>
 

actionpack/test/dispatch/debug_exceptions_test.rb

@@ -620,4 +620,23 @@ def call(env)
       assert_select 'input[value="Action 2"]'
     end
   end
+
+  test "debug exceptions app shows diagnostics when malformed query parameters are provided" do
+    @app = DevelopmentApp
+
+    get "/bad_request?x[y]=1&x[y][][w]=2"
+
+    assert_response 400
+    assert_match "ActionController::BadRequest", body
+  end
+
+  test "debug exceptions app shows diagnostics when malformed query parameters are provided by XHR" do
+    @app = DevelopmentApp
+    xhr_request_env = { "action_dispatch.show_exceptions" => true, "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" }
+
+    get "/bad_request?x[y]=1&x[y][][w]=2", headers: xhr_request_env
+
+    assert_response 400
+    assert_match "ActionController::BadRequest", body
+  end
 end

railties/test/application/middleware/exceptions_test.rb

@@ -136,5 +136,21 @@ def index
       assert_match(/boooom/, last_response.body)
       assert_match(/測試テスト시험/, last_response.body)
     end
+
+    test "displays diagnostics message when malformed query parameters are provided" do
+      controller :foo, <<-RUBY
+        class FooController < ActionController::Base
+          def index
+          end
+        end
+      RUBY
+
+      app.config.action_dispatch.show_exceptions = true
+      app.config.consider_all_requests_local = true
+
+      get "/foo?x[y]=1&x[y][][w]=2"
+      assert_equal 400, last_response.status
+      assert_match "Invalid query parameters", last_response.body
+    end
   end
 end