Merge pull request rack#69 from alphagov/master

Provide full support for arrays of hashes in multipart forms
pixeltrix · Sep 18, 2013 · 8cdb86e · 8cdb86e
2 parents 280ff54 + 1deec79
commit 8cdb86e
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 12 deletions.
diff --git a/lib/rack/test/utils.rb b/lib/rack/test/utils.rb
@@ -57,9 +57,12 @@ def build_multipart(params, first = true)
             value.map do |v|
 
               if (v.is_a?(Hash))
+                nested_params = {}
                 build_multipart(v, false).each { |subkey, subvalue|
-                  flattened_params["#{k}[]#{subkey}"] = subvalue
+                  nested_params[subkey] = subvalue
                 }
+                flattened_params["#{k}[]"] ||= []
+                flattened_params["#{k}[]"] << nested_params
               else
                 flattened_params["#{k}[]"] = value
               end
@@ -85,21 +88,32 @@ def build_multipart(params, first = true)
 
     private
       def build_parts(parameters)
+        get_parts(parameters).join + "--#{MULTIPART_BOUNDARY}--\r"
+      end
+
+      def get_parts(parameters)
         parameters.map { |name, value|
-          if value.respond_to?(:original_filename)
-            build_file_part(name, value)
+          if name =~ /\[\]\Z/ && value.is_a?(Array) && value.all? {|v| v.is_a?(Hash)}
+            value.map { |hash|
+              new_value = {}
+              hash.each { |k, v| new_value[name+k] = v }
+              get_parts(new_value).join
+            }.join
+          else
+            if value.respond_to?(:original_filename)
+              build_file_part(name, value)
 
-          elsif value.is_a?(Array) and value.all? { |v| v.respond_to?(:original_filename) }
-            value.map do |v|
-              build_file_part(name, v)
-            end.join
+            elsif value.is_a?(Array) and value.all? { |v| v.respond_to?(:original_filename) }
+              value.map do |v|
+                build_file_part(name, v)
+              end.join
 
-          else
-            primitive_part = build_primitive_part(name, value)
-            Rack::Test.encoding_aware_strings? ? primitive_part.force_encoding('BINARY') : primitive_part
+            else
+              primitive_part = build_primitive_part(name, value)
+              Rack::Test.encoding_aware_strings? ? primitive_part.force_encoding('BINARY') : primitive_part
+            end
           end
-
-        }.join + "--#{MULTIPART_BOUNDARY}--\r"
+        }
       end
 
       def build_primitive_part(parameter_name, value)

diff --git a/spec/rack/test/utils_spec.rb b/spec/rack/test/utils_spec.rb
@@ -104,6 +104,77 @@
       check params["foo"].should == ["1", "2"]
     end
 
+    it "builds nested multipart bodies with an array of hashes" do
+      files = Rack::Test::UploadedFile.new(multipart_file("foo.txt"))
+      data  = build_multipart("files" => files, "foo" => [{"id" => "1", "name" => 'Dave'}, {"id" => "2", "name" => 'Steve'}])
+
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=#{Rack::Test::MULTIPART_BOUNDARY}",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+      env = Rack::MockRequest.env_for("/", options)
+      params = Rack::Utils::Multipart.parse_multipart(env)
+      check params["files"][:filename].should == "foo.txt"
+      params["files"][:tempfile].read.should == "bar\n"
+      check params["foo"].should == [{"id" => "1", "name" => "Dave"}, {"id" => "2", "name" => "Steve"}]
+    end
+
+    it "builds nested multipart bodies with arbitrarily nested array of hashes" do
+      files = Rack::Test::UploadedFile.new(multipart_file("foo.txt"))
+      data  = build_multipart("files" => files, "foo" => {"bar" => [{"id" => "1", "name" => 'Dave'},
+                                                                    {"id" => "2", "name" => 'Steve', "qux" => [{"id" => '3', "name" => 'mike'},
+                                                                                                               {"id" => '4', "name" => 'Joan'}]}]})
+
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=#{Rack::Test::MULTIPART_BOUNDARY}",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+      env = Rack::MockRequest.env_for("/", options)
+      params = Rack::Utils::Multipart.parse_multipart(env)
+      check params["files"][:filename].should == "foo.txt"
+      params["files"][:tempfile].read.should == "bar\n"
+      check params["foo"].should == {"bar" => [{"id" => "1", "name" => "Dave"},
+                                               {"id" => "2", "name" => "Steve", "qux" => [{"id" => '3', "name" => 'mike'},
+                                                                                          {"id" => '4', "name" => 'Joan'}]}]}
+    end
+
+    it 'does not break with params that look nested, but are not' do
+      files = Rack::Test::UploadedFile.new(multipart_file("foo.txt"))
+      data  = build_multipart("foo[]" => "1", "bar[]" => {"qux" => "2"}, "files[]" => files)
+
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=#{Rack::Test::MULTIPART_BOUNDARY}",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+      env = Rack::MockRequest.env_for("/", options)
+      params = Rack::Utils::Multipart.parse_multipart(env)
+      check params["files"][0][:filename].should == "foo.txt"
+      params["files"][0][:tempfile].read.should == "bar\n"
+      check params["foo"][0].should == "1"
+      check params["bar"][0].should == {"qux" => "2"}
+    end
+
+    it 'allows for nested files' do
+      files = Rack::Test::UploadedFile.new(multipart_file("foo.txt"))
+      data  = build_multipart("foo" => [{"id" => "1", "data" => files},
+                                        {"id" => "2", "data" => ["3", "4"]}])
+
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=#{Rack::Test::MULTIPART_BOUNDARY}",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+      env = Rack::MockRequest.env_for("/", options)
+      params = Rack::Utils::Multipart.parse_multipart(env)
+      check params["foo"][0]["id"].should == "1"
+      check params["foo"][0]["data"][:filename].should == "foo.txt"
+      params["foo"][0]["data"][:tempfile].read.should == "bar\n"
+      check params["foo"][1].should == {"id" => "2", "data" => ["3", "4"]}
+    end
+
     it "returns nil if no UploadedFiles were used" do
       data = build_multipart("people" => [{"submit-name" => "Larry", "files" => "contents"}])
       data.should be_nil