From a5a9eb94db8357bbd8e26d4f28fb4fed873c39a5 Mon Sep 17 00:00:00 2001
From: Brandur <brandur@mutelight.org>
Date: Thu, 25 Aug 2016 11:26:46 -0700
Subject: [PATCH] Produce an error on bad array of maps

This produces an error when we detect an "array of maps" that cannot be
encoded with `application/x-www-form-urlencoded`; that is to say, one
that does not have each hash starting with a consistent key that will
allow a Rack-compliant server to recognize boundaries.

So for example, this is fine:

```
items: [
    { :type => 'sku', :parent => 'sku_94ZYSC0wppRTbk' },
    { :type => 'discount', :amount => -10000, :currency => 'cad', :description => 'potato' }
],
```

But this is _not_ okay:

```
items: [
    { :type => 'sku', :parent => 'sku_94ZYSC0wppRTbk' },
    { :amount => -10000, :currency => 'cad', :description => 'potato', :type => 'discount' }
],
```

(`type` should be moved to the beginning of the array.)

The purpose of this change is to give users better feedback when they
run into an encoding problem like this one. Currently, they just get
something confusing from the server, and someone on support usually
needs to examine a request log to figure out what happened.

CI will fail until the changes in #453 are brought in.
---
 lib/stripe/util.rb         | 40 ++++++++++++++++++++++++++++++++++++++
 test/stripe/charge_test.rb |  2 +-
 test/stripe/util_test.rb   | 15 ++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/lib/stripe/util.rb b/lib/stripe/util.rb
index 89e5ab16..53965756 100644
--- a/lib/stripe/util.rb
+++ b/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
diff --git a/test/stripe/charge_test.rb b/test/stripe/charge_test.rb
index 948af319..34146096 100644
--- a/test/stripe/charge_test.rb
+++ b/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
diff --git a/test/stripe/util_test.rb b/test/stripe/util_test.rb
index 013431e9..27b234d2 100644
--- a/test/stripe/util_test.rb
+++ b/test/stripe/util_test.rb
@@ -20,6 +20,21 @@ module Stripe
       )
     end
 
+    should "#encode_params should throw an error an on 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
+    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)