From 6da5ff45c6d69912fb96870f31aaf0c0590fa212 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 5 Nov 2013 20:36:09 -0800 Subject: Moved AS::JSON::DATE_REGEX as it's only used for decoding --- activesupport/lib/active_support/json/decoding.rb | 3 +++ activesupport/lib/active_support/json/encoding.rb | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/json/decoding.rb b/activesupport/lib/active_support/json/decoding.rb index 315c76199a..8b5fc70dee 100644 --- a/activesupport/lib/active_support/json/decoding.rb +++ b/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. diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 4d73672047..8d8557fe3d 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -21,9 +21,6 @@ module ActiveSupport 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. # -- cgit v1.2.3 From 798881ecd4510b9e1e5e10529fc2d81b9deb961e Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 5 Nov 2013 21:01:38 -0800 Subject: 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]: https://github.com/intridea/multi_json/pull/138#issuecomment-24468223 --- activesupport/CHANGELOG.md | 5 ++ .../lib/active_support/core_ext/object/json.rb | 19 ++----- activesupport/lib/active_support/json/encoding.rb | 61 +++++++++------------- activesupport/test/json/encoding_test.rb | 12 +++-- 4 files changed, 42 insertions(+), 55 deletions(-) (limited to 'activesupport') diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 9e63a2bb17..3bb711c1ab 100644 --- a/activesupport/CHANGELOG.md +++ b/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" diff --git a/activesupport/lib/active_support/core_ext/object/json.rb b/activesupport/lib/active_support/core_ext/object/json.rb index e2a89f5760..34b76e228a 100644 --- a/activesupport/lib/active_support/core_ext/object/json.rb +++ b/activesupport/lib/active_support/core_ext/object/json.rb @@ -139,14 +139,11 @@ end 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 @@ class Hash 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 diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 8d8557fe3d..c9840e9faa 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -10,7 +10,6 @@ require 'active_support/core_ext/object/instance_variables' 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 @@ module ActiveSupport 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 @@ module ActiveSupport 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 diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 99120f1d2a..11a2ca9d90 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -146,19 +146,25 @@ class TestJSONEncoding < ActiveSupport::TestCase 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 -- cgit v1.2.3