Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

MultiJson.load is broken for JSON gem #55

Closed
romanbsd opened this Issue Aug 18, 2012 · 6 comments

Comments

Projects
None yet
3 participants
require 'json'
require 'multi_json'

MultiJson.load MultiJson.dump('string')

results in:

MultiJson::DecodeError: 743: unexpected token at '"string"'
from /Users/roman/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/json/common.rb:148:in `parse'

I believe that you have to use JSON.load instead of JSON.parse

This will work as well:
JSON.parse '"string"', quirks_mode: true

Owner

mbleigh commented Aug 18, 2012

IIRC strings are not considered valid JSON by themselves...can anyone
verify?
On Aug 18, 2012 12:26 PM, "Roman Shterenzon" notifications@github.com
wrote:

require 'json'require 'multi_json'
MultiJson.load MultiJson.dump('string')

results in:

MultiJson::DecodeError: 743: unexpected token at '"string"'
from
/Users/roman/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/json/common.rb:148:in
`parse'

I believe that you have to use JSON.load instead of JSON.parse


Reply to this email directly or view it on GitHubhttps://github.com/intridea/multi_json/issues/55.

The problem is that this behavior is expected by some gems, for instance:

https://github.com/svenfuchs/i18n/blob/master/lib/i18n/backend/key_value.rb#L93

So ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(str)).should eq(str)

yajl works fine, btw.

Member

sferik commented Aug 18, 2012

From json.org:

JSON is built on two structures:

  • A collection of name/value pairs. In various languages, this is realized as an object, record, struct, dictionary, hash table, keyed list, or associative array.
  • An ordered list of values. In most languages, this is realized as an array, vector, list, or sequence.

If gems depend on parsing invalid JSON, that should be reported as a bug in those gems, not in MultiJSON.

If works in YAJL, you can use YAJL. I don't believe we should neuter YAJL to raise an error in this case to be consistent with other JSON libraries. The way I think about this case is similar to using database-specific features in ActiveRecord::Base#find_by_sql. You're free to deviate from the SQL standard, but compatibility across databases is not guaranteed. Likewise, you're free to deviate from the JSON standard, but compatibility across parsers is not guaranteed.

If MultiJSON does not parse standard JSON consistently, then it's a MultiJSON bug.

In this case the MultiJson.dump shouldn't accept anything but Array or Hash

@sferik sferik closed this Aug 18, 2012

@sferik The current situation is still wrong IMHO, as it's possible to serialize something which cannot be deserialized later.

Member

sferik commented Aug 19, 2012

You make a good point. We're violating the robustness principle on both sides.

That said, I suspect Rails depends on this behavior and we can't break Rails.

If you can show that making this change wouldn't break Rails, I'd consider a patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment