From 0cbc6114dc052faf66eab6a844964b93d1e7a4f6 Mon Sep 17 00:00:00 2001 From: HParker Date: Tue, 20 Oct 2020 09:13:59 -0700 Subject: [PATCH] Add ability to set per param encoding previously you could skip encoding which would encode all parameters on an action as ASCII_8BIT After this change you can specify the `param_encoding` for any one parameter on an action Co-authored-by: John Hawthorn --- actionpack/lib/action_controller/metal.rb | 2 +- .../metal/parameter_encoding.rb | 36 ++++++++++++++++--- .../lib/action_dispatch/http/request.rb | 2 +- .../lib/action_dispatch/request/utils.rb | 20 ++++++----- .../request/multipart_params_parsing_test.rb | 6 ++-- actionpack/test/dispatch/routing_test.rb | 6 ++-- 6 files changed, 49 insertions(+), 23 deletions(-) diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index e1812a5c89..ada5e39ead 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -135,7 +135,7 @@ def self.make_response!(request) end end - def self.binary_params_for?(action) # :nodoc: + def self.custom_encoding_for(action, param) # :nodoc: false end diff --git a/actionpack/lib/action_controller/metal/parameter_encoding.rb b/actionpack/lib/action_controller/metal/parameter_encoding.rb index 7a45732d31..4a4e6a204d 100644 --- a/actionpack/lib/action_controller/metal/parameter_encoding.rb +++ b/actionpack/lib/action_controller/metal/parameter_encoding.rb @@ -12,11 +12,11 @@ def inherited(klass) # :nodoc: end def setup_param_encode # :nodoc: - @_parameter_encodings = {} + @_parameter_encodings = Hash.new { |h, k| h[k] = {} } end - def binary_params_for?(action) # :nodoc: - @_parameter_encodings[action.to_s] + def custom_encoding_for(action, param) # :nodoc: + @_parameter_encodings[action.to_s][param.to_s] end # Specify that a given action's parameters should all be encoded as @@ -44,7 +44,35 @@ def binary_params_for?(action) # :nodoc: # encoded as ASCII-8BIT. This is useful in the case where an application # must handle data but encoding of the data is unknown, like file system data. def skip_parameter_encoding(action) - @_parameter_encodings[action.to_s] = true + @_parameter_encodings[action.to_s] = Hash.new { Encoding::ASCII_8BIT } + end + + # Specify the encoding for a a parameter on an action + # ASCII-8BIT (it "skips" the encoding default of UTF-8). + # + # For example, a controller would use it like this: + # + # class RepositoryController < ActionController::Base + # param_encoding :show, :file_path, Encoding::ASCII_8BIT + # + # def show + # @repo = Repository.find_by_filesystem_path params[:file_path] + # + # # `repo_name` is guaranteed to be UTF-8, but was ASCII-8BIT, so + # # tag it as such + # @repo_name = params[:repo_name].force_encoding 'UTF-8' + # end + # + # def index + # @repositories = Repository.all + # end + # end + # + # The file_path parameter on the show action would be encoded as ASCII-8BIT. + # This is useful in the case where an application must handle data + # but encoding of the data is unknown, like file system data. + def param_encoding(action, param, encoding) + @_parameter_encodings[action.to_s][param.to_s] = encoding end end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 818508e6c6..36b55c950c 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -76,7 +76,7 @@ def commit_cookie_jar! # :nodoc: PASS_NOT_FOUND = Class.new { # :nodoc: def self.action(_); self; end def self.call(_); [404, { "X-Cascade" => "pass" }, []]; end - def self.binary_params_for?(action); false; end + def self.custom_encoding_for(action, param); nil; end } def controller_class diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index e2e5023f38..da2b4249ef 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -42,7 +42,7 @@ def self.check_param_encoding(params) end def self.set_binary_encoding(request, params, controller, action) - BinaryParamEncoder.encode(request, params, controller, action) + CustomParamEncoder.encode(request, params, controller, action) end class ParamEncoder # :nodoc: @@ -78,23 +78,25 @@ def self.handle_array(params) end end - class BinaryParamEncoder # :nodoc: + class CustomParamEncoder # :nodoc: def self.encode(request, params, controller, action) return params unless controller && controller.valid_encoding? - if binary_params_for?(request, controller, action) - ActionDispatch::Request::Utils.each_param_value(params.except(:controller, :action)) do |param| - param.force_encoding ::Encoding::ASCII_8BIT + params.except(:controller, :action).each do |key, value| + # next if key == :controller || key == :action + ActionDispatch::Request::Utils.each_param_value(value) do |param| + if desired_encoding = custom_encoding_for(request, controller, action, key) + param.force_encoding(desired_encoding) + end end end - params end - def self.binary_params_for?(request, controller, action) - request.controller_class_for(controller).binary_params_for?(action) + def self.custom_encoding_for(request, controller, action, param) + request.controller_class_for(controller).custom_encoding_for(action, param) rescue MissingController - false + nil end end end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 01a06bf2db..7c0d2bd4b8 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -4,12 +4,10 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base + skip_parameter_encoding("parse_binary") + class << self attr_accessor :last_request_parameters, :last_parameters - - def binary_params_for?(action) - action == "parse_binary" - end end def parse_binary diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 65e42f947d..96ea071531 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4559,9 +4559,7 @@ def app; APP end class TestInvalidUrls < ActionDispatch::IntegrationTest class FooController < ActionController::Base - def self.binary_params_for?(action) - action == "show" - end + param_encoding :show, :id, Encoding::ASCII_8BIT def show render plain: "foo#show" @@ -4595,7 +4593,7 @@ def show end end - test "params encoded with binary_params_for? are treated as ASCII 8bit" do + test "params param_encoding uses ASCII 8bit" do with_routing do |set| set.draw do get "/foo/show(/:id)", to: "test_invalid_urls/foo#show"