From 9db5db1b94bc1000d151315851096dcc6cd3512d Mon Sep 17 00:00:00 2001 From: Darren Stahl Date: Mon, 9 Nov 2015 11:49:16 -0800 Subject: [PATCH] This fixes the case where arguments are escaped twice from Dockerfiles on Windows Signed-off-by: Darren Stahl --- api/client/run.go | 2 ++ builder/dockerfile/dispatchers.go | 4 ++- builder/dockerfile/internals.go | 2 +- daemon/container_windows.go | 1 + daemon/execdriver/driver_windows.go | 1 + .../execdriver/windows/commandlinebuilder.go | 36 +++++++++++++++++++ daemon/execdriver/windows/exec.go | 17 ++------- daemon/execdriver/windows/run.go | 18 ++-------- runconfig/config.go | 1 + 9 files changed, 51 insertions(+), 31 deletions(-) create mode 100644 daemon/execdriver/windows/commandlinebuilder.go diff --git a/api/client/run.go b/api/client/run.go index 205aeebcd7..67837311d4 100644 --- a/api/client/run.go +++ b/api/client/run.go @@ -104,6 +104,8 @@ func (cli *DockerCli) CmdRun(args ...string) error { return nil } + config.ArgsEscaped = false + if !*flDetach { if err := cli.CheckTtyInput(config.AttachStdin, config.Tty); err != nil { return err diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 4322573d03..e2d6f828f5 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -389,6 +389,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) b.runConfig.Cmd = config.Cmd // set build-time environment for 'run'. b.runConfig.Env = append(b.runConfig.Env, cmdBuildEnv...) + // set config as already being escaped, this prevents double escaping on windows + b.runConfig.ArgsEscaped = true logrus.Debugf("[BUILDER] Command to be executed: %v", b.runConfig.Cmd) @@ -479,7 +481,7 @@ func entrypoint(b *Builder, args []string, attributes map[string]bool, original if runtime.GOOS != "windows" { b.runConfig.Entrypoint = stringutils.NewStrSlice("/bin/sh", "-c", parsed[0]) } else { - b.runConfig.Entrypoint = stringutils.NewStrSlice("cmd", "/S /C", parsed[0]) + b.runConfig.Entrypoint = stringutils.NewStrSlice("cmd", "/S", "/C", parsed[0]) } } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index ff98eeb479..2af67e7507 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -181,7 +181,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD if runtime.GOOS != "windows" { b.runConfig.Cmd = stringutils.NewStrSlice("/bin/sh", "-c", fmt.Sprintf("#(nop) %s %s in %s", cmdName, srcHash, dest)) } else { - b.runConfig.Cmd = stringutils.NewStrSlice("cmd", "/S /C", fmt.Sprintf("REM (nop) %s %s in %s", cmdName, srcHash, dest)) + b.runConfig.Cmd = stringutils.NewStrSlice("cmd", "/S", "/C", fmt.Sprintf("REM (nop) %s %s in %s", cmdName, srcHash, dest)) } defer func(cmd *stringutils.StrSlice) { b.runConfig.Cmd = cmd }(cmd) diff --git a/daemon/container_windows.go b/daemon/container_windows.go index 5a73d2f6b4..ac043df991 100644 --- a/daemon/container_windows.go +++ b/daemon/container_windows.go @@ -137,6 +137,7 @@ func (daemon *Daemon) populateCommand(c *Container, env []string) error { LayerPaths: layerPaths, Hostname: c.Config.Hostname, Isolation: c.hostConfig.Isolation, + ArgsEscaped: c.Config.ArgsEscaped, } return nil diff --git a/daemon/execdriver/driver_windows.go b/daemon/execdriver/driver_windows.go index 033051328b..8eddb0c63b 100644 --- a/daemon/execdriver/driver_windows.go +++ b/daemon/execdriver/driver_windows.go @@ -48,6 +48,7 @@ type Command struct { LayerFolder string `json:"layer_folder"` // Layer folder for a command LayerPaths []string `json:"layer_paths"` // Layer paths for a command Isolation runconfig.IsolationLevel `json:"isolation"` // Isolation level for the container + ArgsEscaped bool `json:"args_escaped"` // True if args are already escaped } // ExitStatus provides exit reasons for a container. diff --git a/daemon/execdriver/windows/commandlinebuilder.go b/daemon/execdriver/windows/commandlinebuilder.go new file mode 100644 index 0000000000..3ce8f46fb3 --- /dev/null +++ b/daemon/execdriver/windows/commandlinebuilder.go @@ -0,0 +1,36 @@ +//+build windows + +package windows + +import ( + "errors" + "syscall" + + "github.com/Sirupsen/logrus" + "github.com/docker/docker/daemon/execdriver" +) + +// createCommandLine creates a command line from the Entrypoint and args +// of the ProcessConfig. It escapes the arguments if they are not already +// escaped +func createCommandLine(processConfig *execdriver.ProcessConfig, alreadyEscaped bool) (commandLine string, err error) { + // While this should get caught earlier, just in case, validate that we + // have something to run. + if processConfig.Entrypoint == "" { + return "", errors.New("No entrypoint specified") + } + + // Build the command line of the process + commandLine = processConfig.Entrypoint + logrus.Debugf("Entrypoint: %s", processConfig.Entrypoint) + for _, arg := range processConfig.Arguments { + logrus.Debugf("appending %s", arg) + if !alreadyEscaped { + arg = syscall.EscapeArg(arg) + } + commandLine += " " + arg + } + + logrus.Debugf("commandLine: %s", commandLine) + return commandLine, nil +} diff --git a/daemon/execdriver/windows/exec.go b/daemon/execdriver/windows/exec.go index 85fda473a8..ae7648cb3c 100644 --- a/daemon/execdriver/windows/exec.go +++ b/daemon/execdriver/windows/exec.go @@ -3,7 +3,6 @@ package windows import ( - "errors" "fmt" "github.com/Sirupsen/logrus" @@ -34,22 +33,12 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo // Configure the environment for the process // Note NOT c.ProcessConfig.Tty createProcessParms.Environment = setupEnvironmentVariables(processConfig.Env) - // While this should get caught earlier, just in case, validate that we - // have something to run. - if processConfig.Entrypoint == "" { - err = errors.New("No entrypoint specified") - logrus.Error(err) + createProcessParms.CommandLine, err = createCommandLine(&c.ProcessConfig, c.ArgsEscaped) + + if err != nil { return -1, err } - // Build the command line of the process - createProcessParms.CommandLine = processConfig.Entrypoint - for _, arg := range processConfig.Arguments { - logrus.Debugln("appending ", arg) - createProcessParms.CommandLine += " " + arg - } - logrus.Debugln("commandLine: ", createProcessParms.CommandLine) - // Start the command running in the container. pid, stdin, stdout, stderr, rc, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !processConfig.Tty, createProcessParms) if err != nil { diff --git a/daemon/execdriver/windows/run.go b/daemon/execdriver/windows/run.go index 44ab82258f..f9f8ab4062 100644 --- a/daemon/execdriver/windows/run.go +++ b/daemon/execdriver/windows/run.go @@ -4,13 +4,11 @@ package windows import ( "encoding/json" - "errors" "fmt" "os" "path/filepath" "strconv" "strings" - "syscall" "github.com/Sirupsen/logrus" "github.com/docker/docker/daemon/execdriver" @@ -279,22 +277,12 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd // Configure the environment for the process createProcessParms.Environment = setupEnvironmentVariables(c.ProcessConfig.Env) - // This should get caught earlier, but just in case - validate that we - // have something to run - if c.ProcessConfig.Entrypoint == "" { - err = errors.New("No entrypoint specified") - logrus.Error(err) + createProcessParms.CommandLine, err = createCommandLine(&c.ProcessConfig, c.ArgsEscaped) + + if err != nil { return execdriver.ExitStatus{ExitCode: -1}, err } - // Build the command line of the process - createProcessParms.CommandLine = c.ProcessConfig.Entrypoint - for _, arg := range c.ProcessConfig.Arguments { - logrus.Debugln("appending ", arg) - createProcessParms.CommandLine += " " + syscall.EscapeArg(arg) - } - logrus.Debugf("CommandLine: %s", createProcessParms.CommandLine) - // Start the command running in the container. pid, stdin, stdout, stderr, _, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !c.ProcessConfig.Tty, createProcessParms) if err != nil { diff --git a/runconfig/config.go b/runconfig/config.go index 47e895871e..7e57785b5f 100644 --- a/runconfig/config.go +++ b/runconfig/config.go @@ -30,6 +30,7 @@ type Config struct { StdinOnce bool // If true, close stdin after the 1 attached client disconnects. Env []string // List of environment variable to set in the container Cmd *stringutils.StrSlice // Command to run when starting the container + ArgsEscaped bool `json:",omitempty"` // True if command is already escaped (Windows specific) Image string // Name of the image as it was passed by the operator (eg. could be symbolic) Volumes map[string]struct{} // List of volumes (mounts) used for the container WorkingDir string // Current directory (PWD) in the command will be launched