Permalink
Browse files

Path parameters should default to UTF8

This commit changes the behavior such the path_params now default to
UTF8 just like regular parameters. This also changes the behavior such
that if a path parameter contains invalid UTF8 it returns a 400 bad
request. Previously the behavior was to encode the path params as binary
but that's not the same as query params.

So this commit makes path params behave the same as query params.

It's important to test with a path that's encoded as binary because
that's how paths are encoded from the socket. The test that was altered
was changed to make the behavior for bad encoding the same as query
params. We want to treat path params the same as query params. The params
in the test are invalid UTF8 so they should return a bad request.

Fixes #29669

*Eileen M. Uchitelle, Aaron Patterson, & Tsukuru Tanimichi*
  • Loading branch information...
eileencodes committed Aug 1, 2017
1 parent f4142c8 commit 9668cc3bb03740b13477df0832332eec71563bc5
@@ -55,7 +55,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
@@ -64,6 +64,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)
@@ -83,18 +84,19 @@ 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
end
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
@@ -74,10 +74,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
@@ -41,6 +41,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)
@@ -65,6 +69,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
@@ -117,7 +122,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
}
@@ -94,6 +94,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|
@@ -4414,6 +4414,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

0 comments on commit 9668cc3

Please sign in to comment.