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

Escape "-Command" arguments to prevent Terminal API failure #1772

Closed
wants to merge 1 commit into from
Closed

Escape "-Command" arguments to prevent Terminal API failure #1772

wants to merge 1 commit into from

Conversation

foresthoffman
Copy link

@foresthoffman foresthoffman commented Feb 22, 2019

Fixes: https://github.com/codercom/bugs/issues/270.

PR Summary

On Ubuntu 16.04 containers, the Terminal API fails to start the pwsh executable with any "-Command" arguments. This appears to be due to unescaped single quotes within the argument list.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests (NA)
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

- Fixes: https://github.com/codercom/bugs/issues/270.
On Ubuntu 16.04 containers, the Terminal API fails to
start the pwsh executable with any "-Command" arguments.
This appears to be due to unescaped single quotes within
the argument list.
@TylerLeonhardt
Copy link
Member

Tested this out on Windows which uses PowerShell instead of bash as the driver and unfortunately, this change won't work there.

It simply prints out the full command which could be due to the use of '..' which just prints out whatever is inside.

I tried to leverage PowerShell's -EncodedCommand which is like -Command only it let's you pass a base64 encoded string instead of some mess of a command but I had trouble with it playing nice with the base64 encoded strings that the node side gave.

Ill have to investigate further.

@TylerLeonhardt
Copy link
Member

I've got a possible fix opened in #1774 so I'll close this in favor of that.

@foresthoffman
Copy link
Author

foresthoffman commented Feb 26, 2019 via email

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