-
Notifications
You must be signed in to change notification settings - Fork 468
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
Tests for proposal-well-formed-stringify #1787
Tests for proposal-well-formed-stringify #1787
Conversation
abec281
to
3e020dd
Compare
The proposal advanced to stage 3 at the September 2018 TC39 meeting. Please remove the “awaiting stage 3” label. |
@@ -3,11 +3,15 @@ | |||
|
|||
/*--- | |||
es5id: 15.12.3_4-1-2 | |||
description: JSON.stringify a circular object throws a TypeError | |||
description: JSON.stringify a indirectly circular object throws a error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2×, i.e.
JSON.stringify on an indirectly circular object throws an error
var char_to_json = { | ||
'"': '\\"', | ||
"\\": "\\\\", | ||
"\x00": "\\u0000", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\x00
could be \0
(it’s not an octal escape sequence)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I addressed the grammar nits, but left this \x00
for internal consistency with the following properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine. It was either the preceding property (\\
) or the following ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Left some comments with minor nits.
This seems good to go. @rwaldron @leobalter Would it be possible to prioritize this PR over others please? V8 is looking to ship this and we'd like to wait until there are official (and merged) Test262 tests. |
https://github.com/tc39/proposal-well-formed-stringify
Prevent
JSON.stringify
from returning ill-formed Unicode strings.Includes moderate refactoring of other
JSON.stringify
tests, which can be separated out upon request.