Merge pull request #454 from stripe/brandur-error-on-improper-array-of-maps

Produce an error on bad array of maps

lib/stripe/util.rb

@@ -140,6 +140,7 @@ module Stripe
         if value.is_a?(Hash)
           result += flatten_params(value, calculated_key)
         elsif value.is_a?(Array)
+          check_array_of_maps_start_keys!(value)
           result += flatten_params_array(value, calculated_key)
         else
           result << [calculated_key, value]
@@ -196,5 +197,44 @@ module Stripe
       raise TypeError.new("api_key must be a string") unless key.is_a?(String)
       key
     end
+
+    private
+
+    # We use a pretty janky version of form encoding (Rack's) that supports
+    # more complex data structures like maps and arrays through the use of
+    # specialized syntax. To encode an array of maps like:
+    #
+    #     [{a: 1, b: 2}, {a: 3, b: 4}]
+    #
+    # We have to produce something that looks like this:
+    #
+    #     arr[][a]=1&arr[][b]=2&arr[][a]=3&arr[][b]=4
+    #
+    # The only way for the server to recognize that this is a two item array is
+    # that it notices the repetition of element "a", so it's key that these
+    # repeated elements are encoded first.
+    #
+    # This method is invoked for any arrays being encoded and checks that if
+    # the array contains all non-empty maps, that each of those maps must start
+    # with the same key so that their boundaries can be properly encoded.
+    def self.check_array_of_maps_start_keys!(arr)
+      expected_key = nil
+      arr.each do |item|
+        return if !item.is_a?(Hash)
+        return if item.count == 0
+
+        first_key = item.first[0]
+
+        if expected_key
+          if expected_key != first_key
+            raise ArgumentError,
+              "All maps nested in an array should start with the same key " +
+              "(expected starting key '#{expected_key}', got '#{first_key}')"
+          end
+        else
+          expected_key = first_key
+        end
+      end
+    end
   end
 end

test/stripe/charge_test.rb

@@ -138,7 +138,7 @@ module Stripe
         :amount => 100,
         :source => 'btcrcv_test_receiver',
         :currency => "usd",
-        :level3 => [{:red => 'firstred'}, {:one => 'fish', :red => 'another'}]
+        :level3 => [{:red => 'firstred'}, {:red => 'another', :one => 'fish'}]
       })
       assert c.paid
     end

test/stripe/util_test.rb

@@ -20,6 +20,34 @@ module Stripe
       )
     end
 
+    should "#encode_params should throw an error on an array of maps that cannot be encoded" do
+      params = {
+        :a => [
+          { :a => 1, :b => 2 },
+          { :c => 3, :a => 4 },
+        ]
+      }
+      e = assert_raises(ArgumentError) do
+        Stripe::Util.encode_parameters(params)
+      end
+      expected = "All maps nested in an array should start with the same key " +
+        "(expected starting key 'a', got 'c')"
+      assert_equal expected, e.message
+
+      # Make sure the check is recursive by taking our original params and
+      # nesting it into yet another map and array. Should throw exactly the
+      # same error because it's still the in inner array of maps that's wrong.
+      params = {
+        :x => [
+          params
+        ]
+      }
+      e = assert_raises(ArgumentError) do
+        Stripe::Util.encode_parameters(params)
+      end
+      assert_equal expected, e.message
+    end
+
     should "#url_encode should prepare strings for HTTP" do
       assert_equal "foo",      Stripe::Util.url_encode("foo")
       assert_equal "foo",      Stripe::Util.url_encode(:foo)