Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Enabled quirks mode on JSON.parse, fixes broken test in af9caae #12207

Merged
merged 1 commit into from

2 participants

@chancancode
Owner

It turns out that ActionPack depends on the decoder to parse JSON
"fragments" (e.g. '"a string"', '1', 'null', etc), so we need to
enable quirks mode on JSON.parse. Also added coverage on the decoder
side to prevent regression.

@chancancode chancancode Enabled quirks mode on JSON.parse, fixes broken test in af9caae
It turns out that ActionPack depends on the decoder to parse JSON
"fragments" (e.g. '"a string"', '1', 'null', etc), so we need to
enable quirks mode on JSON.parse. Also added coverage on the decoder
side to prevent regression.
52fb1a9
@rafaelfranca rafaelfranca merged commit 0fbb797 into from
@chancancode
Owner

The JSON gem documentation is horrible =/ It turns out JSON.load and JSON.parse is not that much different (despite the warning), it's just that JSON.load supports IO streams and has the dangerous stuff turned on, but if you are passing a string, both can be tuned to do the same thing by passing different options. These options are more or less undocumented...

As best as I can tell (from the code and the tests), quirks mode should only enable fragments parsing and nothing more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 12, 2013
  1. @chancancode

    Enabled quirks mode on JSON.parse, fixes broken test in af9caae

    chancancode authored
    It turns out that ActionPack depends on the decoder to parse JSON
    "fragments" (e.g. '"a string"', '1', 'null', etc), so we need to
    enable quirks mode on JSON.parse. Also added coverage on the decoder
    side to prevent regression.
This page is out of date. Refresh to see the latest.
View
2  activesupport/lib/active_support/json/decoding.rb
@@ -14,7 +14,7 @@ class << self
# ActiveSupport::JSON.decode("{\"team\":\"rails\",\"players\":\"36\"}")
# => {"team" => "rails", "players" => "36"}
def decode(json, options = {})
- data = ::JSON.parse(json, options.merge(create_additions: false))
+ data = ::JSON.parse(json, options.merge(create_additions: false, quirks_mode: true))
if ActiveSupport.parse_json_times
convert_dates_from(data)
else
View
14 activesupport/test/json/decoding_test.rb
@@ -59,7 +59,16 @@ def self.json_create(object)
%q({"a":"\n"}) => {"a"=>"\n"},
%q({"a":"\u000a"}) => {"a"=>"\n"},
%q({"a":"Line1\u000aLine2"}) => {"a"=>"Line1\nLine2"},
- %q({"json_class":"TestJSONDecoding::Foo"}) => {"json_class"=>"TestJSONDecoding::Foo"}
+ # prevent json unmarshalling
+ %q({"json_class":"TestJSONDecoding::Foo"}) => {"json_class"=>"TestJSONDecoding::Foo"},
+ # json "fragments" - these are invalid JSON, but ActionPack relies on this
+ %q("a string") => "a string",
+ %q(1.1) => 1.1,
+ %q(1) => 1,
+ %q(-1) => -1,
+ %q(true) => true,
+ %q(false) => false,
+ %q(null) => nil
}
TESTS.each_with_index do |(json, expected), index|
@@ -83,7 +92,10 @@ def self.json_create(object)
end
def test_failed_json_decoding
+ assert_raise(ActiveSupport::JSON.parse_error) { ActiveSupport::JSON.decode(%(undefined)) }
+ assert_raise(ActiveSupport::JSON.parse_error) { ActiveSupport::JSON.decode(%({a: 1})) }
assert_raise(ActiveSupport::JSON.parse_error) { ActiveSupport::JSON.decode(%({: 1})) }
+ assert_raise(ActiveSupport::JSON.parse_error) { ActiveSupport::JSON.decode(%()) }
end
def test_cannot_force_json_unmarshalling
Something went wrong with that request. Please try again.