Merge pull request #36196 from st0012/fix-29947

Hide malformed parameters from error page

Accidentally merged this to 6-0-stable so forward porting it to master
here instead.

7 files changed, 37 insertions, 4 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 9fcff6a6ca..1fb0ee126a 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   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) ##
 
 Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/actionpack/CHANGELOG.md) for previous changes.
diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb
index a03650254e..0e8af80996 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_view.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb
@@ -56,5 +56,13 @@ module ActionDispatch
     def protect_against_forgery?
       false
     end
+
+    def params_valid?
+      begin
+        @request.parameters
+      rescue ActionController::BadRequest
+        false
+      end
+    end
   end
 end
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb
index 49b1e83551..04271d8e8a 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb
+++ b/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>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb
index 396768ecee..ca42a6fa8b 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb
@@ -1,5 +1,5 @@
 <%
-  clean_params = @request.filtered_parameters.clone
+  clean_params = params_valid? ? @request.filtered_parameters.clone : {}
   clean_params.delete("action")
   clean_params.delete("controller")
 
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb
index 999e84e4d6..57cdcf9aaf 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb
@@ -1,7 +1,7 @@
 <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>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.text.erb
index 603de54b8b..d3265563a8 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.text.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.text.erb
@@ -1,5 +1,5 @@
 <%= @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 %>
 
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index 5ae8a20ae4..3e57e8f4d9 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -620,4 +620,23 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
       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