Support duplicate form names in multipart forms

lib/http/form_data.rb

@@ -38,11 +38,11 @@ class << self
       # FormData factory. Automatically selects best type depending on given
       # `data` Hash.
       #
-      # @param [#to_h, Hash] data
+      # @param [Enumerable, Hash, #to_h] data
       # @return [Multipart] if any of values is a {FormData::File}
       # @return [Urlencoded] otherwise
       def create(data, encoder: nil)
-        data = ensure_hash data
+        data = ensure_data data
 
         if multipart?(data)
           Multipart.new(data)
@@ -64,6 +64,18 @@ def ensure_hash(obj)
         end
       end
 
+      # Coerce `obj` to an Enumerable of key-value pairs.
+      #
+      # @raise [Error] `obj` can't be coerced.
+      # @return [Enumerable]
+      def ensure_data(obj)
+        if    obj.nil?                  then []
+        elsif obj.is_a?(Enumerable)     then obj
+        elsif obj.respond_to?(:to_h)    then obj.to_h
+        else raise Error, "#{obj.inspect} is neither Enumerable nor responds to :to_h"
+        end
+      end
+
       private
 
       # Tells whenever data contains multipart data or not.

lib/http/form_data/multipart.rb

@@ -14,12 +14,10 @@ class Multipart
 
       attr_reader :boundary
 
-      # @param [#to_h, Hash] data form data key-value Hash
+      # @param [Enumerable, Hash, #to_h] data form data key-value pairs
       def initialize(data, boundary: self.class.generate_boundary)
-        parts = Param.coerce FormData.ensure_hash data
-
         @boundary = boundary.to_s.freeze
-        @io = CompositeIO.new [*parts.flat_map { |part| [glue, part] }, tail]
+        @io = CompositeIO.new(parts(data).flat_map { |part| [glue, part] } << tail)
       end
 
       # Generates a string suitable for using as a boundary in multipart form
@@ -54,6 +52,18 @@ def glue
       def tail
         @tail ||= "--#{@boundary}--#{CRLF}"
       end
+
+      def parts(data)
+        params = []
+
+        FormData.ensure_data(data).each do |name, values|
+          Array(values).each do |value|
+            params << Param.new(name, value)
+          end
+        end
+
+        params
+      end
     end
   end
 end

lib/http/form_data/multipart/param.rb

@@ -41,24 +41,6 @@ def initialize(name, value)
           @io = CompositeIO.new [header, @part, footer]
         end
 
-        # Flattens given `data` Hash into an array of `Param`'s.
-        # Nested array are unwinded.
-        # Behavior is similar to `URL.encode_www_form`.
-        #
-        # @param [Hash] data
-        # @return [Array<FormData::MultiPart::Param>]
-        def self.coerce(data)
-          params = []
-
-          data.each do |name, values|
-            Array(values).each do |value|
-              params << new(name, value)
-            end
-          end
-
-          params
-        end
-
         private
 
         def header

lib/http/form_data/urlencoded.rb

@@ -63,10 +63,10 @@ def encoder
         end
       end
 
-      # @param [#to_h, Hash] data form data key-value Hash
+      # @param [Enumerable, Hash, #to_h] data form data key-value pairs
       def initialize(data, encoder: nil)
         encoder ||= self.class.encoder
-        @io = StringIO.new(encoder.call(FormData.ensure_hash(data)))
+        @io = StringIO.new(encoder.call(FormData.ensure_data(data)))
       end
 
       # Returns MIME type to be used for HTTP request `Content-Type` header.

spec/lib/http/form_data/multipart_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe HTTP::FormData::Multipart do
-  subject(:form_data) { HTTP::FormData::Multipart.new params }
+  subject(:form_data) { described_class.new params }
 
   let(:file)          { HTTP::FormData::File.new fixture "the-http-gem.info" }
   let(:params)        { { :foo => :bar, :baz => file } }
@@ -17,7 +17,6 @@ def disposition(params)
 
     it "properly generates multipart data" do
       boundary_value = form_data.boundary
-
       expect(form_data.to_s).to eq([
         "--#{boundary_value}#{crlf}",
         "#{disposition 'name' => 'foo'}#{crlf}",
@@ -87,6 +86,61 @@ def disposition(params)
         ].join)
       end
     end
