Validate all instance variable keys returned from the API (#1571)

* Normalize all keys

* instead reject invalid keys

* rubocop

* more easy to read constants

.rubocop.yml

@@ -50,6 +50,7 @@ Metrics/CyclomaticComplexity:
 Metrics/PerceivedComplexity:
   Exclude:
     - "lib/stripe/api_requestor.rb"
+    - "lib/stripe/stripe_object.rb"
     - "lib/stripe/util.rb"
 
 Metrics/MethodLength:

README.md

@@ -156,6 +156,9 @@ print(customer.last_response.http_status) # to retrieve status code
 print(customer.last_response.http_headers) # to retrieve headers
 ```
 
+If you are accessing a response field with custom hashes provided by you, such as `Customer.metadata`,
+please access your fields with the `[]` accessor.
+
 ### Configuring a proxy
 
 A proxy can be configured with `Stripe.proxy`:

lib/stripe/stripe_object.rb

@@ -336,7 +336,7 @@ module Stripe
       end
     end
 
-    protected def add_accessors(keys, values)
+    protected def add_accessors(keys, values) # rubocop:todo Metrics/PerceivedComplexity
       # not available in the #instance_eval below
       protected_fields = self.class.protected_fields
 
@@ -372,7 +372,16 @@ module Stripe
       end
 
       keys.each do |k|
-        instance_variable_set(:"@#{k}", values[k])
+        if Util.valid_variable_name?(k)
+          instance_variable_set(:"@#{k}", values[k])
+        else
+          Util.log_info(<<~LOG
+            The variable name '#{k}' is not a valid Ruby variable name.
+            Use ["#{k}"] to access this field, skipping instance variable instantiation...
+          LOG
+                       )
+
+        end
       end
     end
 

lib/stripe/util.rb

@@ -4,6 +4,9 @@ require "cgi"
 
 module Stripe
   module Util
+    LEGAL_FIRST_CHARACTER = /[a-zA-Z_]/.freeze
+    LEGAL_VARIABLE_CHARACTER = /[a-zA-Z0-9_]/.freeze
+
     def self.objects_to_ids(obj)
       case obj
       when APIResource
@@ -309,6 +312,14 @@ module Stripe
       end
     end
 
+    # Return false for strings that are invalid variable names
+    # Does NOT expect there to be a preceding '@' for instance variables
+    def self.valid_variable_name?(key)
+      return false if key.empty? || key[0] !~ LEGAL_FIRST_CHARACTER
+
+      key[1..-1].chars.all? { |char| char =~ LEGAL_VARIABLE_CHARACTER }
+    end
+
     def self.check_string_argument!(key)
       raise TypeError, "argument must be a string" unless key.is_a?(String)
 

test/stripe/stripe_object_test.rb

@@ -238,6 +238,13 @@ module Stripe
       assert_equal true, obj.send(:metaclass).method_defined?(:foo)
     end
 
+    should "nonstandard keys in response hashes work" do
+      stub_request(:post, "#{Stripe.api_base}/v1/customers")
+        .to_return(body: JSON.generate(object: "customer", email: "test@example.com", metadata: { "this-is?a.test" => "foo" }))
+      c = Stripe::Customer.create({ email: "test@example.com", metadata: { "this-is?a.test" => "foo" } })
+      assert_equal "foo", c.metadata["this-is?a.test"]
+    end
+
     should "pass opts down to children when initializing" do
       opts = { custom: "opts" }
 

test/stripe/util_test.rb

@@ -325,6 +325,21 @@ module Stripe
       end
     end
 
+    context ".valid_variable_name?" do
+      should "reject invalid variable name" do
+        assert Util.valid_variable_name?("FOOfoo")
+        assert Util.valid_variable_name?("foo123")
+        assert Util.valid_variable_name?("foo_123_")
+        assert Util.valid_variable_name?("_123_foo")
+        refute Util.valid_variable_name?("123foo")
+        refute Util.valid_variable_name?("foo-bar")
+        refute Util.valid_variable_name?("foo?bar")
+        refute Util.valid_variable_name?("foo!bar")
+        refute Util.valid_variable_name?("foo-?!_bar")
+        refute Util.valid_variable_name?("1FOO-.><bar?")
+      end
+    end
+
     #
     # private
     #