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

See [1] for why this is not a good idea.

As part of this refactor, circular reference protection in as_json has
been removed and the corresponding error class has been deprecated.

As discussed with @jeremy, circular reference error is considered
programmer errors and protecting against it is out of scope for
the encoder.

This is again based on the excellent work by @sergiocampama in rails#11728.

[1]: intridea/multi_json#138 (comment)

activesupport/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Removed circular reference protection in JSON encoder, deprecated
+    ActiveSupport::JSON::Encoding::CircularReferenceError.
+
+    *Godfrey Chan*, *Sergio Campamá*
+
 *   Added Numeric#in_milliseconds, like 1.hour.in_milliseconds, so we can feed them to JavaScript functions like getTime().
 
 	*DHH*

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/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
@@ -31,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',
@@ -135,6 +100,21 @@ 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 offer protection from circular references. " \
+                      "You should stop relying on ActiveSupport::Encoding::CircularReferenceError as it " \
+                      "will be removed from Rails in future."
+
+            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