Merge pull request #9435 from cpuguy83/9414_fix_exec_api_error_no_cmd

Check for no `Cmd` on exec create endpoint

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)
 

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)
 	}

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")
+}

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)
 		}

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")
 	}

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)

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.