Merge query parameters coming from path with params argument

If specifying both query parameters in a path/URL down to Faraday (e.g.,
`/v1/invoices/upcoming?coupon=25OFF`) _and_ query parameters in a hash
(e.g., `{ customer: "cus_123" }`), it will silently overwrite the ones
in the path with the ones in the hash. This can cause problems where
some critical parameters are discarded and causes an error, as seen in
issue #646.

This patch modifies `#execute_request` so that before going out to
Faraday we check whether the incoming path has query parameters. If it
does, we decode them and add them to our `query_params` hash so that
all parameters from either place are preserved.

Fixes #646.

.rubocop_todo.yml

@@ -8,17 +8,17 @@
 
 # Offense count: 19
 Metrics/AbcSize:
-  Max: 45
+  Max: 52
 
 # Offense count: 27
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 469
+  Max: 496
 
 # Offense count: 8
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 597
+  Max: 624
 
 # Offense count: 11
 Metrics/CyclomaticComplexity:
@@ -33,7 +33,7 @@ Metrics/LineLength:
 # Offense count: 32
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
-  Max: 46
+  Max: 48
 
 # Offense count: 1
 # Configuration parameters: CountComments.

lib/stripe/stripe_client.rb

@@ -134,6 +134,23 @@ def execute_request(method, path,
                end
       end
 
+      # This works around an edge case where we end up with both query
+      # parameters in `query_params` and query parameters that are appended
+      # onto the end of the given path. In this case, Faraday will silently
+      # discard the URL's parameters which may break a request.
+      #
+      # Here we decode any parameters that were added onto the end of a path
+      # and add them to `query_params` so that all parameters end up in one
+      # place and all of them are correctly included in the final request.
+      u = URI.parse(path)
+      unless u.query.nil?
+        query_params ||= {}
+        query_params = Hash[URI.decode_www_form(u.query)].merge(query_params)
+
+        # Reset the path minus any query parameters that were specified.
+        path = u.path
+      end
+
       headers = request_headers(api_key, method)
                 .update(Util.normalize_headers(headers))
 

test/stripe/stripe_client_test.rb

@@ -681,6 +681,35 @@ class StripeClientTest < Test::Unit::TestCase
             }
           )
         end
+
+        should "merge query parameters in URL and params" do
+          client = StripeClient.new
+          client.execute_request(:get, "/v1/invoices/upcoming?coupon=25OFF", params: {
+            customer: "cus_123",
+          })
+          assert_requested(
+            :get,
+            "#{Stripe.api_base}/v1/invoices/upcoming?",
+            query: {
+              coupon: "25OFF",
+              customer: "cus_123",
+            }
+          )
+        end
+
+        should "prefer query parameters in params when specified in URL as well" do
+          client = StripeClient.new
+          client.execute_request(:get, "/v1/invoices/upcoming?customer=cus_query", params: {
+            customer: "cus_param",
+          })
+          assert_requested(
+            :get,
+            "#{Stripe.api_base}/v1/invoices/upcoming?",
+            query: {
+              customer: "cus_param",
+            }
+          )
+        end
       end
     end