Oj: Inconsistency between `load` with String and IO in strict mode

[asterite]

For example:

require "oj"

Oj.load("", mode: :strict) # raises EncodingError: An empty string is not a valid JSON string

But...

require "oj"

Oj.load(StringIO.new, mode: :strict) # => nil

I believe the second snippet should also raise EncodingError.

As a side note, maybe instead of EncodingError it should raise Oj::ParseError? It's difficult to rescue exceptions this way (one have to put many types instead of just one).

[asterite]

Hm, I found something else:

require "oj"

# Blank, but not empty, string
Oj.load("  ", mode: :strict) # => nil

Should also raise Oj::ParseError in my opinion

[ohler55]

All good points. I'll address them in the next release. Soon.

[asterite]

Great, thanks! Let me know if you'd like me to send a PR addressing all of these. I can try to fix them if you prefer that.

[ohler55]

I'll get to it in the next day or two. Probably easier for me to address both.

[ohler55]

Pushed a fixed version to master. My mistake. Had some branch issues. Anyway, take a look please.

The EncodingError is used for json gem compatibility. If not mimicking the gem is should be Oj::ParseError.

[asterite]

Hi!

Yes, it's "fixed" in a way: the behaviour is now consistently wrong 😄

I'll show with three sections: how did it work in 3.3.6, how it works in 3.3.7 and what I think is the correct way.

In 3.3.6

irb(main):001:0> require "oj"
=> true
irb(main):002:0> Oj::VERSION
=> "3.3.6"
irb(main):003:0> Oj.load("", mode: :strict)
EncodingError: An empty string is not a valid JSON string.
    from (irb):3:in `load'
    from (irb):3
    from /usr/local/bin/irb:11:in `<main>'
irb(main):004:0> Oj.load("   ", mode: :strict)
=> nil
irb(main):005:0> Oj.load(StringIO.new(""), mode: :strict)
=> nil
irb(main):006:0> Oj.load(StringIO.new("   "), mode: :strict)
=> nil

Conclusion:

• load with an empty string and strict mode gives an error (GOOD)
• load with a blank string or empty/blank StringIO returns nil (BAD)

In 3.3.7

irb(main):001:0> require "oj"
=> true
irb(main):002:0> Oj::VERSION
=> "3.3.7"
irb(main):003:0> Oj.load("", mode: :strict)
=> nil
irb(main):004:0> Oj.load("   ", mode: :strict)
=> nil
irb(main):005:0> Oj.load(StringIO.new(""), mode: :strict)
=> nil
irb(main):006:0> Oj.load(StringIO.new("   "), mode: :strict)
=> nil

Conclusion:

• load with an empty/blank string or StringIO returns nil (BAD)

Correct (expected) behaviour

load with an empty/blank string or StringIO should always raise, because an empty/blank string is not a vaild JSON.

As a comparison, JSON.parse('') and JSON.parse(' ') in Javascript throw an exception, and JSON.parse('') and JSON.parse(' ') in Ruby raise an exception.

[ohler55]

Correct behavior is not the correct behavior. It does not matter what JavaScript does. It is the spec that should be followed. In :strict mode Oj should raise on an empty string, agreed. In :compat mode Oj tries to follow the JSON gem. This is the behavior of the JSON gem:

JSON.parse('') => raise
JSON.parse(' ') => raise
JSON.load('') => nil
JSON.load('', nil, allow_blank: false) => raise
JSON.load('', nil, allow_blank: true) => nil
JSON.load(' ') => raise
JSON.load(' ', nil, allow_blank: false) => raise
JSON.load('. ', nil, allow_blank: true) => raise

That is the target. I see I missed the target so will go back and get that right. You should also notice that JSON.load is not the same as parse and not the same as JavaScript.

[ohler55]

Pushed a new version that covers the JSON mimicking better.

In regard to strict mode, if you want a raise on a zero length string or now an all whitespace string make sure the Oj.default_options :empty_string is set to false.

[asterite]

Awesome! Just tried it and it works really well. Thank you so much!

[ohler55]

Great, I'll get a release out as soon as travis finishes.