Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fixed failing JSON decoding in rails 3-0-stable #9126

Merged
merged 1 commit into from

7 participants

@mbarb0sa

This change fixes a JSON decoding problem for strings, fixnums and nil objects introduced by 3.0.20.

ActiveSupport::JSON.decode(ActiveSupport::JSON.encode("foobar"))
ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(42))
ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(nil))

All 3 cases resulted in an ActiveSupport::OkJson::Error being raised.

I've submitted a pull request for Rails 2.3.16 as well. See #9107

@tenderlove tenderlove merged commit 360af4e into rails:3-0-stable
@justinsoong

would be good to get a 3.0.21 release out, since this fix is for a .20 introduced degradation + include the latest sec fixes

@al2o3cr

@steveklabnik - weren't you just pointing out how people shouldn't be calling JSON.load? This patch explicitly swaps out parse for load...

@tenderlove
Owner

Yes, confirm. I should not have merged this (though it hasn't been released)

@tenderlove
Owner

I guess it never will be released, so I'll go ahead and revert this for now. Sorry. :'(

@tenderlove tenderlove referenced this pull request from a commit
@tenderlove tenderlove Revert "Merge pull request #9126 from mbarb0sa/bugfix/json-decoding-i…
…n-rails-3-0-stable"

This reverts commit 360af4e, reversing
changes made to f93d046.
f2839f1
@zachlipton

I fully acknowledge and understand the "no more 3.0.x" releases policy, but would it not be reasonable to predominantly get the word out that 3.0.20 is known to be broken in fairly critical ways, that developers responsible for applications running on 3.0.20 should look to switch from okjson to JSONGem to avoid this breakage, and that they should then strongly consider upgrading to a supported version of Rails?

From my perspective, maintaining a production app currently running 3.0.20 (and looking to upgrade asap!) and discovering breakage around accented characters and other oddities several days after the fact, it would have been very useful to have some kind of authoritative easy-to-find statement of the situation instead of trawling through pull requests.

@bf4

or since it was reverted

gem 'rails', :git => 'git://github.com/rails/rails.git', :ref => '182d4e3719' # 3.0.21, see https://github.com/rails/rails/pull/9126

could there be a community-maintained branch / fork? (would also likely be useful for the 2.x folks)

@wmakley wmakley referenced this pull request from a commit
@wmakley wmakley Revert "Revert "Merge pull request #9126 from mbarb0sa/bugfix/json-de…
…coding-in-rails-3-0-stable""

This reverts commit f2839f1.
c582cf1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 30, 2013
  1. fixed failing JSON decoding in rails 3-0-stable

    Michel Barbosa authored
This page is out of date. Refresh to see the latest.
View
2  activesupport/lib/active_support/json/backends/jsongem.rb
@@ -12,7 +12,7 @@ def decode(json)
if json.respond_to?(:read)
json = json.read
end
- data = ::JSON.parse(json)
+ data = ::JSON.load(json)
if ActiveSupport.parse_json_times
convert_dates_from(data)
else
View
1  activesupport/lib/active_support/json/backends/okjson.rb
@@ -63,6 +63,7 @@ def textparse(ts)
typ, _, val = ts[0]
case typ
+ when :str, :val then valparse(ts)
when '{' then objparse(ts)
when '[' then arrparse(ts)
else
View
11 activesupport/test/json/decoding_test.rb
@@ -51,7 +51,10 @@ class TestJSONDecoding < ActiveSupport::TestCase
# tests escaping of "\n" char with Yaml backend
%q({"a":"\n"}) => {"a"=>"\n"},
%q({"a":"\u000a"}) => {"a"=>"\n"},
- %q({"a":"Line1\u000aLine2"}) => {"a"=>"Line1\nLine2"}
+ %q({"a":"Line1\u000aLine2"}) => {"a"=>"Line1\nLine2"},
+ "\"foobar\"" => "foobar",
+ "42" => 42,
+ "null" => nil
}
# load the default JSON backend
@@ -87,5 +90,11 @@ class TestJSONDecoding < ActiveSupport::TestCase
def test_failed_json_decoding
assert_raise(ActiveSupport::JSON.parse_error) { ActiveSupport::JSON.decode(%({: 1})) }
end
+
+ def test_decoding_of_json_encoded_string
+ assert_equal "foobar", ActiveSupport::JSON.decode(ActiveSupport::JSON.encode("foobar"))
+ assert_equal 42, ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(42))
+ assert_equal nil, ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(nil))
+ end
end
Something went wrong with that request. Please try again.