ohler55 / oj

Optimized JSON
http://www.ohler.com/oj
MIT License
3.12k stars 250 forks source link

stack level too deep when using `JSON.parse`. #888

Closed ioquatix closed 11 months ago

ioquatix commented 11 months ago

The following code fails:

require 'json'
require 'oj'
require 'oj/json'

JSON.parse("[]")

Also, oj/json should probably require json and oj if it doesn't already. Certainly without require 'oj' it fails with uninitialized constant 'JSON::Oj'.

ohler55 commented 11 months ago

The oj/json file is for compatibility when mimicking the JSON gem. The module is not Oj::JSON nor is is JSON::Oj and it can not be if the JSON gem is to be replaced by Oj.

ioquatix commented 11 months ago

That all seems reasonable to me, and I understand it's a compatibility shim. However that information seems tangential to my bug report?

ohler55 commented 11 months ago

My misunderstanding. I thought the issue was you were trying to access JSON::Oj.

By requiring both json and oj and then also including the oj/json file you have part of Oj and part of the JSON gem working against each other. Oj is not intended to be "half way" setup. The intend is to either use the JSON gem or Oj. Both can be used at the same time but not if Oj is kind-of sort-of setup to mimic the JSON gem which is the situation you get when you use the JSON gem and then require a file intended to support Oj mimic the JSON gem. The appropriate way to mimic the JSON gem is:

require 'oj'
require 'oj/json'

Oj.mimic_JSON

JSON.parse("[]")

A require 'json' can also be included as long as Oj.mimc_JSON is called.

ioquatix commented 11 months ago

I suppose my question is, just doing the obvious thing, does not work.

require 'oj/json'

Fails too.

ohler55 commented 11 months ago

Not sure it's the obvious thing to do but we each have different perspectives. What would you suggest for making it more clear how to use Oj? Additional documentation somewhere? Examples? Something else?

ioquatix commented 11 months ago

Let's say you have a library that you want to use oj, so you write

require 'oj`

You might have another library and you want the JSON shim, so what do you write?

require 'oj/json'

But this does not work. And you can't control what other libraries may or may not do.

It's my impression that it's easiest if require 'oj/json' just works without any further effort.

ohler55 commented 11 months ago

Since you really seem to have your heart set on requiring an internal Oj file I can add a check on load and warn if it is required when it is not appropriate and also not load the contents in that case.

ohler55 commented 11 months ago

Please try the oj-json-protect branch.

ohler55 commented 11 months ago

No response but v3.15.1 has the fix.

ioquatix commented 11 months ago

Thanks!