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

Fix issue with call to cpptools-wordexp with invalid arguments #10070

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

Colengms
Copy link
Contributor

There are 2 issues here.

  1. When command line arguments contain invalid shell escaping/content, cpptools-wordexp will (intentionally) exit with an error code. This is translated into an exception by execFileSync, which wasn't being caught.
  2. On Alpine linux, despite explicit use of shell: false (which is also the default), execFileSync will perform shell parsing on arguments (making the invocation of cpptools-wordexp redundant). This appears to be a bug in node or Alpine itself. This does not occur on non-Alpine linux distros. This also causes an exception to be thrown from execFileSync when the command line args contain invalid shell escaping/content.

Issue 1 is fixed by catching the exception, which also works around the symptoms of 2. In both cases, the compilerPath will not be usable.

This does not address incorrect command line parsing results on Alpine, as shell parsing is occurring twice. (This only impacts when a user pastes a full command line into compilerPath instead of just the compiler path). This would be addressed by #9681 , as that would allow us to call wordexp directly.

@sean-mcmanus
Copy link
Contributor

Did you want this in 1.13.3 or 1.13.4?

@Colengms Colengms merged commit e8a33ff into main Oct 26, 2022
@Colengms Colengms deleted the coleng/alpine_wordexp_fix branch October 26, 2022 22:10
@sean-mcmanus sean-mcmanus added this to the 1.13.4 milestone Oct 26, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants