Merge pull request #5606 from teohm/multipart_unicode_param_name

multipart POST - utf8 param name not encoded

6 files changed, 54 insertions, 32 deletions

diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb
index f945a5c1a2..246d9c121a 100644
--- a/actionpack/lib/action_dispatch/http/parameters.rb
+++ b/actionpack/lib/action_dispatch/http/parameters.rb
@@ -18,7 +18,7 @@ module ActionDispatch
             query_parameters.dup
           end
           params.merge!(path_parameters)
-          encode_params(params).with_indifferent_access
+          params.with_indifferent_access
         end
       end
       alias :params :parameters
@@ -50,40 +50,33 @@ module ActionDispatch
 
     private
 
+      # Convert nested Hash to HashWithIndifferentAccess
+      # and UTF-8 encode both keys and values in nested Hash.
+      #
       # TODO: Validate that the characters are UTF-8. If they aren't,
       # you'll get a weird error down the road, but our form handling
       # should really prevent that from happening
-      def encode_params(params)
+      def normalize_encode_params(params)
         if params.is_a?(String)
           return params.force_encoding(Encoding::UTF_8).encode!
         elsif !params.is_a?(Hash)
           return params
         end
 
-        params.each_value do |v|
-          case v
-          when Hash
-            encode_params(v)
-          when Array
-            v.map! {|el| encode_params(el) }
-          else
-            encode_params(v)
-          end
-        end
-      end
-
-      # Convert nested Hash to ActiveSupport::HashWithIndifferentAccess
-      def normalize_parameters(value)
-        case value
-        when Hash
-          h = {}
-          value.each { |k, v| h[k] = normalize_parameters(v) }
-          h.with_indifferent_access
-        when Array
-          value.map { |e| normalize_parameters(e) }
-        else
-          value
+        new_hash = {}
+        params.each do |k, v|
+          new_key = k.is_a?(String) ? k.dup.force_encoding("UTF-8").encode! : k
+          new_hash[new_key] =
+            case v
+            when Hash
+              normalize_encode_params(v)
+            when Array
+              v.map! {|el| normalize_encode_params(el) }
+            else
+              normalize_encode_params(v)
+            end
         end
+        new_hash.with_indifferent_access
       end
     end
   end
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 97a9b7be86..aff2172788 100644
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -273,7 +273,7 @@ module ActionDispatch
 
     # Override Rack's GET method to support indifferent access
     def GET
-      @env["action_dispatch.request.query_parameters"] ||= (normalize_parameters(super) || {})
+      @env["action_dispatch.request.query_parameters"] ||= (normalize_encode_params(super) || {})
     rescue TypeError => e
       raise ActionController::BadRequest.new(:query, e)
     end
@@ -281,7 +281,7 @@ module ActionDispatch
 
     # Override Rack's POST method to support indifferent access
     def POST
-      @env["action_dispatch.request.request_parameters"] ||= (normalize_parameters(super) || {})
+      @env["action_dispatch.request.request_parameters"] ||= (normalize_encode_params(super) || {})
     rescue TypeError => e
       raise ActionController::BadRequest.new(:request, e)
     end
diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb
index 67cb7fbcb5..319d0197d1 100644
--- a/actionpack/lib/action_dispatch/http/upload.rb
+++ b/actionpack/lib/action_dispatch/http/upload.rb
@@ -75,16 +75,16 @@ module ActionDispatch
     end
 
     module Upload # :nodoc:
-      # Convert nested Hash to ActiveSupport::HashWithIndifferentAccess and replace
-      # file upload hash with UploadedFile objects
-      def normalize_parameters(value)
+      # Replace file upload hash with UploadedFile objects
+      # when normalize and encode parameters.
+      def normalize_encode_params(value)
         if Hash === value && value.has_key?(:tempfile)
           UploadedFile.new(value)
         else
           super
         end
       end
-      private :normalize_parameters
+      private :normalize_encode_params
     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 399f15199c..3c30a705e9 100644
--- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb
+++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb
@@ -1,13 +1,15 @@
+# encoding: utf-8
 require 'abstract_unit'
 
 class MultipartParamsParsingTest < ActionDispatch::IntegrationTest
   class TestController < ActionController::Base
     class << self
-      attr_accessor :last_request_parameters
+      attr_accessor :last_request_parameters, :last_parameters
     end
 
     def parse
       self.class.last_request_parameters = request.request_parameters
+      self.class.last_parameters = request.parameters
       head :ok
     end
 
@@ -30,6 +32,23 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest
     assert_equal({ 'foo' => { 'baz' => 'bar'}}, parse_multipart('bracketed_param'))
   end
 
+  test "parse single utf8 parameter" do
+    assert_equal({ 'Iñtërnâtiônàlizætiøn_name' => 'Iñtërnâtiônàlizætiøn_value'}, 
+                 parse_multipart('single_utf8_param'), "request.request_parameters")
+    assert_equal(
+      'Iñtërnâtiônàlizætiøn_value',
+      TestController.last_parameters['Iñtërnâtiônàlizætiøn_name'], "request.parameters")
+  end
+
+  test "parse bracketed utf8 parameter" do
+    assert_equal({ 'Iñtërnâtiônàlizætiøn_name' => { 
+      'Iñtërnâtiônàlizætiøn_nested_name' => 'Iñtërnâtiônàlizætiøn_value'} }, 
+      parse_multipart('bracketed_utf8_param'), "request.request_parameters")
+    assert_equal(
+      {'Iñtërnâtiônàlizætiøn_nested_name' => 'Iñtërnâtiônàlizætiøn_value'},
+      TestController.last_parameters['Iñtërnâtiônàlizætiøn_name'], "request.parameters")
+  end
+
   test "parses text file" do
     params = parse_multipart('text_file')
     assert_equal %w(file foo), params.keys.sort
diff --git a/actionpack/test/fixtures/multipart/bracketed_utf8_param b/actionpack/test/fixtures/multipart/bracketed_utf8_param
new file mode 100644
index 0000000000..976ca44a45
--- /dev/null
+++ b/actionpack/test/fixtures/multipart/bracketed_utf8_param
@@ -0,0 +1,5 @@
+--AaB03x

+Content-Disposition: form-data; name="Iñtërnâtiônàlizætiøn_name[Iñtërnâtiônàlizætiøn_nested_name]"

+

+Iñtërnâtiônàlizætiøn_value

+--AaB03x--

diff --git a/actionpack/test/fixtures/multipart/single_utf8_param b/actionpack/test/fixtures/multipart/single_utf8_param
new file mode 100644
index 0000000000..b86f62d1e1
--- /dev/null
+++ b/actionpack/test/fixtures/multipart/single_utf8_param
@@ -0,0 +1,5 @@
+--AaB03x

+Content-Disposition: form-data; name="Iñtërnâtiônàlizætiøn_name"

+

+Iñtërnâtiônàlizætiøn_value

+--AaB03x--