From ff1192fea40c55a11c52e26f22a814d68d058170 Mon Sep 17 00:00:00 2001
From: Godfrey Chan <godfreykfc@gmail.com>
Date: Tue, 5 Nov 2013 20:05:58 -0800
Subject: Eliminate `JSON.{parse,load,generate,dump}` and `def to_json`

JSON.{dump,generate} offered by the JSON gem is not compatiable with
Rails at the moment and can cause a lot of subtle bugs when passed
certain data structures. This changed all direct usage of the JSON gem
in internal Rails code to always go through AS::JSON.{decode,encode}.

We also shouldn't be implementing `to_json` most of the time, and
these occurances are replaced with an equivilent `as_json`
implementation to avoid problems down the road.

See [1] for all the juicy details.

[1]: intridea/multi_json#138 (comment)
---
 actionpack/test/controller/routing_test.rb         | 29 +++++++++++-----------
 actionpack/test/controller/test_case_test.rb       |  5 ++--
 actionpack/test/controller/webservice_test.rb      |  3 ++-
 .../test/journey/gtg/transition_table_test.rb      |  4 +--
 actionpack/test/lib/controller/fake_models.rb      |  4 +--
 5 files changed, 24 insertions(+), 21 deletions(-)

(limited to 'actionpack/test')

diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index 46df1a7bd5..2c84e95c6e 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -2,6 +2,7 @@
 require 'abstract_unit'
 require 'controller/fake_controllers'
 require 'active_support/core_ext/object/with_options'
+require 'active_support/core_ext/object/json'
 
 class MilestonesController < ActionController::Base
   def index() head :ok end
@@ -86,36 +87,36 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
   def test_symbols_with_dashes
     rs.draw do
       get '/:artist/:song-omg', :to => lambda { |env|
-        resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+        resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
         [200, {}, [resp]]
       }
     end
 
-    hash = JSON.load get(URI('http://example.org/journey/faithfully-omg'))
+    hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/faithfully-omg'))
     assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
   end
 
   def test_id_with_dash
     rs.draw do
       get '/journey/:id', :to => lambda { |env|
-        resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+        resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
         [200, {}, [resp]]
       }
     end
 
-    hash = JSON.load get(URI('http://example.org/journey/faithfully-omg'))
+    hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/faithfully-omg'))
     assert_equal({"id"=>"faithfully-omg"}, hash)
   end
 
   def test_dash_with_custom_regexp
     rs.draw do
       get '/:artist/:song-omg', :constraints => { :song => /\d+/ }, :to => lambda { |env|
-        resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+        resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
         [200, {}, [resp]]
       }
     end
 
-    hash = JSON.load get(URI('http://example.org/journey/123-omg'))
+    hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/123-omg'))
     assert_equal({"artist"=>"journey", "song"=>"123"}, hash)
     assert_equal 'Not Found', get(URI('http://example.org/journey/faithfully-omg'))
   end
@@ -123,24 +124,24 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
   def test_pre_dash
     rs.draw do
       get '/:artist/omg-:song', :to => lambda { |env|
-        resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+        resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
         [200, {}, [resp]]
       }
     end
 
-    hash = JSON.load get(URI('http://example.org/journey/omg-faithfully'))
+    hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/omg-faithfully'))
     assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
   end
 
   def test_pre_dash_with_custom_regexp
     rs.draw do
       get '/:artist/omg-:song', :constraints => { :song => /\d+/ }, :to => lambda { |env|
-        resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+        resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
         [200, {}, [resp]]
       }
     end
 
-    hash = JSON.load get(URI('http://example.org/journey/omg-123'))
+    hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/omg-123'))
     assert_equal({"artist"=>"journey", "song"=>"123"}, hash)
     assert_equal 'Not Found', get(URI('http://example.org/journey/omg-faithfully'))
   end
@@ -160,14 +161,14 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
   def test_star_paths_are_greedy_but_not_too_much
     rs.draw do
       get "/*path", :to => lambda { |env|
-        x = JSON.dump env["action_dispatch.request.path_parameters"]
+        x = ActiveSupport::JSON.encode env["action_dispatch.request.path_parameters"]
         [200, {}, [x]]
       }
     end
 
     expected = { "path" => "foo/bar", "format" => "html" }
     u = URI('http://example.org/foo/bar.html')
-    assert_equal expected, JSON.parse(get(u))
+    assert_equal expected, ActiveSupport::JSON.decode(get(u))
   end
 
   def test_optional_star_paths_are_greedy
