fixed some issues with JSON encoding

- as_json in ActiveModel should return a hash
  and handle :only/:except/:methods options
- Array and Hash should call as_json on their elements
- json methods should not modify options argument

[#5374 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

1 files changed, 44 insertions, 6 deletions

diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb
index 2f9588e0f4..6e9d62bd16 100644
--- a/activesupport/lib/active_support/json/encoding.rb
+++ b/activesupport/lib/active_support/json/encoding.rb
@@ -41,9 +41,26 @@ module ActiveSupport
           @seen = []
         end
 
-        def encode(value)
+        def encode(value, use_options = true)
           check_for_circular_references(value) do
-            value.as_json(options).encode_json(self)
+            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)
+          check_for_circular_references(value) do
+            value.as_json(options_for(value))
+          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
           end
         end
 
@@ -186,13 +203,22 @@ module Enumerable
 end
 
 class Array
-  def as_json(options = nil) self end #:nodoc:
-  def encode_json(encoder) "[#{map { |v| encoder.encode(v) } * ','}]" end #:nodoc:
+  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) }
+  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) } * ','}]"
+  end
 end
 
 class Hash
   def as_json(options = nil) #:nodoc:
-    if options
+    # create a subset of the hash by applying :only or :except
+    subset = if options
       if attrs = options[:only]
         slice(*Array.wrap(attrs))
       elsif attrs = options[:except]
@@ -203,10 +229,22 @@ class Hash
     else
       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)
+    pairs = subset.map { |k, v| [k.to_s, encoder.as_json(v)] }
+    result = self.is_a?(ActiveSupport::OrderedHash) ? ActiveSupport::OrderedHash.new : Hash.new
+    pairs.inject(result) { |hash, pair| hash[pair.first] = pair.last; hash }
   end
 
   def encode_json(encoder)
-    "{#{map { |k,v| "#{encoder.encode(k.to_s)}:#{encoder.encode(v)}" } * ','}}"
+    # 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)}" } * ','}}"
   end
 end