From 496d744fa31665de810b404de968ba86ed87c319 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Tue, 9 Aug 2016 10:35:59 -0700 Subject: Allow specifying encoding of parameters by action At GitHub we need to handle parameter encodings that are not UTF-8. This patch allows us to specify encodings per parameter per action. --- actionpack/lib/action_controller.rb | 1 + actionpack/lib/action_controller/base.rb | 2 +- actionpack/lib/action_controller/metal.rb | 4 ++ .../action_controller/metal/parameter_encoding.rb | 29 +++++++++ actionpack/lib/action_controller/test_case.rb | 13 ++-- actionpack/lib/action_dispatch/http/parameters.rb | 16 +++++ actionpack/lib/action_dispatch/http/request.rb | 1 + .../action_dispatch/testing/assertions/routing.rb | 2 +- actionpack/test/abstract_unit.rb | 18 ++---- .../test/controller/parameter_encoding_test.rb | 73 ++++++++++++++++++++++ 10 files changed, 139 insertions(+), 20 deletions(-) create mode 100644 actionpack/lib/action_controller/metal/parameter_encoding.rb create mode 100644 actionpack/test/controller/parameter_encoding_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index b9ef11a7e0..fc86a907b3 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -40,6 +40,7 @@ module ActionController autoload :Rescue autoload :Streaming autoload :StrongParameters + autoload :ParameterEncoding autoload :Testing autoload :UrlFor end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 2c7a223971..68a526eccb 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -217,7 +217,7 @@ module ActionController MimeResponds, ImplicitRender, StrongParameters, - + ParameterEncoding, Cookies, Flash, FormBuilder, diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 0364500944..075e4504c2 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -139,6 +139,10 @@ module ActionController end end + def self.encoding_for_param(action, param) # :nodoc: + ::Encoding::UTF_8 + end + # Delegates to the class' controller_name def controller_name self.class.controller_name diff --git a/actionpack/lib/action_controller/metal/parameter_encoding.rb b/actionpack/lib/action_controller/metal/parameter_encoding.rb new file mode 100644 index 0000000000..f5d3dabb45 --- /dev/null +++ b/actionpack/lib/action_controller/metal/parameter_encoding.rb @@ -0,0 +1,29 @@ +module ActionController + module ParameterEncoding + extend ActiveSupport::Concern + + module ClassMethods + def inherited(klass) + super + klass.setup_param_encode + end + + def setup_param_encode + @_parameter_encodings = {} + end + + def encoding_for_param(action, param) + if @_parameter_encodings[action.to_s] && @_parameter_encodings[action.to_s][param.to_s] + @_parameter_encodings[action.to_s][param.to_s] + else + ::Encoding::UTF_8 + end + end + + def parameter_encoding(action, param_name, encoding) + @_parameter_encodings[action.to_s] ||= {} + @_parameter_encodings[action.to_s][param_name.to_s] = encoding + end + end + end +end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index ccedecd6f4..8cffa11500 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -33,12 +33,14 @@ module ActionController TestSession.new end + attr_reader :controller_class + # Create a new test request with default `env` values - def self.create + def self.create(controller_class) env = {} env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application env["rack.request.cookie_hash"] = {}.with_indifferent_access - new(default_env.merge(env), new_session) + new(default_env.merge(env), new_session, controller_class) end def self.default_env @@ -46,11 +48,12 @@ module ActionController end private_class_method :default_env - def initialize(env, session) + def initialize(env, session, controller_class) super(env) self.session = session self.session_options = TestSession::DEFAULT_OPTIONS + @controller_class = controller_class @custom_param_parsers = { xml: lambda { |raw_post| Hash.from_xml(raw_post)["hash"] } } @@ -497,7 +500,7 @@ module ActionController @request.set_header "HTTP_COOKIE", cookies.to_header @request.delete_header "action_dispatch.cookies" - @request = TestRequest.new scrub_env!(@request.env), @request.session + @request = TestRequest.new scrub_env!(@request.env), @request.session, @controller.class @response = build_response @response_klass @response.request = @request @controller.recycle! @@ -591,7 +594,7 @@ module ActionController end end - @request = TestRequest.create + @request = TestRequest.create(@controller.class) @response = build_response @response_klass @response.request = @request diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index f25e50f9f3..a9d1a501ab 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -37,11 +37,23 @@ module ActionDispatch query_parameters.dup end params.merge!(path_parameters) + params = set_custom_encoding(params) set_header("action_dispatch.request.parameters", params) params end alias :params :parameters + def set_custom_encoding(params) + action = params[:action] + params.each do |k, v| + if v.is_a?(String) && v.encoding != encoding_template(action, k) + params[k] = v.force_encoding(encoding_template(action, k)) + end + end + + params + end + def path_parameters=(parameters) #:nodoc: delete_header("action_dispatch.request.parameters") @@ -64,6 +76,10 @@ module ActionDispatch private + def encoding_template(action, param) + controller_class.encoding_for_param(action, param) + end + def parse_formatted_parameters(parsers) return yield if content_length.zero? || content_mime_type.nil? diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 46409a325e..e7cc6d5f31 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -69,6 +69,7 @@ module ActionDispatch PASS_NOT_FOUND = Class.new { # :nodoc: def self.action(_); self; end def self.call(_); [404, {"X-Cascade" => "pass"}, []]; end + def self.encoding_for_param(action, param); ::Encoding::UTF_8; end } def controller_class diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index cba67b2839..2ea4a6c130 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -184,7 +184,7 @@ module ActionDispatch end # Assume given controller - request = ActionController::TestRequest.create + request = ActionController::TestRequest.create @controller.class if path =~ %r{://} fail_on(URI::InvalidURIError, msg) do diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 0c77de3c16..7bff21b5a2 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -122,27 +122,19 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase # Stub Rails dispatcher so it does not get controller references and # simply return the controller#action as Rack::Body. class NullController < ::ActionController::Metal - def initialize(controller_name) - @controller = controller_name - end - - def make_response!(request) - self.class.make_response! request - end - - def dispatch(action, req, res) - [200, {"Content-Type" => "text/html"}, ["#{@controller}##{action}"]] + def self.dispatch(action, req, res) + [200, {"Content-Type" => "text/html"}, ["#{req.params[:controller]}##{action}"]] end end - class NullControllerRequest < DelegateClass(ActionDispatch::Request) + class NullControllerRequest < ActionDispatch::Request def controller_class - NullController.new params[:controller] + NullController end end def make_request(env) - NullControllerRequest.new super + NullControllerRequest.new env end end diff --git a/actionpack/test/controller/parameter_encoding_test.rb b/actionpack/test/controller/parameter_encoding_test.rb new file mode 100644 index 0000000000..69a72c000b --- /dev/null +++ b/actionpack/test/controller/parameter_encoding_test.rb @@ -0,0 +1,73 @@ +require "abstract_unit" + +class ParameterEncodingController < ActionController::Base + parameter_encoding :test_bar, :bar, Encoding::ASCII_8BIT + parameter_encoding :test_baz, :baz, Encoding::ISO_8859_1 + parameter_encoding :test_baz_to_ascii, :baz, Encoding::ASCII_8BIT + + def test_foo + render body: params[:foo].encoding + end + + def test_bar + render body: params[:bar].encoding + end + + def test_baz + render body: params[:baz].encoding + end + + def test_no_change_to_baz + render body: params[:baz].encoding + end + + def test_baz_to_ascii + render body: params[:baz].encoding + end +end + +class ParameterEncodingTest < ActionController::TestCase + tests ParameterEncodingController + + test "properly transcodes UTF8 parameters into declared encodings" do + post :test_foo, params: {"foo" => "foo", "bar" => "bar", "baz" => "baz"} + + assert_response :success + assert_equal "UTF-8", @response.body + end + + test "properly transcodes ASCII_8BIT parameters into declared encodings" do + post :test_bar, params: {"foo" => "foo", "bar" => "bar", "baz" => "baz"} + + assert_response :success + assert_equal "ASCII-8BIT", @response.body + end + + test "properly transcodes ISO_8859_1 parameters into declared encodings" do + post :test_baz, params: {"foo" => "foo", "bar" => "bar", "baz" => "baz"} + + assert_response :success + assert_equal "ISO-8859-1", @response.body + end + + test "does not transcode parameters when not specified" do + post :test_no_change_to_baz, params: {"foo" => "foo", "bar" => "bar", "baz" => "baz"} + + assert_response :success + assert_equal "UTF-8", @response.body + end + + test "respects different encoding declarations for a param per action" do + post :test_baz_to_ascii, params: {"foo" => "foo", "bar" => "bar", "baz" => "baz"} + + assert_response :success + assert_equal "ASCII-8BIT", @response.body + end + + test "does not raise an error when passed a param declared as ASCII-8BIT that contains invalid bytes" do + get :test_bar, params: { "bar" => URI.parser.escape("bar\xE2baz".b) } + + assert_response :success + assert_equal "ASCII-8BIT", @response.body + end +end -- cgit v1.2.3