From cfaa2aa99fbd2149872af1c43673cff97673f138 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Fri, 15 Nov 2013 10:26:12 -0800 Subject: Added some failing tests where the JSON encoder is not resolving as_json correctly --- activesupport/test/json/encoding_test.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 00f43be6d4..c28a8a6c24 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -18,8 +18,12 @@ class TestJSONEncoding < ActiveSupport::TestCase end class Custom + def initialize(serialized) + @serialized = serialized + end + def as_json(options) - 'custom' + @serialized end end @@ -81,7 +85,13 @@ class TestJSONEncoding < ActiveSupport::TestCase ObjectTests = [[ Foo.new(1, 2), %({\"a\":1,\"b\":2}) ]] HashlikeTests = [[ Hashlike.new, %({\"bar\":\"world\",\"foo\":\"hello\"}) ]] - CustomTests = [[ Custom.new, '"custom"' ]] + CustomTests = [[ Custom.new("custom"), '"custom"' ], + [ Custom.new(nil), 'null' ], + [ Custom.new(:a), '"a"' ], + [ Custom.new([ :foo, "bar" ]), '["foo","bar"]' ], + [ Custom.new({ :foo => "hello", :bar => "world" }), '{"bar":"world","foo":"hello"}' ], + [ Custom.new(Hashlike.new), '{"bar":"world","foo":"hello"}' ], + [ Custom.new(Custom.new(Custom.new(:a))), '"a"' ]] RegexpTests = [[ /^a/, '"(?-mix:^a)"' ], [/^\w{1,2}[a-z]+/ix, '"(?ix-m:^\\\\w{1,2}[a-z]+)"']] -- cgit v1.2.3 From ca12db0efbf07c1a3182556e99af4ebbfcd59dee Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Fri, 15 Nov 2013 10:29:38 -0800 Subject: Expanded coverage on JSON encoding --- activesupport/test/json/encoding_test.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index c28a8a6c24..1e32830ced 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -22,7 +22,7 @@ class TestJSONEncoding < ActiveSupport::TestCase @serialized = serialized end - def as_json(options) + def as_json(options = nil) @serialized end end @@ -338,7 +338,7 @@ class TestJSONEncoding < ActiveSupport::TestCase assert_equal(%([{"address":{"city":"London"}},{"address":{"city":"Paris"}}]), json) end - def test_to_json_should_not_keep_options_around + def test_hash_to_json_should_not_keep_options_around f = CustomWithOptions.new f.foo = "hello" f.bar = "world" @@ -348,6 +348,16 @@ class TestJSONEncoding < ActiveSupport::TestCase "other_hash" => {"foo"=>"other_foo","test"=>"other_test"}}, ActiveSupport::JSON.decode(hash.to_json)) end + def test_array_to_json_should_not_keep_options_around + f = CustomWithOptions.new + f.foo = "hello" + f.bar = "world" + + array = [f, {"foo" => "other_foo", "test" => "other_test"}] + assert_equal([{"foo"=>"hello","bar"=>"world"}, + {"foo"=>"other_foo","test"=>"other_test"}], ActiveSupport::JSON.decode(array.to_json)) + end + def test_hash_as_json_without_options json = { foo: OptionsTest.new }.as_json assert_equal({"foo" => :default}, json) -- cgit v1.2.3 From 4d02296cfbd69b4d2757dfd20f23d778bb23b81b Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 19 Nov 2013 19:17:55 -0800 Subject: Removed support for encoding BigDecimal as a JSON number This is because the new encoder will no longer support encode_json. Therefore our only choice is to return `to_i` or `to_s` in `BigDecimal#as_json`. Since casting a BigDecimal to an integer is most likely a lossy operation, we chose to encode it as a string. Support for encoding BigDecimal as a string will return via the `activesupport-json_encoder` gem. --- activesupport/lib/active_support/core_ext/object/json.rb | 9 +-------- activesupport/lib/active_support/json/encoding.rb | 6 ------ activesupport/test/json/encoding_test.rb | 11 ----------- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/object/json.rb b/activesupport/lib/active_support/core_ext/object/json.rb index 7c11d44f57..bae9190687 100644 --- a/activesupport/lib/active_support/core_ext/object/json.rb +++ b/activesupport/lib/active_support/core_ext/object/json.rb @@ -132,15 +132,8 @@ class BigDecimal # if the other end knows by contract that the data is supposed to be a # BigDecimal, it still has the chance to post-process the string and get the # real value. - # - # Use ActiveSupport.encode_big_decimal_as_string = true to - # override this behavior. def as_json(options = nil) #:nodoc: - if finite? - ActiveSupport.encode_big_decimal_as_string ? to_s : self - else - nil - end + finite? ? to_s : nil end end diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 0e1c379b5b..5fad5887c6 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -7,7 +7,6 @@ module ActiveSupport class << self delegate :use_standard_json_time_format, :use_standard_json_time_format=, :escape_html_entities_in_json, :escape_html_entities_in_json=, - :encode_big_decimal_as_string, :encode_big_decimal_as_string=, :to => :'ActiveSupport::JSON::Encoding' end @@ -69,10 +68,6 @@ module ActiveSupport # to the Active Support legacy format. attr_accessor :use_standard_json_time_format - # If false, serializes BigDecimal objects as numeric instead of wrapping - # them in a string. - attr_accessor :encode_big_decimal_as_string - attr_accessor :escape_regex attr_reader :escape_html_entities_in_json @@ -118,7 +113,6 @@ module ActiveSupport self.use_standard_json_time_format = true self.escape_html_entities_in_json = true - self.encode_big_decimal_as_string = true end end end diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 1e32830ced..335023e2c3 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -399,17 +399,6 @@ class TestJSONEncoding < ActiveSupport::TestCase ActiveSupport::JSON.decode(json_string_and_date)) end - def test_opt_out_big_decimal_string_serialization - big_decimal = BigDecimal('2.5') - - begin - ActiveSupport.encode_big_decimal_as_string = false - assert_equal big_decimal.to_s, big_decimal.to_json - ensure - ActiveSupport.encode_big_decimal_as_string = true - end - end - def test_nil_true_and_false_represented_as_themselves assert_equal nil, nil.as_json assert_equal true, true.as_json -- cgit v1.2.3 From 80e7552073712e102c584cfc54cb3eff2c1f0f52 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 19 Nov 2013 19:47:34 -0800 Subject: Removed the Ruby encoder and switched to using the JSON gem Got all the tests passing again. Support for `encode_json` has been removed (and consequently the ability to encode `BigDecimal`s as numbers, as mentioned in the previous commit). Install the `activesupport-json_encoder` gem to get it back. --- .../lib/active_support/core_ext/object/json.rb | 28 ----- activesupport/lib/active_support/json/encoding.rb | 124 ++++++++++++--------- activesupport/test/json/encoding_test.rb | 4 +- 3 files changed, 74 insertions(+), 82 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/object/json.rb b/activesupport/lib/active_support/core_ext/object/json.rb index bae9190687..75ab3db268 100644 --- a/activesupport/lib/active_support/core_ext/object/json.rb +++ b/activesupport/lib/active_support/core_ext/object/json.rb @@ -62,40 +62,24 @@ class TrueClass def as_json(options = nil) #:nodoc: self end - - def encode_json(encoder) #:nodoc: - to_s - end end class FalseClass def as_json(options = nil) #:nodoc: self end - - def encode_json(encoder) #:nodoc: - to_s - end end class NilClass def as_json(options = nil) #:nodoc: self end - - def encode_json(encoder) #:nodoc: - 'null' - end end class String def as_json(options = nil) #:nodoc: self end - - def encode_json(encoder) #:nodoc: - encoder.escape(self) - end end class Symbol @@ -108,10 +92,6 @@ class Numeric def as_json(options = nil) #:nodoc: self end - - def encode_json(encoder) #:nodoc: - to_s - end end class Float @@ -159,10 +139,6 @@ class Array def as_json(options = nil) #:nodoc: map { |v| options ? v.as_json(options.dup) : v.as_json } end - - def encode_json(encoder) #:nodoc: - "[#{map { |v| v.as_json.encode_json(encoder) } * ','}]" - end end class Hash @@ -182,10 +158,6 @@ class Hash Hash[subset.map { |k, v| [k.to_s, options ? v.as_json(options.dup) : v.as_json] }] end - - def encode_json(encoder) #:nodoc: - "{#{map { |k,v| "#{k.as_json.encode_json(encoder)}:#{v.as_json.encode_json(encoder)}" } * ','}}" - end end class Time diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 5fad5887c6..945aaaa2cd 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -1,5 +1,3 @@ -#encoding: us-ascii - require 'active_support/core_ext/object/json' require 'active_support/core_ext/module/delegation' @@ -21,72 +19,94 @@ module ActiveSupport end module Encoding #:nodoc: - class Encoder + class Encoder #:nodoc: attr_reader :options def initialize(options = nil) @options = options || {} end + # Encode the given object into a JSON string def encode(value) - value.as_json(options.dup).encode_json(self) + stringify jsonify value.as_json(options.dup) end - def escape(string) - Encoding.escape(string) - end - end + private + # Rails does more escaping than the JSON gem natively does (we + # escape \u2028 and \u2029 and optionally >, <, & to work around + # certain browser problems). + ESCAPED_CHARS = { + "\u2028" => '\u2028', + "\u2029" => '\u2029', + '>' => '\u003e', + '<' => '\u003c', + '&' => '\u0026', + } + + ESCAPE_REGEX_WITH_HTML_ENTITIES = /[\u2028\u2029><&]/u + ESCAPE_REGEX_WITHOUT_HTML_ENTITIES = /[\u2028\u2029]/u + + # This class wraps all the strings we see and does the extra escaping + class EscapedString < String + def to_json(*) + if Encoding.escape_html_entities_in_json + super.gsub ESCAPE_REGEX_WITH_HTML_ENTITIES, ESCAPED_CHARS + else + super.gsub ESCAPE_REGEX_WITHOUT_HTML_ENTITIES, ESCAPED_CHARS + end + end + end - ESCAPED_CHARS = { - "\x00" => '\u0000', "\x01" => '\u0001', "\x02" => '\u0002', - "\x03" => '\u0003', "\x04" => '\u0004', "\x05" => '\u0005', - "\x06" => '\u0006', "\x07" => '\u0007', "\x0B" => '\u000B', - "\x0E" => '\u000E', "\x0F" => '\u000F', "\x10" => '\u0010', - "\x11" => '\u0011', "\x12" => '\u0012', "\x13" => '\u0013', - "\x14" => '\u0014', "\x15" => '\u0015', "\x16" => '\u0016', - "\x17" => '\u0017', "\x18" => '\u0018', "\x19" => '\u0019', - "\x1A" => '\u001A', "\x1B" => '\u001B', "\x1C" => '\u001C', - "\x1D" => '\u001D', "\x1E" => '\u001E', "\x1F" => '\u001F', - "\010" => '\b', - "\f" => '\f', - "\n" => '\n', - "\xe2\x80\xa8" => '\u2028', - "\xe2\x80\xa9" => '\u2029', - "\r" => '\r', - "\t" => '\t', - '"' => '\"', - '\\' => '\\\\', - '>' => '\u003E', - '<' => '\u003C', - '&' => '\u0026', - "#{0xe2.chr}#{0x80.chr}#{0xa8.chr}" => '\u2028', - "#{0xe2.chr}#{0x80.chr}#{0xa9.chr}" => '\u2029', - } + # Mark these as private so we don't leak encoding-specific constructs + private_constant :ESCAPED_CHARS, :ESCAPE_REGEX_WITH_HTML_ENTITIES, + :ESCAPE_REGEX_WITHOUT_HTML_ENTITIES, :EscapedString + + # Recursively turn the given object into a "jsonified" Ruby data structure + # that the JSON gem understands - i.e. we want only Hash, Array, String, + # Numeric, true, false and nil in the final tree. Calls #as_json on it if + # it's not from one of these base types. + # + # This allows developers to implement #as_json withouth having to worry + # about what base types of objects they are allowed to return and having + # to remember calling #as_json recursively. + # + # By default, the options hash is not passed to the children data structures + # to avoid undesiarable result. Develoers must opt-in by implementing + # custom #as_json methods (e.g. Hash#as_json and Array#as_json). + def jsonify(value) + if value.is_a?(Hash) + Hash[value.map { |k, v| [jsonify(k), jsonify(v)] }] + elsif value.is_a?(Array) + value.map { |v| jsonify(v) } + elsif value.is_a?(String) + EscapedString.new(value) + elsif value.is_a?(Numeric) + value + elsif value == true + true + elsif value == false + false + elsif value == nil + nil + else + jsonify value.as_json + end + end + + # Encode a "jsonified" Ruby data structure using the JSON gem + def stringify(jsonified) + ::JSON.generate(jsonified, quirks_mode: true, max_nesting: false) + end + end class << self # If true, use ISO 8601 format for dates and times. Otherwise, fall back # to the Active Support legacy format. attr_accessor :use_standard_json_time_format - attr_accessor :escape_regex - attr_reader :escape_html_entities_in_json - - def escape_html_entities_in_json=(value) - self.escape_regex = \ - if @escape_html_entities_in_json = value - /\xe2\x80\xa8|\xe2\x80\xa9|[\x00-\x1F"\\><&]/ - else - /\xe2\x80\xa8|\xe2\x80\xa9|[\x00-\x1F"\\]/ - end - end - - def escape(string) - string = string.encode(::Encoding::UTF_8, :undef => :replace).force_encoding(::Encoding::BINARY) - json = string.gsub(escape_regex) { |s| ESCAPED_CHARS[s] } - json = %("#{json}") - json.force_encoding(::Encoding::UTF_8) - json - end + # If true, encode >, <, & as escaped unicode sequences (e.g. > as \u003e) + # as a safety measure. + attr_accessor :escape_html_entities_in_json # Deprecate CircularReferenceError def const_missing(name) diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 335023e2c3..c98fe3a7ab 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -66,11 +66,11 @@ class TestJSONEncoding < ActiveSupport::TestCase [ BigDecimal('0.0')/BigDecimal('0.0'), %(null) ], [ BigDecimal('2.5'), %("#{BigDecimal('2.5').to_s}") ]] - StringTests = [[ 'this is the ', %("this is the \\u003Cstring\\u003E")], + StringTests = [[ 'this is the ', %("this is the \\u003cstring\\u003e")], [ 'a "string" with quotes & an ampersand', %("a \\"string\\" with quotes \\u0026 an ampersand") ], [ 'http://test.host/posts/1', %("http://test.host/posts/1")], [ "Control characters: \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\342\200\250\342\200\251", - %("Control characters: \\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\b\\t\\n\\u000B\\f\\r\\u000E\\u000F\\u0010\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017\\u0018\\u0019\\u001A\\u001B\\u001C\\u001D\\u001E\\u001F\\u2028\\u2029") ]] + %("Control characters: \\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\b\\t\\n\\u000b\\f\\r\\u000e\\u000f\\u0010\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017\\u0018\\u0019\\u001a\\u001b\\u001c\\u001d\\u001e\\u001f\\u2028\\u2029") ]] ArrayTests = [[ ['a', 'b', 'c'], %([\"a\",\"b\",\"c\"]) ], [ [1, 'a', :b, nil, false], %([1,\"a\",\"b\",null,false]) ]] -- cgit v1.2.3 From d4ef6c00298103144eeda3f5f40ead2f184afacb Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Wed, 20 Nov 2013 09:21:38 -0800 Subject: Make the JSON encoder pluggable --- activesupport/lib/active_support/json/encoding.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 945aaaa2cd..935dd88a03 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -5,6 +5,7 @@ module ActiveSupport class << self delegate :use_standard_json_time_format, :use_standard_json_time_format=, :escape_html_entities_in_json, :escape_html_entities_in_json=, + :json_encoder, :json_encoder=, :to => :'ActiveSupport::JSON::Encoding' end @@ -15,11 +16,11 @@ module ActiveSupport # ActiveSupport::JSON.encode({ team: 'rails', players: '36' }) # # => "{\"team\":\"rails\",\"players\":\"36\"}" def self.encode(value, options = nil) - Encoding::Encoder.new(options).encode(value) + Encoding.json_encoder.new(options).encode(value) end module Encoding #:nodoc: - class Encoder #:nodoc: + class JSONGemEncoder #:nodoc: attr_reader :options def initialize(options = nil) @@ -108,6 +109,10 @@ module ActiveSupport # as a safety measure. attr_accessor :escape_html_entities_in_json + # Sets the encoder used by Rails to encode Ruby objects into JSON strings + # in +Object#to_json+ and +ActiveSupport::JSON.encode+. + attr_accessor :json_encoder + # Deprecate CircularReferenceError def const_missing(name) if name == :CircularReferenceError @@ -133,6 +138,7 @@ module ActiveSupport self.use_standard_json_time_format = true self.escape_html_entities_in_json = true + self.json_encoder = JSONGemEncoder end end end -- cgit v1.2.3 From 6ef53181ebd2299ba8a3e0e605d955a43c9c6236 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sat, 23 Nov 2013 23:45:33 -0800 Subject: Process::Status should get a :nodoc: [ci skip] --- activesupport/lib/active_support/core_ext/object/json.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/object/json.rb b/activesupport/lib/active_support/core_ext/object/json.rb index 75ab3db268..1675145ffe 100644 --- a/activesupport/lib/active_support/core_ext/object/json.rb +++ b/activesupport/lib/active_support/core_ext/object/json.rb @@ -190,7 +190,7 @@ class DateTime end end -class Process::Status +class Process::Status #:nodoc: def as_json(options = nil) { :exitstatus => exitstatus, :pid => pid } end -- cgit v1.2.3 From 262e2e1188c65cd83183e9e218dd58411ddd9fbf Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 24 Nov 2013 03:22:25 -0800 Subject: Be explicit and use the actual unicode sequence --- activesupport/test/json/encoding_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index c98fe3a7ab..79e639b508 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -69,7 +69,7 @@ class TestJSONEncoding < ActiveSupport::TestCase StringTests = [[ 'this is the ', %("this is the \\u003cstring\\u003e")], [ 'a "string" with quotes & an ampersand', %("a \\"string\\" with quotes \\u0026 an ampersand") ], [ 'http://test.host/posts/1', %("http://test.host/posts/1")], - [ "Control characters: \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\342\200\250\342\200\251", + [ "Control characters: \x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\u2028\u2029", %("Control characters: \\u0000\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006\\u0007\\b\\t\\n\\u000b\\f\\r\\u000e\\u000f\\u0010\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017\\u0018\\u0019\\u001a\\u001b\\u001c\\u001d\\u001e\\u001f\\u2028\\u2029") ]] ArrayTests = [[ ['a', 'b', 'c'], %([\"a\",\"b\",\"c\"]) ], -- cgit v1.2.3