From c8a3d31332074ddc226086ff1f0c042b6e120232 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 1 Dec 2014 12:16:49 -0500 Subject: [PATCH] Check for no `Cmd` on exec create endpoint Fixes #9414 Signed-off-by: Brian Goff --- daemon/exec.go | 5 +++- integration-cli/docker_api_containers_test.go | 6 ++--- integration-cli/docker_api_exec_test.go | 25 +++++++++++++++++++ integration-cli/docker_api_inspect_test.go | 2 +- integration-cli/docker_api_resize_test.go | 4 +-- integration-cli/docker_utils.go | 11 ++++++-- runconfig/exec.go | 20 +++++++++------ 7 files changed, 57 insertions(+), 16 deletions(-) create mode 100644 integration-cli/docker_api_exec_test.go diff --git a/daemon/exec.go b/daemon/exec.go index 2b0f1bcb26..7d6755118e 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -118,7 +118,10 @@ func (d *Daemon) ContainerExecCreate(job *engine.Job) engine.Status { return job.Error(err) } - config := runconfig.ExecConfigFromJob(job) + config, err := runconfig.ExecConfigFromJob(job) + if err != nil { + return job.Error(err) + } entrypoint, args := d.getEntrypointAndArgs(nil, config.Cmd) diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 605c24bf91..f02f619c44 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -23,7 +23,7 @@ func TestContainerApiGetAll(t *testing.T) { t.Fatalf("Error on container creation: %v, output: %q", err, out) } - body, err := sockRequest("GET", "/containers/json?all=1") + body, err := sockRequest("GET", "/containers/json?all=1", nil) if err != nil { t.Fatalf("GET all containers sockRequest failed: %v", err) } @@ -56,7 +56,7 @@ func TestContainerApiGetExport(t *testing.T) { t.Fatalf("Error on container creation: %v, output: %q", err, out) } - body, err := sockRequest("GET", "/containers/"+name+"/export") + body, err := sockRequest("GET", "/containers/"+name+"/export", nil) if err != nil { t.Fatalf("GET containers/export sockRequest failed: %v", err) } @@ -92,7 +92,7 @@ func TestContainerApiGetChanges(t *testing.T) { t.Fatalf("Error on container creation: %v, output: %q", err, out) } - body, err := sockRequest("GET", "/containers/"+name+"/changes") + body, err := sockRequest("GET", "/containers/"+name+"/changes", nil) if err != nil { t.Fatalf("GET containers/changes sockRequest failed: %v", err) } diff --git a/integration-cli/docker_api_exec_test.go b/integration-cli/docker_api_exec_test.go new file mode 100644 index 0000000000..df7122dd75 --- /dev/null +++ b/integration-cli/docker_api_exec_test.go @@ -0,0 +1,25 @@ +package main + +import ( + "bytes" + "fmt" + "os/exec" + "testing" +) + +// Regression test for #9414 +func TestExecApiCreateNoCmd(t *testing.T) { + defer deleteAllContainers() + name := "exec_test" + runCmd := exec.Command(dockerBinary, "run", "-d", "-t", "--name", name, "busybox", "/bin/sh") + if out, _, err := runCommandWithOutput(runCmd); err != nil { + t.Fatal(out, err) + } + + body, err := sockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": nil}) + if err == nil || !bytes.Contains(body, []byte("No exec command specified")) { + t.Fatalf("Expected error when creating exec command with no Cmd specified: %q", err) + } + + logDone("exec create API - returns error when missing Cmd") +} diff --git a/integration-cli/docker_api_inspect_test.go b/integration-cli/docker_api_inspect_test.go index 112299484c..1ff0312581 100644 --- a/integration-cli/docker_api_inspect_test.go +++ b/integration-cli/docker_api_inspect_test.go @@ -24,7 +24,7 @@ func TestInspectApiContainerResponse(t *testing.T) { if testVersion != "latest" { endpoint = "/" + testVersion + endpoint } - body, err := sockRequest("GET", endpoint) + body, err := sockRequest("GET", endpoint, nil) if err != nil { t.Fatalf("sockRequest failed for %s version: %v", testVersion, err) } diff --git a/integration-cli/docker_api_resize_test.go b/integration-cli/docker_api_resize_test.go index 355bfd9977..6ba95c3052 100644 --- a/integration-cli/docker_api_resize_test.go +++ b/integration-cli/docker_api_resize_test.go @@ -16,7 +16,7 @@ func TestResizeApiResponse(t *testing.T) { cleanedContainerID := stripTrailingCharacters(out) endpoint := "/containers/" + cleanedContainerID + "/resize?h=40&w=40" - _, err = sockRequest("POST", endpoint) + _, err = sockRequest("POST", endpoint, nil) if err != nil { t.Fatalf("resize Request failed %v", err) } @@ -41,7 +41,7 @@ func TestResizeApiResponseWhenContainerNotStarted(t *testing.T) { } endpoint := "/containers/" + cleanedContainerID + "/resize?h=40&w=40" - body, err := sockRequest("POST", endpoint) + body, err := sockRequest("POST", endpoint, nil) if err == nil { t.Fatalf("resize should fail when container is not started") } diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index 9b5fa76c0c..2c66ce2d0c 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "encoding/json" "errors" "fmt" "io" @@ -249,7 +251,7 @@ func (d *Daemon) Cmd(name string, arg ...string) (string, error) { return string(b), err } -func sockRequest(method, endpoint string) ([]byte, error) { +func sockRequest(method, endpoint string, data interface{}) ([]byte, error) { // FIX: the path to sock should not be hardcoded sock := filepath.Join("/", "var", "run", "docker.sock") c, err := net.DialTimeout("unix", sock, time.Duration(10*time.Second)) @@ -260,7 +262,12 @@ func sockRequest(method, endpoint string) ([]byte, error) { client := httputil.NewClientConn(c, nil) defer client.Close() - req, err := http.NewRequest(method, endpoint, nil) + jsonData := bytes.NewBuffer(nil) + if err := json.NewEncoder(jsonData).Encode(data); err != nil { + return nil, err + } + + req, err := http.NewRequest(method, endpoint, jsonData) req.Header.Set("Content-Type", "application/json") if err != nil { return nil, fmt.Errorf("could not create new request: %v", err) diff --git a/runconfig/exec.go b/runconfig/exec.go index b83c11bd1d..1ced70a86a 100644 --- a/runconfig/exec.go +++ b/runconfig/exec.go @@ -1,6 +1,8 @@ package runconfig import ( + "fmt" + "github.com/docker/docker/engine" flag "github.com/docker/docker/pkg/mflag" ) @@ -17,7 +19,7 @@ type ExecConfig struct { Cmd []string } -func ExecConfigFromJob(job *engine.Job) *ExecConfig { +func ExecConfigFromJob(job *engine.Job) (*ExecConfig, error) { execConfig := &ExecConfig{ // TODO(vishh): Expose 'User' once it is supported. //User: job.Getenv("User"), @@ -28,11 +30,14 @@ func ExecConfigFromJob(job *engine.Job) *ExecConfig { AttachStderr: job.GetenvBool("AttachStderr"), AttachStdout: job.GetenvBool("AttachStdout"), } - if cmd := job.GetenvList("Cmd"); cmd != nil { - execConfig.Cmd = cmd + cmd := job.GetenvList("Cmd") + if len(cmd) == 0 { + return nil, fmt.Errorf("No exec command specified") } - return execConfig + execConfig.Cmd = cmd + + return execConfig, nil } func ParseExec(cmd *flag.FlagSet, args []string) (*ExecConfig, error) { @@ -47,10 +52,11 @@ func ParseExec(cmd *flag.FlagSet, args []string) (*ExecConfig, error) { return nil, err } parsedArgs := cmd.Args() - if len(parsedArgs) > 1 { - container = cmd.Arg(0) - execCmd = parsedArgs[1:] + if len(parsedArgs) < 2 { + return nil, fmt.Errorf("not enough arguments to create exec command") } + container = cmd.Arg(0) + execCmd = parsedArgs[1:] execConfig := &ExecConfig{ // TODO(vishh): Expose '-u' flag once it is supported.