diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index d80d1d714c20b4b5f3f8518d9bf692ee663bd527..ae875eb830a2b5b9a52fa43c7e07e7ae2da81aef 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -57,7 +57,7 @@ def parameters query_parameters.dup end params.merge!(path_parameters) - params = set_binary_encoding(params) + params = set_binary_encoding(params, params[:controller], params[:action]) set_header("action_dispatch.request.parameters", params) params end @@ -66,6 +66,7 @@ def parameters def path_parameters=(parameters) #:nodoc: delete_header("action_dispatch.request.parameters") + parameters = set_binary_encoding(parameters, parameters[:controller], parameters[:action]) # If any of the path parameters has an invalid encoding then # raise since it's likely to trigger errors further on. Request::Utils.check_param_encoding(parameters) @@ -85,9 +86,10 @@ def path_parameters private - def set_binary_encoding(params) - action = params[:action] - if binary_params_for?(action) + def set_binary_encoding(params, controller, action) + return params unless controller && controller.valid_encoding? + + if binary_params_for?(controller, action) ActionDispatch::Request::Utils.each_param_value(params) do |param| param.force_encoding ::Encoding::ASCII_8BIT end @@ -95,8 +97,8 @@ def set_binary_encoding(params) params end - def binary_params_for?(action) - controller_class.binary_params_for?(action) + def binary_params_for?(controller, action) + controller_class_for(controller).binary_params_for?(action) rescue NameError false end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 94b0d4880cb0a0b31c7f70708024b42049b11539..0b38ab7afb5685ae2d7cef0fb88d26ed3c0b9498 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -76,10 +76,13 @@ def self.binary_params_for?(action); false; end def controller_class params = path_parameters + params[:action] ||= "index" + controller_class_for(params[:controller]) + end - if params.key?(:controller) - controller_param = params[:controller].underscore - params[:action] ||= "index" + def controller_class_for(name) + if name + controller_param = name.underscore const_name = "#{controller_param.camelize}Controller" ActiveSupport::Dependencies.constantize(const_name) else diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 809bfcbe8ad1bddc4b9490d8652b877db77f4562..9987a9bfa1251c53d3e44ecb337ba03f6e20f85e 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -43,6 +43,10 @@ def serve(req) req.path_info = "/" + req.path_info unless req.path_info.start_with? "/" end + parameters = route.defaults.merge parameters.transform_values { |val| + val.dup.force_encoding(::Encoding::UTF_8) + } + req.path_parameters = set_params.merge parameters status, headers, body = route.app.serve(req) @@ -67,6 +71,7 @@ def recognize(rails_req) rails_req.path_info = match.post_match.sub(/^([^\/])/, '/\1') end + parameters = route.defaults.merge parameters yield(route, parameters) end end @@ -119,7 +124,7 @@ def find_routes(req) routes.map! { |r| match_data = r.path.match(req.path_info) - path_parameters = r.defaults.dup + path_parameters = {} match_data.names.zip(match_data.captures) { |name, val| path_parameters[name.to_sym] = Utils.unescape_uri(val) if val } diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index fefb84e095d00b7bb1861de874d72dd01a4ed4ec..f09051b306e481b507e9292d0440e113cc89c0b0 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -96,6 +96,22 @@ def test_symbols_with_dashes assert_equal({ "artist" => "journey", "song" => "faithfully" }, hash) end + def test_id_encoding + rs.draw do + get "/journey/:id", to: lambda { |env| + param = ActionDispatch::Request.new(env).path_parameters + resp = ActiveSupport::JSON.encode param + [200, {}, [resp]] + } + end + + # The encoding of the URL in production is *binary*, so we add a + # .b here. + hash = ActiveSupport::JSON.decode get(URI("http://example.org/journey/%E5%A4%AA%E9%83%8E".b)) + assert_equal({ "id" => "太郎" }, hash) + assert_equal ::Encoding::UTF_8, hash["id"].encoding + end + def test_id_with_dash rs.draw do get "/journey/:id", to: lambda { |env| diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 1af0edc1d3de8dd34f2c37c0e10d220e5fb7c19d..2bb814e5f453750c6fd285c5b69c65cc033dd45e 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4416,6 +4416,10 @@ def app; APP end class TestInvalidUrls < ActionDispatch::IntegrationTest class FooController < ActionController::Base + def self.binary_params_for?(action) + action == "show" + end + def show render plain: "foo#show" end @@ -4437,19 +4441,19 @@ def show end get "/%E2%EF%BF%BD%A6" - assert_response :not_found + assert_response :bad_request get "/foo/%E2%EF%BF%BD%A6" - assert_response :not_found + assert_response :bad_request get "/foo/show/%E2%EF%BF%BD%A6" assert_response :ok get "/bar/%E2%EF%BF%BD%A6" - assert_response :redirect + assert_response :bad_request get "/foobar/%E2%EF%BF%BD%A6" - assert_response :ok + assert_response :bad_request end end end