fix: escape empty arguments #20
Closed
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.
A try to fix #19 .
I read https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/sh.js#L30
and understood that
shis equivalent toShellString.sh(...).toString().Although examples of https://gitlab.com/rhendric/puka/-/tree/v1.0.1#quoteforshell
applies
quoteForShellonly forshtemplate arguments ,the format function of https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/ShellStringText.js
delegated by https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/ShellString.js#L49
also quotes parsed arguments of a
shtemplate string .The format function of
ShellStringTextusesformatter.quotewhich actually is
quoteForCmdregistered inhttps://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/Formatter.js
and is equivalent to
quoteForShellas seen inhttps://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/quoteForShell.js .
That means
ShellString.sh(...).toString()virtually appliesquoteForShellto parsed arguments of a
shtemplate string .Although
ShellString.sh(...).toString()does not haveforceQuoteparameter ,escapeCmdin make-spawn-args passesfalseasforceQuotetoquoteForShellandthe default of
forceQuoteis falsy as seen inhttps://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/Formatter.js#L72 and
https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/Formatter.js#L54 .
(But one of uses of
formatter.quoteinShellStringTextpassestrueasquoteForShellas seee in
https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/ShellStringText.js#L46 .
This may cause incompatibility. )
Thus I think replacing
escapeCmdin make-spawn-args withShellString.sh(...).toString()is safe .
One of the differences is that
ShellString.sh(...).toString()replaces an empty stringwith a quoted empty string as seen in
https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/ShellStringText.js#L54
and
https://gitlab.com/rhendric/puka/-/blob/v1.0.1/src/Formatter.js#L30 .
I added a test case for escaping empty arguments in windows environment
and run
npm run testwith node v15.8.0 and npm 7.5.2 on windows 10 cmd.exe .References
Fixes #19