Merge pull request #12183 from chancancode/json_encoder_refactor

JSON encoder refactor

3 files changed, 107 insertions, 113 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..1675145ffe 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
@@ -132,15 +112,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 <tt>ActiveSupport.encode_big_decimal_as_string = true</tt> 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
 
@@ -166,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
@@ -189,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
@@ -225,7 +190,7 @@ class DateTime
   end
 end
 
-class Process::Status
+class Process::Status #:nodoc:
   def as_json(options = nil)
     { :exitstatus => exitstatus, :pid => pid }
   end
diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb
index 0e1c379b5b..935dd88a03 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'
 
@@ -7,7 +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=,
-      :encode_big_decimal_as_string, :encode_big_decimal_as_string=,
+      :json_encoder, :json_encoder=,
       :to => :'ActiveSupport::JSON::Encoding'
   end
 
@@ -18,80 +16,102 @@ 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
+      class JSONGemEncoder #: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
 
-        # 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
-
-        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
+        # If true, encode >, <, & as escaped unicode sequences (e.g. > as \u003e)
+        # as a safety measure.
+        attr_accessor :escape_html_entities_in_json
 
-        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
+        # 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)
@@ -118,7 +138,7 @@ module ActiveSupport
 
       self.use_standard_json_time_format = true
       self.escape_html_entities_in_json  = true
-      self.encode_big_decimal_as_string  = true
+      self.json_encoder = JSONGemEncoder
     end
   end
 end
diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb
index 00f43be6d4..79e639b508 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 as_json(options)
-      'custom'
+    def initialize(serialized)
+      @serialized = serialized
+    end
+
+    def as_json(options = nil)
+      @serialized
     end
   end
 
@@ -62,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 <string>',     %("this is the \\u003Cstring\\u003E")],
+  StringTests   = [[ 'this is the <string>',     %("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: \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\"])          ],
                    [ [1, 'a', :b, nil, false], %([1,\"a\",\"b\",null,false]) ]]
@@ -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]+)"']]
 
@@ -328,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"
@@ -338,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)
@@ -379,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