Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

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 #11728.

[1]: intridea/multi_json#138 (comment)
  • Loading branch information...
commit 798881ecd4510b9e1e5e10529fc2d81b9deb961e 1 parent 6da5ff4
@chancancode chancancode authored
View
5 activesupport/CHANGELOG.md
@@ -1,3 +1,8 @@
+* 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"
View
19 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
View
61 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,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
View
12 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
Please sign in to comment.
Something went wrong with that request. Please try again.