Skip to content

[release/0.9] Fix commandline double quoting for job containers#1226

Merged
dcantah merged 2 commits intomicrosoft:release/0.9from
dcantah:fix-jobcontainer-quotes-0.9
Nov 10, 2021
Merged

[release/0.9] Fix commandline double quoting for job containers#1226
dcantah merged 2 commits intomicrosoft:release/0.9from
dcantah:fix-jobcontainer-quotes-0.9

Conversation

@dcantah
Copy link
Copy Markdown
Contributor

@dcantah dcantah commented Nov 10, 2021

We already escape the arguments passed to us by Containerd to form a
Windows style commandline, however the commandline was being split back
into arguments and then passed to exec.Cmd from the go stdlib. exec.Cmd
internally also does escaping, which ended up applying some extra quotes
in some cases where the commandline had double/single quotes present. This change
just passes the commandline as is to the Cmdline field on the Windows
syscall.SysProcAttr. Go takes this field as is and doesn't do any further
processing on it which is the behavior we desire.

Signed-off-by: Daniel Canter dcanter@microsoft.com
(cherry picked from commit 7cb95b6)
Signed-off-by: Daniel Canter dcanter@microsoft.com

@dcantah dcantah requested a review from a team as a code owner November 10, 2021 18:43
@katiewasnothere
Copy link
Copy Markdown

Why is this not a cherry-pick of #1207?

@dcantah
Copy link
Copy Markdown
Contributor Author

dcantah commented Nov 10, 2021

Why is this not a cherry-pick of #1207?

Sorry, it is. Forgot to -x

We already escape the arguments passed to us by Containerd to form a
Windows style commandline, however the commandline was being split back
into arguments and then passed to exec.Cmd from the go stdlib. exec.Cmd
internally also does escaping, which ended up applying some extra quotes
in some cases where the commandline had double/single quotes present. This change
just passes the commandline as is to the Cmdline field on the Windows
syscall.SysProcAttr. Go takes this field as is and doesn't do any further
processing on it which is the behavior we desire.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
(cherry picked from commit 7cb95b6)
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah force-pushed the fix-jobcontainer-quotes-0.9 branch from 4b4aed0 to 07893ac Compare November 10, 2021 18:49
@dcantah
Copy link
Copy Markdown
Contributor Author

dcantah commented Nov 10, 2021

@katiewasnothere fixed

@katiewasnothere
Copy link
Copy Markdown

Can you add the tests in from that PR as well?

This change adds a test to verify that commandlines with quotes don't get
additional quotes added on to them when combining the arguments given.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
(cherry picked from commit bc5e914)
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah
Copy link
Copy Markdown
Contributor Author

dcantah commented Nov 10, 2021

@katiewasnothere done

@dcantah dcantah merged commit c7759dd into microsoft:release/0.9 Nov 10, 2021
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