From 3f81b3753ffdca8617422e518e1fddd581f5a712 Mon Sep 17 00:00:00 2001
From: Agis Anastasopoulos <agis.anast@gmail.com>
Date: Mon, 19 Oct 2015 17:18:09 +0300
Subject: Show helpful messages on invalid param. encodings

Prior to this change, given a route:

    # config/routes.rb
    get ':a' => "foo#bar"

If one pointed to http://example.com/%BE (param `a` has invalid encoding),
a `BadRequest` would be raised with the following non-informative message:

    ActionController::BadRequest

From now on the message displayed is:

    Invalid parameter encoding: hi => "\xBE"

Fixes #21923.
---
 actionpack/CHANGELOG.md                              |  7 +++++++
 actionpack/lib/action_controller/metal/exceptions.rb |  8 +++-----
 actionpack/lib/action_dispatch/http/request.rb       |  6 +++---
 actionpack/test/dispatch/request_test.rb             | 14 ++++++++++++++
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index b9b771b930..58af62ee76 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Show helpful message in `BadRequest` exceptions due to invalid path
+    parameter encodings.
+
+    Fixes #21923.
+
+    *Agis Anastasopoulos*
+
 *   Deprecate `config.static_cache_control` in favor of
     `config.public_file_server.headers`
 
diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb
index 18e003741d..5260dc0336 100644
--- a/actionpack/lib/action_controller/metal/exceptions.rb
+++ b/actionpack/lib/action_controller/metal/exceptions.rb
@@ -5,12 +5,10 @@ module ActionController
   class BadRequest < ActionControllerError #:nodoc:
     attr_reader :original_exception
 
-    def initialize(type = nil, e = nil)
-      return super() unless type && e
-
-      super("Invalid #{type} parameters: #{e.message}")
+    def initialize(msg = nil, e = nil)
+      super(msg)
       @original_exception = e
-      set_backtrace e.backtrace
+      set_backtrace e.backtrace if e
     end
   end
 
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index bf20a33d36..c6ab4dbc9a 100644
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -65,7 +65,7 @@ module ActionDispatch
       path_parameters.each do |key, value|
         next unless value.respond_to?(:valid_encoding?)
         unless value.valid_encoding?
-          raise ActionController::BadRequest, "Invalid parameter: #{key} => #{value}"
+          raise ActionController::BadRequest, "Invalid parameter encoding: #{key} => #{value.inspect}"
         end
       end
     end
@@ -341,7 +341,7 @@ module ActionDispatch
         set_header k, Request::Utils.normalize_encode_params(super || {})
       end
     rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
-      raise ActionController::BadRequest.new(:query, e)
+      raise ActionController::BadRequest.new("Invalid query parameters: #{e.message}", e)
     end
     alias :query_parameters :GET
 
@@ -357,7 +357,7 @@ module ActionDispatch
       self.request_parameters = Request::Utils.normalize_encode_params(super || {})
       raise
     rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
-      raise ActionController::BadRequest.new(:request, e)
+      raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}", e)
     end
     alias :request_parameters :POST
 
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index af2ed24f43..dfedc8ae25 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -961,6 +961,20 @@ class RequestParameters < BaseRequestTest
     end
   end
 
+  test "path parameters with invalid UTF8 encoding" do
+    request = stub_request(
+      "action_dispatch.request.path_parameters" => { foo: "\xBE" }
+    )
+
+    err = assert_raises(ActionController::BadRequest) do
+      request.check_path_parameters!
+    end
+
+    assert_match "Invalid parameter encoding", err.message
+    assert_match "foo", err.message
+    assert_match "\\xBE", err.message
+  end
+
   test "parameters not accessible after rack parse error of invalid UTF8 character" do
     request = stub_request("QUERY_STRING" => "foo%81E=1")
 
-- 
cgit v1.2.3