diff --git a/internal/jobcontainers/jobcontainer.go b/internal/jobcontainers/jobcontainer.go index 504a254a39..f8b2c6d56b 100644 --- a/internal/jobcontainers/jobcontainer.go +++ b/internal/jobcontainers/jobcontainer.go @@ -253,12 +253,12 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_ Env: env, Dir: workDir, Path: absPath, - Args: splitArgs(commandLine), SysProcAttr: &syscall.SysProcAttr{ // CREATE_BREAKAWAY_FROM_JOB to make sure that we're not inheriting the job object (and by extension its limits) // from whatever process is going to launch the container. CreationFlags: windows.CREATE_NEW_PROCESS_GROUP | windows.CREATE_BREAKAWAY_FROM_JOB, Token: syscall.Token(token), + CmdLine: commandLine, }, } process := newProcess(cmd) diff --git a/test/cri-containerd/jobcontainer_test.go b/test/cri-containerd/jobcontainer_test.go index 5c82248f33..45ba95af60 100644 --- a/test/cri-containerd/jobcontainer_test.go +++ b/test/cri-containerd/jobcontainer_test.go @@ -1,3 +1,4 @@ +//go:build functional // +build functional package cri_containerd @@ -524,3 +525,42 @@ func Test_RunContainer_WorkingDirectory_JobContainer_WCOW(t *testing.T) { }) } } + +// Test of the fix for the behavior detailed here https://github.com/microsoft/hcsshim/issues/1199 +// The underlying issue was that we would escape the args passed to us to form a commandline, and then split the commandline back into args +// to pass to exec.Cmd in the stdlib. exec.Cmd internally does escaping of its own and thus would result in double quoting for certain +// commandlines. +func Test_DoubleQuoting_JobContainer_WCOW(t *testing.T) { + requireFeatures(t, featureWCOWProcess, featureHostProcess) + + pullRequiredImages(t, []string{imageJobContainerCmdline}) + client := newTestRuntimeClient(t) + + podctx := context.Background() + sandboxRequest := getJobContainerPodRequestWCOW(t) + + podID := runPodSandbox(t, client, podctx, sandboxRequest) + defer removePodSandbox(t, client, podctx, podID) + defer stopPodSandbox(t, client, podctx, podID) + + containerRequest := getJobContainerRequestWCOW(t, podID, sandboxRequest.Config, imageJobContainerCmdline, nil) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + containerID := createContainer(t, client, ctx, containerRequest) + defer removeContainer(t, client, ctx, containerID) + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + + execResponse := execSync(t, client, ctx, &runtime.ExecSyncRequest{ + ContainerId: containerID, + Cmd: []string{"cmdline.exe", `"quote test"`}, + }) + + expected := `cmdline.exe "quote test"` + // Check that there's no double quoting going on. + // e.g. `cmdline.exe ""quote test"" ` + if string(execResponse.Stdout) != expected { + t.Fatalf("expected cmdline for exec to be %q but got %q", expected, string(execResponse.Stdout)) + } +} diff --git a/test/cri-containerd/main_test.go b/test/cri-containerd/main_test.go index 551f77fa96..5c362b1acd 100644 --- a/test/cri-containerd/main_test.go +++ b/test/cri-containerd/main_test.go @@ -1,3 +1,4 @@ +//go:build functional // +build functional package cri_containerd @@ -55,6 +56,7 @@ const ( imageJobContainerHNS = "cplatpublic.azurecr.io/jobcontainer_hns:latest" imageJobContainerETW = "cplatpublic.azurecr.io/jobcontainer_etw:latest" imageJobContainerVHD = "cplatpublic.azurecr.io/jobcontainer_vhd:latest" + imageJobContainerCmdline = "cplatpublic.azurecr.io/jobcontainer_cmdline:latest" imageJobContainerWorkDir = "cplatpublic.azurecr.io/jobcontainer_workdir:latest" alpineAspNet = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11" alpineAspnetUpgrade = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11" diff --git a/test/cri-containerd/test-images/jobcontainer_cmdline/dockerfile b/test/cri-containerd/test-images/jobcontainer_cmdline/dockerfile new file mode 100644 index 0000000000..c3bfda382c --- /dev/null +++ b/test/cri-containerd/test-images/jobcontainer_cmdline/dockerfile @@ -0,0 +1,11 @@ +# Irrelevant what image version we use for job containers as there's no container <-> host OS version restraint. +FROM golang:1.15.10-nanoserver-1809 + +# Get administrator privileges +USER containeradministrator + +WORKDIR C:\\go\\src\\cmdline +COPY main.go . + +RUN go get -d -v ./... +RUN go build -mod=mod \ No newline at end of file diff --git a/test/cri-containerd/test-images/jobcontainer_cmdline/main.go b/test/cri-containerd/test-images/jobcontainer_cmdline/main.go new file mode 100644 index 0000000000..08ed020c0e --- /dev/null +++ b/test/cri-containerd/test-images/jobcontainer_cmdline/main.go @@ -0,0 +1,11 @@ +package main + +import ( + "fmt" + "os" + "strings" +) + +func main() { + fmt.Print(strings.Join(os.Args, " ")) +}