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

Retain string characters if add'l argument has spaces #2551

Merged
merged 1 commit into from
Jan 25, 2017

Conversation

farisj
Copy link
Contributor

@farisj farisj commented Jan 25, 2017

Summary

Consider the instance where you're using optional arguments with yarn run, and one of these arguments is a string that contains spaces. At the moment, using the raw process.argv to obtain these arguments strips the individual arguments of any string separators. Since these arguments are then used in another command, their original intent is lost.

Examples of this problem, as run on the yarn repo itself:

bin/yarn run build -- --module "run | init"
#=> gulp build --module run | init
#=> sh: init: command not found

In the above, the command defined in build interprets | init as a bash command. This is not the intent. "run | init" should be maintained as a single argument.

Test plan

To fix this, this PR sanitizes the optional arguments used in execCommand for the run command. It wraps any argument in " characters if the argument value itself contains a string character.

bin/yarn run build -- --module "run | init"
gulp build --module "run | init"
#=> success!

Not that those arguments would be valid for that command, but one can see that the arguments are passed correctly to whatever internal script is specified.

@bestander bestander merged commit 741bb68 into yarnpkg:master Jan 25, 2017
ConAntonakos pushed a commit to ConAntonakos/yarn that referenced this pull request Jan 30, 2017
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