Repository navigation
util: do not treat a dash in a short option group as the terminator - #67228
Open
yohannesjoya wants to merge 1 commit into
Open
yohannesjoya wants to merge 1 commit into
yohannesjoya wants to merge 1 commit into
Conversation
parseArgs() expands a short option group such as -a-b into -a, -- and -b. The expanded -- was then handled as the options terminator, so -b became a positional, an option-terminator token was reported although the user never passed --, and the index of every following token was off by one. Report the expanded -- as an option named -, like any other unknown short option in a group. Strict mode now throws ERR_PARSE_ARGS_UNKNOWN_OPTION for -a-b and -a-, and token indexes point at the argument they came from. Fixes: nodejs#67226 Signed-off-by: yohannesjoya <yohannesdestagebru10@gmail.com>
Contributor
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
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.
parseArgs()expands a short option group such as-a-binto-a,--and-bbefore it reads them. The expanded--was then handled as the real options terminator. So-bbecame a positional, the tokens contained anoption-terminatorthe user never passed, and the index of every following token was off by one, which breaks the documentedargs[token.index]contract. In strict mode-a-breported a misleadingERR_PARSE_ARGS_UNEXPECTED_POSITIONALand-a-was accepted silently.This change remembers whether the current argument came from a group expansion. An expanded
--is reported as an option named-, the same way any other unknown short option in a group is reported. Strict mode now throwsERR_PARSE_ARGS_UNKNOWN_OPTIONfor-a-band-a-, and in non-strict mode the tokens point at the argument they came from. A bare--typed by the user is handled as before. This matchesgetopt(3), which reports-a-basinvalid option -- '-'.Fixes: #67226
The three new tests fail on main and pass with this change.
tools/test.pyforparallel/test-parse-args*andmake lint-jspass locally.AI disclosure: I used a coding agent to help find the bug and draft the tests. I reproduced the behavior on v22.20.0 and main, reviewed the change line by line, and ran the tests above locally.