review feedback

proxy/common.go

@@ -50,10 +50,6 @@ func unmarshalRequestBody(r *http.Request, target interface{}) error {
 	}
 	r.Body = ioutil.NopCloser(bytes.NewReader(body))
 
-	if len(body) == 0 {
-		return nil
-	}
-
 	d := json.NewDecoder(bytes.NewReader(body))
 	d.UseNumber() // don't want large numbers in scientific format
 	return d.Decode(&target)
@@ -66,7 +62,6 @@ func marshalRequestBody(r *http.Request, body interface{}) error {
 	}
 	r.Body = ioutil.NopCloser(bytes.NewReader(newBody))
 	r.ContentLength = int64(len(newBody))
-	r.Header.Set("Content-Type", "application/json")
 	return nil
 }
 

proxy/proxy.go

@@ -396,6 +396,10 @@ func (proxy *Proxy) setWeaveDNS(hostConfig *docker.HostConfig, hostname, dnsDoma
 }
 
 func (proxy *Proxy) getDNSDomain() (domain string, running bool) {
+	if proxy.WithoutDNS {
+		return "", false
+	}
+
 	domain = nameserver.DefaultDomain
 	running = proxy.WithDNS
 	weaveContainer, err := proxy.client.InspectContainer("weave")

proxy/start_container_interceptor.go

@@ -14,11 +14,11 @@ type startContainerRequestBody struct {
 
 func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error {
 	container, err := inspectContainerInPath(i.proxy.client, r.URL.Path)
-	if err != nil || !containerShouldAttach(container) {
+	if err != nil {
 		return err
 	}
 
-	if r.Header.Get("Content-Type") != "application/json" {
+	if !containerShouldAttach(container) || r.Header.Get("Content-Type") != "application/json" {
 		i.proxy.createWait(r, container.ID)
 		return nil
 	}
@@ -35,8 +35,11 @@ func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error {
 		}
 	}
 
+	if err := marshalRequestBody(r, hostConfig); err != nil {
+		return err
+	}
 	i.proxy.createWait(r, container.ID)
-	return marshalRequestBody(r, hostConfig)
+	return nil
 }
 
 func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error {

test/666_abuse_of_start_test.sh

@@ -5,17 +5,17 @@
 start_suite "Abuse of 'start' operation"
 
 weave_on $HOST1 launch
-
-proxy_start_container $HOST1 --name=c1
-proxy docker_on $HOST1 create --name=c2 $SMALL_IMAGE grep "^1$" /sys/class/net/ethwe/carrier
-# Now start c2 with a sneaky HostConfig
-curl -X POST -H Content-Type:application/json -d '{"NetworkMode": "container:c1"}' http://$HOST1:12375/v1.20/containers/c2/start
 docker_bridge_ip=$(weave_on $HOST1 docker-bridge-ip)
+proxy_start_container $HOST1 --name=c1
+
+# Start c2 with a sneaky HostConfig
+proxy docker_on $HOST1 create --name=c2 $SMALL_IMAGE $CHECK_ETHWE_UP
+proxy docker_api_on $HOST1 POST /containers/c2/start '{"NetworkMode": "container:c1"}'
 assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}}' c2" "false 0 [$docker_bridge_ip]"
 
-# Now start c3 with a mostly null HostConfig
-proxy docker_on $HOST1 create --name=c3 -v /tmp:/hosttmp $SMALL_IMAGE grep "^1$" /sys/class/net/ethwe/carrier
-curl -X POST -H Content-Type:application/json -d '{"Binds":[],"ContainerIDFile":"","LxcConf":null,"Memory":0,"MemorySwap":0,"CpuShares":0,"CpuPeriod":0,"CpusetCpus":"","CpusetMems":"","CpuQuota":0,"BlkioWeight":0,"OomKillDisable":false,"Privileged":false,"PortBindings":null,"Links":[],"PublishAllPorts":false,"Dns":null,"DnsSearch":null,"ExtraHosts":null,"VolumesFrom":null,"Devices":null,"NetworkMode":"","IpcMode":"","PidMode":"","UTSMode":"","CapAdd":null,"CapDrop":null,"RestartPolicy":{"Name":"","MaximumRetryCount":0},"SecurityOpt":null,"ReadonlyRootfs":false,"Ulimits":null,"LogConfig":{"type":"","config":null},"CgroupParent":""}' http://$HOST1:12375/v1.20/containers/c3/start
-assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}} {{index .HostConfig.Binds 0}}' c3" "false 0 [$docker_bridge_ip] /tmp:/hosttmp"
+# Start c3 with HostConfig having empty binds and null dns/networking settings
+proxy docker_on $HOST1 create --name=c3 -v /tmp:/hosttmp $SMALL_IMAGE $CHECK_ETHWE_UP
+proxy docker_api_on $HOST1 POST /containers/c3/start '{"Binds":[],"Dns":null,"DnsSearch":null,"ExtraHosts":null,"VolumesFrom":null,"Devices":null,"NetworkMode":""}'
+assert "docker_on $HOST1 inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.HostConfig.Dns}}' c3" "false 0 [$docker_bridge_ip]"
 
 end_suite

test/config.sh

@@ -99,6 +99,16 @@ docker_on() {
     docker -H tcp://$host:$DOCKER_PORT "$@"
 }
 
+docker_api_on() {
+    host=$1
+    method=$2
+    url=$3
+    data=$4
+    shift 4
+    [ -z "$DEBUG" ] || greyly echo "Docker (API) on $host:$DOCKER_PORT: $method $url" >&2
+    echo -n "$data" | curl -s -X "$method" -H Content-Type:application/json "http://$host:$DOCKER_PORT/v1.15$url" -d @-
+}
+
 proxy() {
     DOCKER_PORT=12375 "$@"
 }