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

Prevent infinite recursions when using refsoption as false #27

Merged
merged 2 commits into from
Dec 2, 2018

Conversation

zalmoxisus
Copy link
Collaborator

@zalmoxisus zalmoxisus commented Dec 1, 2018

When using new refsoption as false introduced in 8021553, jsan will not handle circular references (as there will be no references to check), so there will be an infinite recursion in those cases. A simple solution is to check if regular JSON.stringify fails and disable that option in that case. It is not the best solution as there could be other exceptions. We could check error message, but it's different in every browser.

I'll keep it open for suggestions and will ship a patch without handling it as refs is new option which won't break any current usage. And anyway it was infinite recursion in recent versions when using reviver.

@zalmoxisus
Copy link
Collaborator Author

zalmoxisus commented Dec 1, 2018

Disregard my previous commit, the second one should handle it, and also with circular option.

@kolodny
Copy link
Owner

kolodny commented Dec 2, 2018

Looks good, feel free to merge and release

@zalmoxisus
Copy link
Collaborator Author

Thanks for reviewing!

@zalmoxisus zalmoxisus merged commit 5b9190c into master Dec 2, 2018
@zalmoxisus zalmoxisus deleted the refs-circular branch December 2, 2018 17:23
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