@@ -185,7 +186,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
   def test_optional_star_paths_are_greedy_but_not_too_much
     rs.draw do
       get "/(*filters)", :to => lambda { |env|
-        x = JSON.dump env["action_dispatch.request.path_parameters"]
+        x = ActiveSupport::JSON.encode env["action_dispatch.request.path_parameters"]
         [200, {}, [x]]
       }
     end
@@ -193,7 +194,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
     expected = { "filters" => "ne_27.065938,-80.6092/sw_25.489856,-82",
                  "format"  => "542794" }
     u = URI('http://example.org/ne_27.065938,-80.6092/sw_25.489856,-82.542794')
-    assert_equal expected, JSON.parse(get(u))
+    assert_equal expected, ActiveSupport::JSON.decode(get(u))
   end
 
   def test_regexp_precidence
diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb
index f75c604277..de0476dbde 100644
--- a/actionpack/test/controller/test_case_test.rb
+++ b/actionpack/test/controller/test_case_test.rb
@@ -1,5 +1,6 @@
 require 'abstract_unit'
 require 'controller/fake_controllers'
+require 'active_support/json/decoding'
 
 class TestCaseTest < ActionController::TestCase
   class TestController < ActionController::Base
@@ -622,7 +623,7 @@ XML
     @request.headers['Referer'] = "http://nohost.com/home"
     @request.headers['Content-Type'] = "application/rss+xml"
     get :test_headers
-    parsed_env = JSON.parse(@response.body)
+    parsed_env = ActiveSupport::JSON.decode(@response.body)
     assert_equal "http://nohost.com/home", parsed_env["HTTP_REFERER"]
     assert_equal "application/rss+xml", parsed_env["CONTENT_TYPE"]
   end
@@ -631,7 +632,7 @@ XML
     @request.headers['HTTP_REFERER'] = "http://example.com/about"
     @request.headers['CONTENT_TYPE'] = "application/json"
     get :test_headers
-    parsed_env = JSON.parse(@response.body)
+    parsed_env = ActiveSupport::JSON.decode(@response.body)
     assert_equal "http://example.com/about", parsed_env["HTTP_REFERER"]
     assert_equal "application/json", parsed_env["CONTENT_TYPE"]
   end
diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb
index b2dfd96606..d80b0e2da0 100644
--- a/actionpack/test/controller/webservice_test.rb
+++ b/actionpack/test/controller/webservice_test.rb
@@ -1,4 +1,5 @@
 require 'abstract_unit'
+require 'active_support/json/decoding'
 
 class WebServiceTest < ActionDispatch::IntegrationTest
   class TestController < ActionController::Base
@@ -54,7 +55,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest
 
   def test_register_and_use_json_simple
     with_test_route_set do
-      with_params_parsers Mime::JSON => Proc.new { |data| JSON.parse(data)['request'].with_indifferent_access } do
+      with_params_parsers Mime::JSON => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do
         post "/", '{"request":{"summary":"content...","title":"JSON"}}',
           'CONTENT_TYPE' => 'application/json'
 
diff --git a/actionpack/test/journey/gtg/transition_table_test.rb b/actionpack/test/journey/gtg/transition_table_test.rb
index 33acba8b65..b968780d8d 100644
--- a/actionpack/test/journey/gtg/transition_table_test.rb
+++ b/actionpack/test/journey/gtg/transition_table_test.rb
@@ -1,5 +1,5 @@
 require 'abstract_unit'
-require 'json'
+require 'active_support/json/decoding'
 
 module ActionDispatch
   module Journey
@@ -13,7 +13,7 @@ module ActionDispatch
             /articles/:id(.:format)
           }
 
-          json = JSON.load table.to_json
+          json = ActiveSupport::JSON.decode table.to_json
           assert json['regexp_states']
           assert json['string_states']
           assert json['accepting']
diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb
index 08af187311..b8b51d86c2 100644
--- a/actionpack/test/lib/controller/fake_models.rb
+++ b/actionpack/test/lib/controller/fake_models.rb
@@ -112,7 +112,7 @@ module Blog
 end
 
 class RenderJsonTestException < Exception
-  def to_json(options = nil)
-    return { :error => self.class.name, :message => self.to_s }.to_json
+  def as_json(options = nil)
+    { :error => self.class.name, :message => self.to_s }
   end
 end
-- 
cgit v1.2.3