mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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)
This commit is contained in:
parent
dcee010ce8
commit
ff1192fea4
8 changed files with 32 additions and 31 deletions
|
@ -43,9 +43,7 @@ module ActionDispatch
|
||||||
move_string(t, a).concat(move_regexp(t, a))
|
move_string(t, a).concat(move_regexp(t, a))
|
||||||
end
|
end
|
||||||
|
|
||||||
def to_json
|
def as_json(options = nil)
|
||||||
require 'json'
|
|
||||||
|
|
||||||
simple_regexp = Hash.new { |h,k| h[k] = {} }
|
simple_regexp = Hash.new { |h,k| h[k] = {} }
|
||||||
|
|
||||||
@regexp_states.each do |from, hash|
|
@regexp_states.each do |from, hash|
|
||||||
|
@ -54,11 +52,11 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
JSON.dump({
|
{
|
||||||
regexp_states: simple_regexp,
|
regexp_states: simple_regexp,
|
||||||
string_states: @string_states,
|
string_states: @string_states,
|
||||||
accepting: @accepting
|
accepting: @accepting
|
||||||
})
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
def to_svg
|
def to_svg
|
||||||
|
|
|
@ -2,6 +2,7 @@
|
||||||
require 'abstract_unit'
|
require 'abstract_unit'
|
||||||
require 'controller/fake_controllers'
|
require 'controller/fake_controllers'
|
||||||
require 'active_support/core_ext/object/with_options'
|
require 'active_support/core_ext/object/with_options'
|
||||||
|
require 'active_support/core_ext/object/json'
|
||||||
|
|
||||||
class MilestonesController < ActionController::Base
|
class MilestonesController < ActionController::Base
|
||||||
def index() head :ok end
|
def index() head :ok end
|
||||||
|
@ -86,36 +87,36 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
|
||||||
def test_symbols_with_dashes
|
def test_symbols_with_dashes
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get '/:artist/:song-omg', :to => lambda { |env|
|
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]]
|
[200, {}, [resp]]
|
||||||
}
|
}
|
||||||
end
|
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)
|
assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_id_with_dash
|
def test_id_with_dash
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get '/journey/:id', :to => lambda { |env|
|
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]]
|
[200, {}, [resp]]
|
||||||
}
|
}
|
||||||
end
|
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)
|
assert_equal({"id"=>"faithfully-omg"}, hash)
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_dash_with_custom_regexp
|
def test_dash_with_custom_regexp
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get '/:artist/:song-omg', :constraints => { :song => /\d+/ }, :to => lambda { |env|
|
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]]
|
[200, {}, [resp]]
|
||||||
}
|
}
|
||||||
end
|
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({"artist"=>"journey", "song"=>"123"}, hash)
|
||||||
assert_equal 'Not Found', get(URI('http://example.org/journey/faithfully-omg'))
|
assert_equal 'Not Found', get(URI('http://example.org/journey/faithfully-omg'))
|
||||||
end
|
end
|
||||||
|
@ -123,24 +124,24 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
|
||||||
def test_pre_dash
|
def test_pre_dash
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get '/:artist/omg-:song', :to => lambda { |env|
|
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]]
|
[200, {}, [resp]]
|
||||||
}
|
}
|
||||||
end
|
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)
|
assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_pre_dash_with_custom_regexp
|
def test_pre_dash_with_custom_regexp
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get '/:artist/omg-:song', :constraints => { :song => /\d+/ }, :to => lambda { |env|
|
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]]
|
[200, {}, [resp]]
|
||||||
}
|
}
|
||||||
end
|
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({"artist"=>"journey", "song"=>"123"}, hash)
|
||||||
assert_equal 'Not Found', get(URI('http://example.org/journey/omg-faithfully'))
|
assert_equal 'Not Found', get(URI('http://example.org/journey/omg-faithfully'))
|
||||||
end
|
end
|
||||||
|
@ -160,14 +161,14 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
|
||||||
def test_star_paths_are_greedy_but_not_too_much
|
def test_star_paths_are_greedy_but_not_too_much
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get "/*path", :to => lambda { |env|
|
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]]
|
[200, {}, [x]]
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
expected = { "path" => "foo/bar", "format" => "html" }
|
expected = { "path" => "foo/bar", "format" => "html" }
|
||||||
u = URI('http://example.org/foo/bar.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
|
end
|
||||||
|
|
||||||
def test_optional_star_paths_are_greedy
|
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
|
def test_optional_star_paths_are_greedy_but_not_too_much
|
||||||
rs.draw do
|
rs.draw do
|
||||||
get "/(*filters)", :to => lambda { |env|
|
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]]
|
[200, {}, [x]]
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
@ -193,7 +194,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
|
||||||
expected = { "filters" => "ne_27.065938,-80.6092/sw_25.489856,-82",
|
expected = { "filters" => "ne_27.065938,-80.6092/sw_25.489856,-82",
|
||||||
"format" => "542794" }
|
"format" => "542794" }
|
||||||
u = URI('http://example.org/ne_27.065938,-80.6092/sw_25.489856,-82.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
|
end
|
||||||
|
|
||||||
def test_regexp_precidence
|
def test_regexp_precidence
|
||||||
|
|
|
@ -1,5 +1,6 @@
|
||||||
require 'abstract_unit'
|
require 'abstract_unit'
|
||||||
require 'controller/fake_controllers'
|
require 'controller/fake_controllers'
|
||||||
|
require 'active_support/json/decoding'
|
||||||
|
|
||||||
class TestCaseTest < ActionController::TestCase
|
class TestCaseTest < ActionController::TestCase
|
||||||
class TestController < ActionController::Base
|
class TestController < ActionController::Base
|
||||||
|
@ -622,7 +623,7 @@ XML
|
||||||
@request.headers['Referer'] = "http://nohost.com/home"
|
@request.headers['Referer'] = "http://nohost.com/home"
|
||||||
@request.headers['Content-Type'] = "application/rss+xml"
|
@request.headers['Content-Type'] = "application/rss+xml"
|
||||||
get :test_headers
|
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 "http://nohost.com/home", parsed_env["HTTP_REFERER"]
|
||||||
assert_equal "application/rss+xml", parsed_env["CONTENT_TYPE"]
|
assert_equal "application/rss+xml", parsed_env["CONTENT_TYPE"]
|
||||||
end
|
end
|
||||||
|
@ -631,7 +632,7 @@ XML
|
||||||
@request.headers['HTTP_REFERER'] = "http://example.com/about"
|
@request.headers['HTTP_REFERER'] = "http://example.com/about"
|
||||||
@request.headers['CONTENT_TYPE'] = "application/json"
|
@request.headers['CONTENT_TYPE'] = "application/json"
|
||||||
get :test_headers
|
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 "http://example.com/about", parsed_env["HTTP_REFERER"]
|
||||||
assert_equal "application/json", parsed_env["CONTENT_TYPE"]
|
assert_equal "application/json", parsed_env["CONTENT_TYPE"]
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,4 +1,5 @@
|
||||||
require 'abstract_unit'
|
require 'abstract_unit'
|
||||||
|
require 'active_support/json/decoding'
|
||||||
|
|
||||||
class WebServiceTest < ActionDispatch::IntegrationTest
|
class WebServiceTest < ActionDispatch::IntegrationTest
|
||||||
class TestController < ActionController::Base
|
class TestController < ActionController::Base
|
||||||
|
@ -54,7 +55,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest
|
||||||
|
|
||||||
def test_register_and_use_json_simple
|
def test_register_and_use_json_simple
|
||||||
with_test_route_set do
|
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"}}',
|
post "/", '{"request":{"summary":"content...","title":"JSON"}}',
|
||||||
'CONTENT_TYPE' => 'application/json'
|
'CONTENT_TYPE' => 'application/json'
|
||||||
|
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
require 'abstract_unit'
|
require 'abstract_unit'
|
||||||
require 'json'
|
require 'active_support/json/decoding'
|
||||||
|
|
||||||
module ActionDispatch
|
module ActionDispatch
|
||||||
module Journey
|
module Journey
|
||||||
|
@ -13,7 +13,7 @@ module ActionDispatch
|
||||||
/articles/:id(.:format)
|
/articles/:id(.:format)
|
||||||
}
|
}
|
||||||
|
|
||||||
json = JSON.load table.to_json
|
json = ActiveSupport::JSON.decode table.to_json
|
||||||
assert json['regexp_states']
|
assert json['regexp_states']
|
||||||
assert json['string_states']
|
assert json['string_states']
|
||||||
assert json['accepting']
|
assert json['accepting']
|
||||||
|
|
|
@ -112,7 +112,7 @@ module Blog
|
||||||
end
|
end
|
||||||
|
|
||||||
class RenderJsonTestException < Exception
|
class RenderJsonTestException < Exception
|
||||||
def to_json(options = nil)
|
def as_json(options = nil)
|
||||||
return { :error => self.class.name, :message => self.to_s }.to_json
|
{ :error => self.class.name, :message => self.to_s }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -91,7 +91,7 @@ end
|
||||||
|
|
||||||
class Float
|
class Float
|
||||||
# Encoding Infinity or NaN to JSON should return "null". The default returns
|
# Encoding Infinity or NaN to JSON should return "null". The default returns
|
||||||
# "Infinity" or "NaN" which breaks parsing the JSON. E.g. JSON.parse('[NaN]').
|
# "Infinity" or "NaN" which are not valid JSON.
|
||||||
def as_json(options = nil) #:nodoc:
|
def as_json(options = nil) #:nodoc:
|
||||||
finite? ? self : nil
|
finite? ? self : nil
|
||||||
end
|
end
|
||||||
|
|
|
@ -310,7 +310,7 @@ class TestJSONEncoding < ActiveSupport::TestCase
|
||||||
|
|
||||||
hash = {"foo" => f, "other_hash" => {"foo" => "other_foo", "test" => "other_test"}}
|
hash = {"foo" => f, "other_hash" => {"foo" => "other_foo", "test" => "other_test"}}
|
||||||
assert_equal({"foo"=>{"foo"=>"hello","bar"=>"world"},
|
assert_equal({"foo"=>{"foo"=>"hello","bar"=>"world"},
|
||||||
"other_hash" => {"foo"=>"other_foo","test"=>"other_test"}}, JSON.parse(hash.to_json))
|
"other_hash" => {"foo"=>"other_foo","test"=>"other_test"}}, ActiveSupport::JSON.decode(hash.to_json))
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_struct_encoding
|
def test_struct_encoding
|
||||||
|
@ -335,13 +335,13 @@ class TestJSONEncoding < ActiveSupport::TestCase
|
||||||
assert_equal({"name" => "David",
|
assert_equal({"name" => "David",
|
||||||
"sub" => {
|
"sub" => {
|
||||||
"name" => "David",
|
"name" => "David",
|
||||||
"date" => "2010-01-01" }}, JSON.parse(json_custom))
|
"date" => "2010-01-01" }}, ActiveSupport::JSON.decode(json_custom))
|
||||||
|
|
||||||
assert_equal({"name" => "David", "email" => "sample@example.com"},
|
assert_equal({"name" => "David", "email" => "sample@example.com"},
|
||||||
JSON.parse(json_strings))
|
ActiveSupport::JSON.decode(json_strings))
|
||||||
|
|
||||||
assert_equal({"name" => "David", "date" => "2010-01-01"},
|
assert_equal({"name" => "David", "date" => "2010-01-01"},
|
||||||
JSON.parse(json_string_and_date))
|
ActiveSupport::JSON.decode(json_string_and_date))
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_opt_out_big_decimal_string_serialization
|
def test_opt_out_big_decimal_string_serialization
|
||||||
|
|
Loading…
Reference in a new issue