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

Make sure JSON output strings are sanitized #114

Merged
merged 4 commits into from
May 8, 2018
Merged

Conversation

julik
Copy link
Contributor

@julik julik commented May 7, 2018

Make sure we only output UTF-8 data in JSON generated from the FormatParser
results.

The reason we need to do it in this (rather elaborate) way is this: JSON.generate and
friends use a totally different code path than "foo".to_json(generator_state). We cannot predict
which one of these two ways our users will be using, and at the same time we need to prevent
invalid Strings (ones which cannot be encoded into UTF-8) as well as Float::INFINITY values
from being passed to the JSON encoder. Since we cannot override the JSON generator with
these additions, instead we will deep-convert the entire object being output to make sure
it is up to snuff.

All of the output of AttributesJSON will be sanitized this way.

Used for sanitizing values that are sourced to `JSON::Generator::State#generate`
The reason we need to do this is as follows: `JSON.generate / JSON.dump / JSON.pretty_generate`
use a totally different code path than `"foo".to_json(generator_state)`. We cannot predict
which one of these two ways our users will be using, and at the same time we need to prevent
invalid Strings (ones which cannot be encoded into UTF-8) as well as Float::INFINITY values
from being passed to the JSON encoder. Since we cannot override the JSON generator with
these additions, instead we will deep-convert the entire object being output to make sure
it is up to snuff.

All of the output of AttributesJSON will be sanitized this way.
@julik julik requested a review from roysab May 7, 2018 13:59
@julik
Copy link
Contributor Author

julik commented May 7, 2018

Closes #91

@julik julik merged commit 49d7d26 into master May 8, 2018
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

Successfully merging this pull request may close these issues.

2 participants