Repository navigation
Reject array literals without ',' between elements - #1905
Open
gengjiawen wants to merge 1 commit into
Open
gengjiawen wants to merge 1 commit into
gengjiawen wants to merge 1 commit into
Conversation
js_parse_array_literal() parses what follows a hole or the 32nd
element in a second loop. Unlike the first loop, it accepted a ','
after each element but did not require one:
[, 1 2] // [ <empty>, 1, 2 ]
[1, , 2 3] // length 4
[, 1 ...[2]] // [ <empty>, 1, 2 ]
and likewise for any ',' missing after the 32nd element. These are now
a SyntaxError.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
js_parse_array_literal()parses an array in up to three loops: the first one handles at most 32 elements without holes, the second one continues with explicit indices after a hole or after the 32nd element, and the third one takes over at the first spread element. The first and third loops require a,between elements. The second one consumes a,if there is one but does not require it, so on master:This adds the check that the first loop already has. They are now rejected with the same messages as in V8 (
SyntaxError: Unexpected number,Unexpected token '...').bellard/quickjs master (e585640) has the same code.
Testing
test_array_literal()intests/test_language.js: holes, trailing commas and more than 32 elements still work, and 5 of the 6assert_throwslines compile on master ([1 2]is the one that was already rejected).make test(0/123 errors),api-test,lre-test,make jscheck, andmake codegenleaves the tree clean. Built with gcc 15 (Release) and clang 21 (Debug,QJS_BUILD_WERROR=ON).make test262: 64/82550 errors, same as master, also with ASan+UBSan. (With ASan+UBSan,make testhas one failure,test_std.jstest_poll_handler_order. It fails the same way on master on this machine and passes when run alone; it depends on a 100 ms timer.)tests/that compile produce the same bytecode as on master (compared theJS_WriteObject()output), and so do 12 large real-world files (24 MB: typescript.js, antd, monaco-editor, three.js, ...).Prepared with the assistance of Claude Fable 5.2.