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.

Author: kerrizor

actionpack/lib/action_controller.rb

@@ -40,6 +40,7 @@ module ActionController
     autoload :Rescue
     autoload :Streaming
     autoload :StrongParameters
+    autoload :ParameterEncoding
     autoload :Testing
     autoload :UrlFor
   end

actionpack/lib/action_controller/base.rb

@@ -217,7 +217,7 @@ def self.without_modules(*modules)
       MimeResponds,
       ImplicitRender,
       StrongParameters,
-
+      ParameterEncoding,
       Cookies,
       Flash,
       FormBuilder,

actionpack/lib/action_controller/metal.rb

@@ -139,6 +139,10 @@ def self.make_response!(request)
       end
     end
 
+    def self.encoding_for_param(action, param) # :nodoc:
+      ::Encoding::UTF_8
+    end
+
     # Delegates to the class' <tt>controller_name</tt>
     def controller_name
       self.class.controller_name

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

actionpack/lib/action_controller/test_case.rb

@@ -33,24 +33,27 @@ def self.new_session
       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
       DEFAULT_ENV
     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 @@ def process(action, *args)
         @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 @@ def setup_controller_request_and_response
           end
         end
 
-        @request          = TestRequest.create
+        @request          = TestRequest.create(@controller.class)
         @response         = build_response @response_klass
         @response.request = @request
 

actionpack/lib/action_dispatch/http/parameters.rb

@@ -37,11 +37,23 @@ def parameters
                    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 @@ def path_parameters
 
       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?
 

actionpack/lib/action_dispatch/http/request.rb

@@ -69,6 +69,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.encoding_for_param(action, param); ::Encoding::UTF_8; end
     }
 
     def controller_class

actionpack/lib/action_dispatch/testing/assertions/routing.rb

@@ -184,7 +184,7 @@ def recognized_request_for(path, extras = {}, msg)
           end
 
           # Assume given controller
-          request = ActionController::TestRequest.create
+          request = ActionController::TestRequest.create @controller.class
 
           if path =~ %r{://}
             fail_on(URI::InvalidURIError, msg) do

actionpack/test/abstract_unit.rb

@@ -122,27 +122,19 @@ class DeadEndRoutes < ActionDispatch::Routing::RouteSet
     # 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
 

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