Merge pull request #110 from thaJeztah/18.09_backport_handle_invalid_…

…json

[18.09 backport] API: properly handle invalid JSON to return a 400 status

api/server/router/container/copy.go

@@ -6,12 +6,14 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"io"
 	"net/http"
 
 	"github.com/docker/docker/api/server/httputils"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/errdefs"
 	gddohttputil "github.com/golang/gddo/httputil"
 )
 
@@ -37,7 +39,10 @@ func (s *containerRouter) postContainersCopy(ctx context.Context, w http.Respons
 
 	cfg := types.CopyConfig{}
 	if err := json.NewDecoder(r.Body).Decode(&cfg); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	if cfg.Resource == "" {

api/server/router/container/exec.go

@@ -3,6 +3,7 @@ package container // import "github.com/docker/docker/api/server/router/containe
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -44,7 +45,10 @@ func (s *containerRouter) postContainerExecCreate(ctx context.Context, w http.Re
 
 	execConfig := &types.ExecConfig{}
 	if err := json.NewDecoder(r.Body).Decode(execConfig); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	if len(execConfig.Cmd) == 0 {
@@ -84,7 +88,10 @@ func (s *containerRouter) postContainerExecStart(ctx context.Context, w http.Res
 
 	execStartCheck := &types.ExecStartCheck{}
 	if err := json.NewDecoder(r.Body).Decode(execStartCheck); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	if exists, err := s.backend.ExecExists(execName); !exists {

api/server/router/network/network_routes.go

@@ -3,6 +3,7 @@ package network // import "github.com/docker/docker/api/server/router/network"
 import (
 	"context"
 	"encoding/json"
+	"io"
 	"net/http"
 	"strconv"
 	"strings"
@@ -215,7 +216,10 @@ func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWr
 	}
 
 	if err := json.NewDecoder(r.Body).Decode(&create); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	if nws, err := n.cluster.GetNetworksByName(create.Name); err == nil && len(nws) > 0 {
@@ -261,7 +265,10 @@ func (n *networkRouter) postNetworkConnect(ctx context.Context, w http.ResponseW
 	}
 
 	if err := json.NewDecoder(r.Body).Decode(&connect); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	// Unlike other operations, we does not check ambiguity of the name/ID here.
@@ -282,7 +289,10 @@ func (n *networkRouter) postNetworkDisconnect(ctx context.Context, w http.Respon
 	}
 
 	if err := json.NewDecoder(r.Body).Decode(&disconnect); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	return n.backend.DisconnectContainerFromNetwork(disconnect.Container, vars["id"], disconnect.Force)

api/server/router/plugin/plugin_routes.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"io"
 	"net/http"
 	"strconv"
 	"strings"
@@ -12,6 +13,7 @@ import (
 	"github.com/docker/docker/api/server/httputils"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/pkg/errors"
@@ -276,7 +278,10 @@ func (pr *pluginRouter) pushPlugin(ctx context.Context, w http.ResponseWriter, r
 func (pr *pluginRouter) setPlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var args []string
 	if err := json.NewDecoder(r.Body).Decode(&args); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 	if err := pr.backend.Set(vars["name"], args); err != nil {
 		return err

api/server/router/swarm/cluster_routes.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"strconv"
 
@@ -21,7 +22,10 @@ import (
 func (sr *swarmRouter) initCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var req types.InitRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 	version := httputils.VersionFromContext(ctx)
 	// DefaultAddrPool and SubnetSize were added in API 1.39. Ignore on older API versions.
@@ -40,7 +44,10 @@ func (sr *swarmRouter) initCluster(ctx context.Context, w http.ResponseWriter, r
 func (sr *swarmRouter) joinCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var req types.JoinRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 	return sr.backend.Join(req)
 }
@@ -67,7 +74,10 @@ func (sr *swarmRouter) inspectCluster(ctx context.Context, w http.ResponseWriter
 func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var swarm types.Spec
 	if err := json.NewDecoder(r.Body).Decode(&swarm); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	rawVersion := r.URL.Query().Get("version")
@@ -118,7 +128,10 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter,
 func (sr *swarmRouter) unlockCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var req types.UnlockRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	if err := sr.backend.UnlockSwarm(req); err != nil {
@@ -181,7 +194,10 @@ func (sr *swarmRouter) getService(ctx context.Context, w http.ResponseWriter, r
 func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var service types.ServiceSpec
 	if err := json.NewDecoder(r.Body).Decode(&service); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	// Get returns "" if the header does not exist
@@ -204,7 +220,10 @@ func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter,
 func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var service types.ServiceSpec
 	if err := json.NewDecoder(r.Body).Decode(&service); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	rawVersion := r.URL.Query().Get("version")
@@ -297,7 +316,10 @@ func (sr *swarmRouter) getNode(ctx context.Context, w http.ResponseWriter, r *ht
 func (sr *swarmRouter) updateNode(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var node types.NodeSpec
 	if err := json.NewDecoder(r.Body).Decode(&node); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	rawVersion := r.URL.Query().Get("version")
@@ -376,7 +398,10 @@ func (sr *swarmRouter) getSecrets(ctx context.Context, w http.ResponseWriter, r
 func (sr *swarmRouter) createSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var secret types.SecretSpec
 	if err := json.NewDecoder(r.Body).Decode(&secret); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 	version := httputils.VersionFromContext(ctx)
 	if secret.Templating != nil && versions.LessThan(version, "1.37") {
@@ -414,6 +439,9 @@ func (sr *swarmRouter) getSecret(ctx context.Context, w http.ResponseWriter, r *
 func (sr *swarmRouter) updateSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var secret types.SecretSpec
 	if err := json.NewDecoder(r.Body).Decode(&secret); err != nil {
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
 		return errdefs.InvalidParameter(err)
 	}
 
@@ -447,7 +475,10 @@ func (sr *swarmRouter) getConfigs(ctx context.Context, w http.ResponseWriter, r
 func (sr *swarmRouter) createConfig(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var config types.ConfigSpec
 	if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
-		return err
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
+		return errdefs.InvalidParameter(err)
 	}
 
 	version := httputils.VersionFromContext(ctx)
@@ -486,6 +517,9 @@ func (sr *swarmRouter) getConfig(ctx context.Context, w http.ResponseWriter, r *
 func (sr *swarmRouter) updateConfig(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	var config types.ConfigSpec
 	if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
+		if err == io.EOF {
+			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
+		}
 		return errdefs.InvalidParameter(err)
 	}
 

api/server/router/volume/volume_routes.go

@@ -56,7 +56,7 @@ func (v *volumeRouter) postVolumesCreate(ctx context.Context, w http.ResponseWri
 		if err == io.EOF {
 			return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
 		}
-		return err
+		return errdefs.InvalidParameter(err)
 	}
 
 	volume, err := v.backend.Create(ctx, req.Name, req.Driver, opts.WithCreateOptions(req.DriverOpts), opts.WithCreateLabels(req.Labels))

integration/container/container_test.go

@@ -0,0 +1,42 @@
+package container // import "github.com/docker/docker/integration/container"
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/docker/docker/internal/test/request"
+	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
+)
+
+func TestContainerInvalidJSON(t *testing.T) {
+	defer setupTest(t)()
+
+	endpoints := []string{
+		"/containers/foobar/copy",
+		"/containers/foobar/exec",
+		"/exec/foobar/start",
+	}
+
+	for _, ep := range endpoints {
+		t.Run(ep, func(t *testing.T) {
+			t.Parallel()
+
+			res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON)
+			assert.NilError(t, err)
+			assert.Equal(t, res.StatusCode, http.StatusBadRequest)
+
+			buf, err := request.ReadBody(body)
+			assert.NilError(t, err)
+			assert.Check(t, is.Contains(string(buf), "invalid character 'i' looking for beginning of object key string"))
+
+			res, body, err = request.Post(ep, request.JSON)
+			assert.NilError(t, err)
+			assert.Equal(t, res.StatusCode, http.StatusBadRequest)
+
+			buf, err = request.ReadBody(body)
+			assert.NilError(t, err)
+			assert.Check(t, is.Contains(string(buf), "got EOF while reading request body"))
+		})
+	}
+}

integration/network/network_test.go

@@ -3,13 +3,15 @@ package network // import "github.com/docker/docker/integration/network"
 import (
 	"bytes"
 	"context"
+	"net/http"
 	"os/exec"
 	"strings"
 	"testing"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/internal/test/daemon"
+	"github.com/docker/docker/internal/test/request"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	"gotest.tools/skip"
@@ -56,3 +58,35 @@ func TestRunContainerWithBridgeNone(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(stdout.String(), result.Combined()), "The network namspace of container should be the same with host when --net=host and bridge network is disabled")
 }
+
+func TestNetworkInvalidJSON(t *testing.T) {
+	defer setupTest(t)()
+
+	endpoints := []string{
+		"/networks/create",
+		"/networks/bridge/connect",
+		"/networks/bridge/disconnect",
+	}
+
+	for _, ep := range endpoints {
+		t.Run(ep, func(t *testing.T) {
+			t.Parallel()
+
+			res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON)
+			assert.NilError(t, err)
+			assert.Equal(t, res.StatusCode, http.StatusBadRequest)
+
+			buf, err := request.ReadBody(body)
+			assert.NilError(t, err)
+			assert.Check(t, is.Contains(string(buf), "invalid character 'i' looking for beginning of object key string"))
+
+			res, body, err = request.Post(ep, request.JSON)
+			assert.NilError(t, err)
+			assert.Equal(t, res.StatusCode, http.StatusBadRequest)
+
+			buf, err = request.ReadBody(body)
+			assert.NilError(t, err)
+			assert.Check(t, is.Contains(string(buf), "got EOF while reading request body"))
+		})
+	}
+}

integration/plugin/common/main_test.go

@@ -0,0 +1,27 @@
+package common // import "github.com/docker/docker/integration/plugin/common"
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/docker/docker/internal/test/environment"
+)
+
+var testEnv *environment.Execution
+
+func TestMain(m *testing.M) {
+	var err error
+	testEnv, err = environment.New()
+	if err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+	testEnv.Print()
+	os.Exit(m.Run())
+}
+
+func setupTest(t *testing.T) func() {
+	environment.ProtectAll(t, testEnv)
+	return func() { testEnv.Clean(t) }
+}