+
+    it "supports any Enumerable of pairs" do
+      enum = Enumerator.new { |y| y << [:foo, :bar] << [:foo, :baz] }
+      form_data = described_class.new(enum)
+
+      boundary_value = form_data.boundary
+      expect(form_data.to_s).to eq([
+        "--#{boundary_value}#{crlf}",
+        "#{disposition 'name' => 'foo'}#{crlf}",
+        "#{crlf}bar#{crlf}",
+        "--#{boundary_value}#{crlf}",
+        "#{disposition 'name' => 'foo'}#{crlf}",
+        "#{crlf}baz#{crlf}",
+        "--#{boundary_value}--#{crlf}"
+      ].join)
+    end
+
+    # https://github.com/httprb/http/issues/663
+    context "when params is an Array of pairs" do
+      let(:params) do
+        [
+          ["metadata", %(filename="first.txt")],
+          ["file", HTTP::FormData::File.new(StringIO.new("uno"), :content_type => "plain/text", :filename => "abc")],
+          ["metadata", %(filename="second.txt")],
+          ["file", HTTP::FormData::File.new(StringIO.new("dos"), :content_type => "plain/text", :filename => "xyz")],
+          ["metadata", %w[question=why question=not]]
+        ]
+      end
+
+      it "allows duplicate param names and preserves given order" do
+        expect(form_data.to_s).to eq([
+          %(--#{form_data.boundary}\r\n),
+          %(Content-Disposition: form-data; name="metadata"\r\n),
+          %(\r\nfilename="first.txt"\r\n),
+          %(--#{form_data.boundary}\r\n),
+          %(Content-Disposition: form-data; name="file"; filename="abc"\r\n),
+          %(Content-Type: plain/text\r\n),
+          %(\r\nuno\r\n),
+          %(--#{form_data.boundary}\r\n),
+          %(Content-Disposition: form-data; name="metadata"\r\n),
+          %(\r\nfilename="second.txt"\r\n),
+          %(--#{form_data.boundary}\r\n),
+          %(Content-Disposition: form-data; name="file"; filename="xyz"\r\n),
+          %(Content-Type: plain/text\r\n),
+          %(\r\ndos\r\n),
+          %(--#{form_data.boundary}\r\n),
+          %(Content-Disposition: form-data; name="metadata"\r\n),
+          %(\r\nquestion=why\r\n),
+          %(--#{form_data.boundary}\r\n),
+          %(Content-Disposition: form-data; name="metadata"\r\n),
+          %(\r\nquestion=not\r\n),
+          %(--#{form_data.boundary}--\r\n)
+        ].join)
+      end
+    end
   end
 
   describe "#size" do

spec/lib/http/form_data/urlencoded_spec.rb

@@ -4,6 +4,12 @@
   let(:data) { { "foo[bar]" => "test" } }
   subject(:form_data) { HTTP::FormData::Urlencoded.new data }
 
+  it "supports any Enumerables of pairs" do
+    form_data = described_class.new([%w[foo bar], ["foo", %w[baz moo]]])
+
+    expect(form_data.to_s).to eq("foo=bar&foo=baz&foo=moo")
+  end
+
   describe "#content_type" do
     subject { form_data.content_type }
     it { is_expected.to eq "application/x-www-form-urlencoded" }

spec/lib/http/form_data_spec.rb

@@ -22,6 +22,42 @@
     end
   end
 
+  describe ".ensure_data" do
+    subject(:ensure_data) { HTTP::FormData.ensure_data data }
+
+    context "when Hash given" do
+      let(:data) { { :foo => :bar } }
+      it { is_expected.to eq :foo => :bar }
+    end
+
+    context "when Array given" do
+      let(:data) { [[:foo, :bar], [:foo, :baz]] }
+      it { is_expected.to eq [[:foo, :bar], [:foo, :baz]] }
+    end
+
+    context "when Enumerator given" do
+      let(:data) { Enumerator.new { |y| y << [:foo, :bar] } }
+      it { is_expected.to be_a Enumerator }
+    end
+
+    context "when #to_h given" do
+      let(:data) { double(:to_h => { :foo => :bar }) }
+      it { is_expected.to eq :foo => :bar }
+    end
+
+    context "when nil given" do
+      let(:data) { nil }
+      it { is_expected.to eq([]) }
+    end
+
+    context "when neither Enumerable nor #to_h given" do
+      let(:data) { double }
+      it "fails with HTTP::FormData::Error" do
+        expect { ensure_data }.to raise_error HTTP::FormData::Error
+      end
+    end
+  end
+
   describe ".ensure_hash" do
     subject(:ensure_hash) { HTTP::FormData.ensure_hash data }