Merge pull request #12785 from chancancode/do_not_leak_internal_state

Do not expose internal state in the public encoder API (i.e. as_json)

activesupport/CHANGELOG.md

+*   Removed circular reference protection in JSON encoder, deprecated
+    ActiveSupport::JSON::Encoding::CircularReferenceError.
+
+    *Godfrey Chan*, *Sergio Campamá*
+
 *   Add `capitalize` option to Inflector.humanize, so strings can be humanized without being capitalized:
 
         'employee_salary'.humanize                    # => "Employee salary"

activesupport/lib/active_support/core_ext/object/json.rb

@@ -139,14 +139,11 @@ def as_json(options = nil) #:nodoc:
 
 class Array
   def as_json(options = nil) #:nodoc:
-    # use encoder as a proxy to call as_json on all elements, to protect from circular references
-    encoder = options && options[:encoder] || ActiveSupport::JSON::Encoding::Encoder.new(options)
-    map { |v| encoder.as_json(v, options) }
+    map { |v| v.as_json(options && options.dup) }
   end
 
   def encode_json(encoder) #:nodoc:
-    # we assume here that the encoder has already run as_json on self and the elements, so we run encode_json directly
-    "[#{map { |v| v.encode_json(encoder) } * ','}]"
+    "[#{map { |v| v.as_json.encode_json(encoder) } * ','}]"
   end
 end
 
@@ -165,19 +162,11 @@ def as_json(options = nil) #:nodoc:
       self
     end
 
-    # use encoder as a proxy to call as_json on all values in the subset, to protect from circular references
-    encoder = options && options[:encoder] || ActiveSupport::JSON::Encoding::Encoder.new(options)
-    Hash[subset.map { |k, v| [k.to_s, encoder.as_json(v, options)] }]
+    Hash[subset.map { |k, v| [k.to_s, v.as_json(options && options.dup)] }]
   end
 
   def encode_json(encoder) #:nodoc:
-    # values are encoded with use_options = false, because we don't want hash representations from ActiveModel to be
-    # processed once again with as_json with options, as this could cause unexpected results (i.e. missing fields);
-
-    # on the other hand, we need to run as_json on the elements, because the model representation may contain fields
-    # like Time/Date in their original (not jsonified) form, etc.
-
-    "{#{map { |k,v| "#{encoder.encode(k.to_s)}:#{encoder.encode(v, false)}" } * ','}}"
+    "{#{map { |k,v| "#{k.as_json.encode_json(encoder)}:#{v.as_json.encode_json(encoder)}" } * ','}}"
   end
 end
 

activesupport/lib/active_support/json/decoding.rb

@@ -7,6 +7,9 @@ module ActiveSupport
   mattr_accessor :parse_json_times
 
   module JSON
+    # matches YAML-formatted dates
+    DATE_REGEX = /^(?:\d{4}-\d{2}-\d{2}|\d{4}-\d{1,2}-\d{1,2}[T \t]+\d{1,2}:\d{2}:\d{2}(\.[0-9]*)?(([ \t]*)Z|[-+]\d{2}?(:\d{2})?))$/
+    
     class << self
       # Parses a JSON string (JavaScript Object Notation) into a hash.
       # See www.json.org for more info.

activesupport/lib/active_support/json/encoding.rb

@@ -10,7 +10,6 @@
 require 'active_support/core_ext/time/conversions'
 require 'active_support/core_ext/date_time/conversions'
 require 'active_support/core_ext/date/conversions'
-require 'set'
 
 module ActiveSupport
   class << self
@@ -21,9 +20,6 @@ class << self
   end
 
   module JSON
-    # matches YAML-formatted dates
-    DATE_REGEX = /^(?:\d{4}-\d{2}-\d{2}|\d{4}-\d{1,2}-\d{1,2}[T \t]+\d{1,2}:\d{2}:\d{2}(\.[0-9]*)?(([ \t]*)Z|[-+]\d{2}?(:\d{2})?))$/
-
     # Dumps objects in JSON (JavaScript Object Notation).
     # See www.json.org for more info.
     #
@@ -34,56 +30,22 @@ def self.encode(value, options = nil)
     end
 
     module Encoding #:nodoc:
