runconfig: ContainerDecoder(): fix handling of invalid JSON

Implement similar logic as is used in httputils.ReadJSON(). Before
this patch, endpoints using the ContainerDecoder would incorrectly
return a 500 (internal server error) status.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

integration/container/container_test.go

@@ -17,6 +17,8 @@ func TestContainerInvalidJSON(t *testing.T) {
 
 	// POST endpoints that accept / expect a JSON body;
 	endpoints := []string{
+		"/commit",
+		"/containers/create",
 		"/containers/foobar/exec",
 		"/containers/foobar/update",
 		"/exec/foobar/start",
@@ -26,7 +28,8 @@ func TestContainerInvalidJSON(t *testing.T) {
 	if runtime.GOOS != "windows" {
 		endpoints = append(
 			endpoints,
-			"/v1.23/containers/foobar/copy", // deprecated since 1.8 (API v1.20), errors out since 1.12 (API v1.24)
+			"/v1.23/containers/foobar/copy",  // deprecated since 1.8 (API v1.20), errors out since 1.12 (API v1.24)
+			"/v1.23/containers/foobar/start", // accepts a body on API < v1.24
 		)
 	}
 

runconfig/config.go

@@ -40,7 +40,7 @@ func (r ContainerDecoder) DecodeHostConfig(src io.Reader) (*container.HostConfig
 // it's your business to do so
 func decodeContainerConfig(src io.Reader, si *sysinfo.SysInfo) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) {
 	var w ContainerConfigWrapper
-	if err := json.NewDecoder(src).Decode(&w); err != nil {
+	if err := loadJSON(src, &w); err != nil {
 		return nil, nil, nil, err
 	}
 
@@ -72,3 +72,18 @@ func decodeContainerConfig(src io.Reader, si *sysinfo.SysInfo) (*container.Confi
 	}
 	return w.Config, hc, w.NetworkingConfig, nil
 }
+
+// loadJSON is similar to api/server/httputils.ReadJSON()
+func loadJSON(src io.Reader, out interface{}) error {
+	dec := json.NewDecoder(src)
+	if err := dec.Decode(&out); err != nil {
+		if err == io.EOF {
+			return validationError("invalid JSON: got EOF while reading request body")
+		}
+		return validationError("invalid JSON: " + err.Error())
+	}
+	if dec.More() {
+		return validationError("unexpected content after JSON")
+	}
+	return nil
+}

runconfig/config_test.go

@@ -42,27 +42,30 @@ func TestDecodeContainerConfig(t *testing.T) {
 	}
 
 	for _, f := range fixtures {
-		b, err := os.ReadFile(f.file)
+		f := f
-		if err != nil {
+		t.Run(f.file, func(t *testing.T) {
-			t.Fatal(err)
+			b, err := os.ReadFile(f.file)
-		}
+			if err != nil {
+				t.Fatal(err)
+			}
 
-		c, h, _, err := decodeContainerConfig(bytes.NewReader(b), sysinfo.New())
+			c, h, _, err := decodeContainerConfig(bytes.NewReader(b), sysinfo.New())
-		if err != nil {
+			if err != nil {
-			t.Fatal(fmt.Errorf("Error parsing %s: %v", f, err))
+				t.Fatal(err)
-		}
+			}
 
-		if c.Image != image {
+			if c.Image != image {
-			t.Fatalf("Expected %s image, found %s\n", image, c.Image)
+				t.Fatalf("Expected %s image, found %s", image, c.Image)
-		}
+			}
 
-		if len(c.Entrypoint) != len(f.entrypoint) {
+			if len(c.Entrypoint) != len(f.entrypoint) {
-			t.Fatalf("Expected %v, found %v\n", f.entrypoint, c.Entrypoint)
+				t.Fatalf("Expected %v, found %v", f.entrypoint, c.Entrypoint)
-		}
+			}
 
-		if h != nil && h.Memory != 1000 {
+			if h != nil && h.Memory != 1000 {
-			t.Fatalf("Expected memory to be 1000, found %d\n", h.Memory)
+				t.Fatalf("Expected memory to be 1000, found %d", h.Memory)
-		}
+			}
+		})
 	}
 }
 

runconfig/hostconfig.go

@@ -1,7 +1,6 @@
 package runconfig // import "github.com/docker/docker/runconfig"
 
 import (
-	"encoding/json"
 	"io"
 	"strings"
 
@@ -12,7 +11,7 @@ import (
 // It assumes the content of the reader will be JSON, and decodes it.
 func decodeHostConfig(src io.Reader) (*container.HostConfig, error) {
 	var w ContainerConfigWrapper
-	if err := json.NewDecoder(src).Decode(&w); err != nil {
+	if err := loadJSON(src, &w); err != nil {
 		return nil, err
 	}
 	return w.getHostConfig(), nil