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

ActiveSupport::JSON used in json_common.rb #73

Closed
ogtfaber opened this issue Jan 3, 2013 · 2 comments

Comments

@ogtfaber
Copy link

@ogtfaber ogtfaber commented Jan 3, 2013

There seems to be an error in json_common.rb, which we found out due to a bug in the ActiveSupport::JSON module.

ActiveSupport::JSON does not encode 16+ bits characters, but chops them off – as we found out the other day.
JSON.generate however, does not have this bug and does the job well.

The point is that in json_common.rb on line 7, the library uses JSON.parse, but on line 11, the .to_json uses ActiveSupport::JSON, which is another library that contains that bug.

Shouldn't the library use JSON.generate here?

@sferik sferik closed this in b11cf39 Jan 3, 2013
@ogtfaber

This comment has been minimized.

Copy link
Author

@ogtfaber ogtfaber commented Jan 3, 2013

Thanks!

@apepper

This comment has been minimized.

Copy link

@apepper apepper commented May 9, 2017

AFAICS this is still the case with the most recent version of multi_json (1.12.1).

I know, that there has been quite some discussion about this, e.g. in #86 and #138 (comment).

But if I'm not mistaken, a patch has landed in Rails since late 2013, which provides compatibility between ActiveSupport::JSON and pure JSON (see rails/rails#12862 for details). It's included since rails 4.1.0 (see https://github.com/rails/rails/blob/v4.1.0/activesupport/lib/active_support/core_ext/object/json.rb#L14-L43 as proof).

So if it is fine to no longer support rails prior to version 4.1 (which is out since April 2014) I suggest to switch to JSON.generate for json_common.rb again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.