-      class CircularReferenceError < StandardError; end
-
       class Encoder
         attr_reader :options
 
         def initialize(options = nil)
           @options = options || {}
-          @seen = Set.new
-        end
-
-        def encode(value, use_options = true)
-          check_for_circular_references(value) do
-            jsonified = use_options ? value.as_json(options_for(value)) : value.as_json
-            jsonified.encode_json(self)
-          end
         end
 
-        # like encode, but only calls as_json, without encoding to string.
-        def as_json(value, use_options = true)
-          check_for_circular_references(value) do
-            use_options ? value.as_json(options_for(value)) : value.as_json
-          end
-        end
-
-        def options_for(value)
-          if value.is_a?(Array) || value.is_a?(Hash)
-            # hashes and arrays need to get encoder in the options, so that
-            # they can detect circular references.
-            options.merge(:encoder => self)
-          else
-            options.dup
-          end
+        def encode(value)
+          value.as_json(options.dup).encode_json(self)
         end
 
         def escape(string)
           Encoding.escape(string)
         end
-
-        private
-          def check_for_circular_references(value)
-            unless @seen.add?(value.__id__)
-              raise CircularReferenceError, 'object references itself'
-            end
-            yield
-          ensure
-            @seen.delete(value.__id__)
-          end
       end
 
-
       ESCAPED_CHARS = {
         "\x00" => '\u0000', "\x01" => '\u0001', "\x02" => '\u0002',
         "\x03" => '\u0003', "\x04" => '\u0004', "\x05" => '\u0005',
@@ -138,6 +100,28 @@ def escape(string)
           json.force_encoding(::Encoding::UTF_8)
           json
         end
+
+        # Deprecate CircularReferenceError
+        def const_missing(name)
+          if name == :CircularReferenceError
+            message = "The JSON encoder in Rails 4.1 no longer offers protection from circular references. " \
+                      "You are seeing this warning because you are rescuing from (or otherwise referencing) " \
+                      "ActiveSupport::Encoding::CircularReferenceError. In the future, this error will be " \
+                      "removed from Rails. You should remove these rescue blocks from your code and ensure " \
+                      "that your data structures are free of circular references so they can be properly " \
+                      "serialized into JSON.\n\n" \
+                      "For example, the following Hash contains a circular reference to itself:\n" \
+                      "   h = {}\n" \
+                      "   h['circular'] = h\n" \
+                      "In this case, calling h.to_json would not work properly."
+
+            ActiveSupport::Deprecation.warn message
+
+            SystemStackError
+          else
+            super
+          end
+        end
       end
 
       self.use_standard_json_time_format = true

activesupport/test/json/encoding_test.rb

@@ -146,19 +146,25 @@ def test_wide_utf8_roundtrip
   def test_exception_raised_when_encoding_circular_reference_in_array
     a = [1]
     a << a
-    assert_raise(ActiveSupport::JSON::Encoding::CircularReferenceError) { ActiveSupport::JSON.encode(a) }
+    assert_deprecated do
+      assert_raise(ActiveSupport::JSON::Encoding::CircularReferenceError) { ActiveSupport::JSON.encode(a) }
+    end
   end
 
   def test_exception_raised_when_encoding_circular_reference_in_hash
     a = { :name => 'foo' }
     a[:next] = a
-    assert_raise(ActiveSupport::JSON::Encoding::CircularReferenceError) { ActiveSupport::JSON.encode(a) }
+    assert_deprecated do
+      assert_raise(ActiveSupport::JSON::Encoding::CircularReferenceError) { ActiveSupport::JSON.encode(a) }
+    end
   end
 
   def test_exception_raised_when_encoding_circular_reference_in_hash_inside_array
     a = { :name => 'foo', :sub => [] }
     a[:sub] << a
-    assert_raise(ActiveSupport::JSON::Encoding::CircularReferenceError) { ActiveSupport::JSON.encode(a) }
+    assert_deprecated do
+      assert_raise(ActiveSupport::JSON::Encoding::CircularReferenceError) { ActiveSupport::JSON.encode(a) }
+    end
   end
 
   def test_hash_key_identifiers_are_always_quoted