Skip to content

README: Replace deserialize eval with JSON.parse#38

Merged
redonkulus merged 1 commit intoyahoo:masterfrom
unlobito:patch-1
Jan 9, 2024
Merged

README: Replace deserialize eval with JSON.parse#38
redonkulus merged 1 commit intoyahoo:masterfrom
unlobito:patch-1

Conversation

@unlobito
Copy link
Copy Markdown

From json.org:

The eval function is very fast. However, it can compile and execute any JavaScript program, so there can be security issues. The use of eval is indicated when the source is trusted and competent. It is much safer to use a JSON parser. In web applications over XMLHttpRequest, communication is permitted only to the same origin that provide that page, so it is trusted. But it might not be competent. If the server is not rigorous in its JSON encoding, or if it does not scrupulously validate all of its inputs, then it could deliver invalid JSON text that could be carrying dangerous script. The eval function would execute the script, unleashing its malice.

This updates README.md to suggest using JSON.parse instead.

@yahoocla
Copy link
Copy Markdown

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄

This updates README.md to suggest using [JSON.parse](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse) instead of eval to address security concerns.
@gronke
Copy link
Copy Markdown

gronke commented Jun 18, 2018

But isn't the serialization of complex objects the main purpose of this utility? So being able to deserialize it in the same way seems to be a plausible cause for the eval.

@unlobito you're right that this is dangerous and the warning sign should be huge.

@redonkulus redonkulus merged commit 85b0435 into yahoo:master Jan 9, 2024
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.

4 participants