Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oj skips colons in strings #285

Closed
thedrow opened this issue Feb 8, 2016 · 16 comments
Closed

Oj skips colons in strings #285

thedrow opened this issue Feb 8, 2016 · 16 comments

Comments

@thedrow
Copy link
Contributor

thedrow commented Feb 8, 2016

2.2.0 :001 > require 'oj'
 => true
2.2.0 :002 > Oj.load('{"k": ":"}')
 => {"k"=>:""}

This prevents us from deploying Oj to production.

@ohler55
Copy link
Owner

ohler55 commented Feb 8, 2016

You are using the default :object mode. Change to :compat and it will return a string.

irb(main):001:0> Oj.load('{"k": ":"}', mode: :compat)
=> {"k"=>":"}

@ohler55
Copy link
Owner

ohler55 commented Feb 8, 2016

Or you can change the default_options.

@thedrow
Copy link
Contributor Author

thedrow commented Feb 8, 2016

Why is this the correct behavior? Colons are allowed in JSON strings.

@ohler55
Copy link
Owner

ohler55 commented Feb 8, 2016

Oj has several modes. One, the default is for serializing Ruby objects. It produces valid JSON but has conventions to allow it to create the original objects. http://ohler.com/oj/Oj.html describes the modes. The format used to encode objects is described on http://www.ohler.com/dev/oj_misc/encoding_format.html. Generally you should not try to write those strings directly. Let Oj do it if thats the way you want to use it.

The compat mode loads as native types only so can not be used for direct and optimized object marshaling.

@thedrow
Copy link
Contributor Author

thedrow commented Feb 9, 2016

I still don't understand why colons inside a value of an object's key should be omitted in object mode.
{"k":":-)"} is valid JSON and should be parsed into { :k => ':-)' } at all modes. You are losing information in a non-obvious way.

@thedrow
Copy link
Contributor Author

thedrow commented Feb 9, 2016

I have an example of a JSON that omits ':' inside string values in compat mode as well but I can't share it here since it contains actual production data.
Unfortunately when I scrub the sensitive data out of it, the test I wrote passes.
@ohler55 Can I please send it to you by email?

@ohler55
Copy link
Owner

ohler55 commented Feb 9, 2016

Email would be fine. As for the :object mode, the information is not dropped but if the key does not match the class of object being created it can not be set and if the object class is not set then the rules for converting certain syntax to specific types is followed. The :object is really only for object marshaling with Oj on both ends. Trying to hand craft the JSON for loading is not the intent.

If you can share the :compat example that would be create. The conversion or loss should not occur there unless you are exercising the 'json_class' feature of the Ruby JSON library. If that is the case and it is not what you want try the :strict mode.

@thedrow
Copy link
Contributor Author

thedrow commented Feb 9, 2016

But that means that if Oj serializes a value with a colon in it in object mode it will be dropped. It doesn't make any sense as a sane default.
It also doesn't make any sense if you are serializing an object that has a string attribute with a value of ':-)' in it like in my case.
Moreover, using compat really affects performance and I don't understand why I need it.

As for the problem with compat mode, it turns out I was looking at the wrong JSON. I can't find the single mismatch between Oj and JSON in our logs. I'm debugging though.

@ohler55
Copy link
Owner

ohler55 commented Feb 9, 2016

Oj serializes strings with a colon in it without any problem even in :object mode. Try it and see for yourself. As I mentioned earlier, you should not be trying to construct the :object mode JSON directly. That is not the intended use. Either :compat or :strict mode are appropriate for hand build JSON or generic JSON.

As for what is a sane default, there are many uses for JSON. It happens that that original use of Oj was as a replacement for YAML and marshaling in Ruby to make persistence of Ruby objects in a Redis database faster and more flexible. That is why :object mode is the default. It that context it is a very sane default. It differs from your use and maybe even for a majority of the current users but changing the default now would break a lot of code and it does not make sense to cause those kind of problem just because you prefer a different default when all you need to do is set the default value with one line of code.

Oj.default_options = { :mode => :compat }

@thedrow
Copy link
Contributor Author

thedrow commented Feb 9, 2016

I just demonstrated that Oj doesn't deserialize strings with a colon in them correctly. It omits one colon in object mode.

@thedrow
Copy link
Contributor Author

thedrow commented Feb 9, 2016

I'm not serializing. I'm _deserialising_

@ohler55
Copy link
Owner

ohler55 commented Feb 9, 2016

Can you provide an example that fails. Here is what I get.

irb(main):001:0> Oj.dump(":-)")
=> "\"\\u003a-)\""
irb(main):002:0> Oj.load(Oj.dump(":-)"))
=> ":-)"

@ohler55
Copy link
Owner

ohler55 commented Feb 9, 2016

If you have not generated the JSON with Oj.dump() in object mode, why would you expect load in to create the proper objects? Object mode is for marshaling. If use YAML for marshaling and give it JSON would you expect it to load correctly? You are trying to do the same thing. If you are generating the JSON from anything other than Oj.dump in object mode then just assume it will not give you what you. That is especially true if you do not follow the description of the format in the link I provided earlier. Use compat or strict mode.

@thedrow
Copy link
Contributor Author

thedrow commented Feb 11, 2016

I get it now. See pull request.

@ohler55
Copy link
Owner

ohler55 commented Feb 16, 2016

Ok to close this issue?

@thedrow
Copy link
Contributor Author

thedrow commented Feb 17, 2016

I guess. I hope you'll merge my pull request though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants