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 in strict mode cannot format Symbols into json #1230

Closed
tagomoris opened this issue Sep 13, 2016 · 6 comments
Closed

Oj in strict mode cannot format Symbols into json #1230

tagomoris opened this issue Sep 13, 2016 · 6 comments
Assignees
Labels
bug Something isn't working v0.12 v0.14

Comments

@tagomoris
Copy link
Member

tagomoris commented Sep 13, 2016

As I noted on the comments below, setting Oj's default options in parser_json affects to the behavior of Oj in format_json.
#1147 (comment)

It's too strict, and a kind of bug.
We should use compat mode of Oj globally (need to consider about Oj's default options in both of formatter and parser).

@tagomoris tagomoris added v0.12 bug Something isn't working v0.14 labels Sep 13, 2016
@repeatedly
Copy link
Member

I checked the behaviour and strict is not fit for fluentd.
I will write a patch.

require 'oj'

record = {'key1' => 'value1', :key2 => :value2, 'key3' => ':value3'}

p Oj.default_options[:mode]
result = Oj.dump(record)
puts result
p Oj.load(result)

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

p Oj.default_options[:mode]
result = Oj.dump(record)
puts result
p Oj.load(result)

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

p Oj.default_options[:mode]
result = Oj.dump(record)
puts result
p Oj.load(result)
:object
{"key1":"value1",":key2":":value2","key3":"\u003avalue3"}
{"key1"=>"value1", :key2=>:value2, "key3"=>":value3"}
:compat
{"key1":"value1","key2":"value2","key3":":value3"}
{"key1"=>"value1", "key2"=>"value2", "key3"=>":value3"}
:strict
oj_test.rb:20:in `dump': In :strict mode all Hash keys must be Strings, not Symbol. (TypeError)
        from oj_test.rb:20:in `<main>'

@repeatedly
Copy link
Member

@tagomoris Could you give me an example configuration for checking this issue?
This is hard to write unittests so I want actual configuration.

@tagomoris
Copy link
Member Author

No configuration (for plugins) are needed.
Just write tests which formats hashes including Symbol as values (should be formatted into string), and also parses JSON strings including string values with preceeding ':' (parsed into String).

@tagomoris
Copy link
Member Author

@repeatedly
Copy link
Member

repeatedly commented Sep 23, 2016

Just write tests which formats hashes including Symbol as values (should be formatted into string), and also parses JSON strings including string values with preceeding ':' (parsed into String).

Sorry, I can't understand this sentence correctly.
The problem is oj supports only global options and it causes a conflict between parser/formatter.
In formatter_json tests, all tests are passed because formatter_json uses compat mode.
Adding above test doesn't test this problem.

@tagomoris
Copy link
Member Author

The root cause of this problem depends on the order of files loaded. It can't be tested, because test runner loads all files just once in a test execution.
But it can be tested to confirm Oj's default_option is always equal to one designed, and it can parse/format json objects as expected, regardless of order of loading files.

If you want to test it strictly, spawn another process and do:

  1. require json formatter
  2. require json parser
  3. test Oj's default options are equal to one set in formatter code

and reverse these in sight of parser.
IMO, no need to do these tests... we can't be sure about Oj's default options anytime, because 3rd party plugin can overwrite it anytime.

repeatedly added a commit that referenced this issue Sep 26, 2016
Use compat instead of strict to follow json/yajl way. fix #1230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v0.12 v0.14
Projects
None yet
Development

No branches or pull requests

2 participants