diff --git a/api/server/httputils/httputils.go b/api/server/httputils/httputils.go index 1d671693f6..386695ca32 100644 --- a/api/server/httputils/httputils.go +++ b/api/server/httputils/httputils.go @@ -2,6 +2,7 @@ package httputils // import "github.com/docker/docker/api/server/httputils" import ( "context" + "encoding/json" "io" "mime" "net/http" @@ -56,6 +57,35 @@ func CheckForJSON(r *http.Request) error { return matchesContentType(ct, "application/json") } +// ReadJSON validates the request to have the correct content-type, and decodes +// the request's Body into out. +func ReadJSON(r *http.Request, out interface{}) error { + err := CheckForJSON(r) + if err != nil { + return err + } + if r.Body == nil || r.ContentLength == 0 { + // an empty body is not invalid, so don't return an error; see + // https://lists.w3.org/Archives/Public/ietf-http-wg/2010JulSep/0272.html + return nil + } + + dec := json.NewDecoder(r.Body) + err = dec.Decode(out) + defer r.Body.Close() + if err != nil { + if err == io.EOF { + return errdefs.InvalidParameter(errors.New("invalid JSON: got EOF while reading request body")) + } + return errdefs.InvalidParameter(errors.Wrap(err, "invalid JSON")) + } + + if dec.More() { + return errdefs.InvalidParameter(errors.New("unexpected content after JSON")) + } + return nil +} + // ParseForm ensures the request form is parsed even with invalid content types. // If we don't do this, POST method without Content-type (even with empty body) will fail. func ParseForm(r *http.Request) error { diff --git a/api/server/httputils/httputils_test.go b/api/server/httputils/httputils_test.go index 81602c0836..6f796feede 100644 --- a/api/server/httputils/httputils_test.go +++ b/api/server/httputils/httputils_test.go @@ -1,6 +1,10 @@ package httputils // import "github.com/docker/docker/api/server/httputils" -import "testing" +import ( + "net/http" + "strings" + "testing" +) // matchesContentType func TestJsonContentType(t *testing.T) { @@ -26,3 +30,101 @@ func TestJsonContentType(t *testing.T) { t.Errorf(`expected "%s", got "%v"`, expected, err) } } + +func TestReadJSON(t *testing.T) { + t.Run("nil body", func(t *testing.T) { + req, err := http.NewRequest("POST", "https://example.com/some/path", nil) + if err != nil { + t.Error(err) + } + foo := struct{}{} + err = ReadJSON(req, &foo) + if err != nil { + t.Error(err) + } + }) + + t.Run("empty body", func(t *testing.T) { + req, err := http.NewRequest("POST", "https://example.com/some/path", strings.NewReader("")) + if err != nil { + t.Error(err) + } + foo := struct{ SomeField string }{} + err = ReadJSON(req, &foo) + if err != nil { + t.Error(err) + } + if foo.SomeField != "" { + t.Errorf("expected: '', got: %s", foo.SomeField) + } + }) + + t.Run("with valid request", func(t *testing.T) { + req, err := http.NewRequest("POST", "https://example.com/some/path", strings.NewReader(`{"SomeField":"some value"}`)) + if err != nil { + t.Error(err) + } + req.Header.Set("Content-Type", "application/json") + foo := struct{ SomeField string }{} + err = ReadJSON(req, &foo) + if err != nil { + t.Error(err) + } + if foo.SomeField != "some value" { + t.Errorf("expected: 'some value', got: %s", foo.SomeField) + } + }) + t.Run("with whitespace", func(t *testing.T) { + req, err := http.NewRequest("POST", "https://example.com/some/path", strings.NewReader(` + + {"SomeField":"some value"} + +`)) + if err != nil { + t.Error(err) + } + req.Header.Set("Content-Type", "application/json") + foo := struct{ SomeField string }{} + err = ReadJSON(req, &foo) + if err != nil { + t.Error(err) + } + if foo.SomeField != "some value" { + t.Errorf("expected: 'some value', got: %s", foo.SomeField) + } + }) + + t.Run("with extra content", func(t *testing.T) { + req, err := http.NewRequest("POST", "https://example.com/some/path", strings.NewReader(`{"SomeField":"some value"} and more content`)) + if err != nil { + t.Error(err) + } + req.Header.Set("Content-Type", "application/json") + foo := struct{ SomeField string }{} + err = ReadJSON(req, &foo) + if err == nil { + t.Error("expected an error, got none") + } + expected := "unexpected content after JSON" + if err.Error() != expected { + t.Errorf("expected: '%s', got: %s", expected, err.Error()) + } + }) + + t.Run("invalid JSON", func(t *testing.T) { + req, err := http.NewRequest("POST", "https://example.com/some/path", strings.NewReader(`{invalid json`)) + if err != nil { + t.Error(err) + } + req.Header.Set("Content-Type", "application/json") + foo := struct{ SomeField string }{} + err = ReadJSON(req, &foo) + if err == nil { + t.Error("expected an error, got none") + } + expected := "invalid JSON: invalid character 'i' looking for beginning of object key string" + if err.Error() != expected { + t.Errorf("expected: '%s', got: %s", expected, err.Error()) + } + }) +} diff --git a/api/server/router/checkpoint/checkpoint_routes.go b/api/server/router/checkpoint/checkpoint_routes.go index 6c03f976e2..678497b2a3 100644 --- a/api/server/router/checkpoint/checkpoint_routes.go +++ b/api/server/router/checkpoint/checkpoint_routes.go @@ -2,7 +2,6 @@ package checkpoint // import "github.com/docker/docker/api/server/router/checkpo import ( "context" - "encoding/json" "net/http" "github.com/docker/docker/api/server/httputils" @@ -15,9 +14,7 @@ func (s *checkpointRouter) postContainerCheckpoint(ctx context.Context, w http.R } var options types.CheckpointCreateOptions - - decoder := json.NewDecoder(r.Body) - if err := decoder.Decode(&options); err != nil { + if err := httputils.ReadJSON(r, &options); err != nil { return err } diff --git a/api/server/router/container/container.go b/api/server/router/container/container.go index df68fcb791..7ac9dc7aac 100644 --- a/api/server/router/container/container.go +++ b/api/server/router/container/container.go @@ -56,7 +56,7 @@ func (r *containerRouter) initRoutes() { router.NewPostRoute("/containers/{name:.*}/wait", r.postContainersWait), router.NewPostRoute("/containers/{name:.*}/resize", r.postContainersResize), router.NewPostRoute("/containers/{name:.*}/attach", r.postContainersAttach), - router.NewPostRoute("/containers/{name:.*}/copy", r.postContainersCopy), // Deprecated since 1.8, Errors out since 1.12 + router.NewPostRoute("/containers/{name:.*}/copy", r.postContainersCopy), // Deprecated since 1.8 (API v1.20), errors out since 1.12 (API v1.24) router.NewPostRoute("/containers/{name:.*}/exec", r.postContainerExecCreate), router.NewPostRoute("/exec/{name:.*}/start", r.postContainerExecStart), router.NewPostRoute("/exec/{name:.*}/resize", r.postContainerExecResize), diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index de0a4474a7..392efc023e 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -427,14 +427,9 @@ func (s *containerRouter) postContainerUpdate(ctx context.Context, w http.Respon if err := httputils.ParseForm(r); err != nil { return err } - if err := httputils.CheckForJSON(r); err != nil { - return err - } var updateConfig container.UpdateConfig - - decoder := json.NewDecoder(r.Body) - if err := decoder.Decode(&updateConfig); err != nil { + if err := httputils.ReadJSON(r, &updateConfig); err != nil { return err } if versions.LessThan(httputils.VersionFromContext(ctx), "1.40") { diff --git a/api/server/router/container/copy.go b/api/server/router/container/copy.go index 47de35a455..28920b6d7b 100644 --- a/api/server/router/container/copy.go +++ b/api/server/router/container/copy.go @@ -6,14 +6,12 @@ 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" ) @@ -26,23 +24,18 @@ func (pathError) Error() string { func (pathError) InvalidParameter() {} // postContainersCopy is deprecated in favor of getContainersArchive. +// +// Deprecated since 1.8 (API v1.20), errors out since 1.12 (API v1.24) func (s *containerRouter) postContainersCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - // Deprecated since 1.8, Errors out since 1.12 version := httputils.VersionFromContext(ctx) if versions.GreaterThanOrEqualTo(version, "1.24") { w.WriteHeader(http.StatusNotFound) return nil } - if err := httputils.CheckForJSON(r); err != nil { - return err - } cfg := types.CopyConfig{} - if err := json.NewDecoder(r.Body).Decode(&cfg); err != nil { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &cfg); err != nil { + return err } if cfg.Resource == "" { diff --git a/api/server/router/container/exec.go b/api/server/router/container/exec.go index cf92be1d2b..5a612bc977 100644 --- a/api/server/router/container/exec.go +++ b/api/server/router/container/exec.go @@ -2,8 +2,6 @@ package container // import "github.com/docker/docker/api/server/router/containe import ( "context" - "encoding/json" - "errors" "fmt" "io" "net/http" @@ -38,17 +36,10 @@ func (s *containerRouter) postContainerExecCreate(ctx context.Context, w http.Re if err := httputils.ParseForm(r); err != nil { return err } - if err := httputils.CheckForJSON(r); err != nil { - return err - } - name := vars["name"] execConfig := &types.ExecConfig{} - if err := json.NewDecoder(r.Body).Decode(execConfig); err != nil { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, execConfig); err != nil { + return err } if len(execConfig.Cmd) == 0 { @@ -56,9 +47,9 @@ func (s *containerRouter) postContainerExecCreate(ctx context.Context, w http.Re } // Register an instance of Exec in container. - id, err := s.backend.ContainerExecCreate(name, execConfig) + id, err := s.backend.ContainerExecCreate(vars["name"], execConfig) if err != nil { - logrus.Errorf("Error setting up exec command in container %s: %v", name, err) + logrus.Errorf("Error setting up exec command in container %s: %v", vars["name"], err) return err } @@ -74,9 +65,11 @@ func (s *containerRouter) postContainerExecStart(ctx context.Context, w http.Res } version := httputils.VersionFromContext(ctx) - if versions.GreaterThan(version, "1.21") { - if err := httputils.CheckForJSON(r); err != nil { - return err + if versions.LessThan(version, "1.22") { + // API versions before 1.22 did not enforce application/json content-type. + // Allow older clients to work by patching the content-type. + if r.Header.Get("Content-Type") != "application/json" { + r.Header.Set("Content-Type", "application/json") } } @@ -87,11 +80,8 @@ func (s *containerRouter) postContainerExecStart(ctx context.Context, w http.Res ) execStartCheck := &types.ExecStartCheck{} - if err := json.NewDecoder(r.Body).Decode(execStartCheck); err != nil { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, execStartCheck); err != nil { + return err } if exists, err := s.backend.ExecExists(execName); !exists { diff --git a/api/server/router/network/network_routes.go b/api/server/router/network/network_routes.go index 69fc26a129..01114c8682 100644 --- a/api/server/router/network/network_routes.go +++ b/api/server/router/network/network_routes.go @@ -2,8 +2,6 @@ package network // import "github.com/docker/docker/api/server/router/network" import ( "context" - "encoding/json" - "io" "net/http" "strconv" "strings" @@ -205,23 +203,15 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r } func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - var create types.NetworkCreateRequest - if err := httputils.ParseForm(r); err != nil { return err } - if err := httputils.CheckForJSON(r); err != nil { + var create types.NetworkCreateRequest + if err := httputils.ReadJSON(r, &create); err != nil { return err } - if err := json.NewDecoder(r.Body).Decode(&create); err != nil { - 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 { return nameConflict(create.Name) } @@ -255,22 +245,15 @@ func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWr } func (n *networkRouter) postNetworkConnect(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - var connect types.NetworkConnect if err := httputils.ParseForm(r); err != nil { return err } - if err := httputils.CheckForJSON(r); err != nil { + var connect types.NetworkConnect + if err := httputils.ReadJSON(r, &connect); err != nil { return err } - if err := json.NewDecoder(r.Body).Decode(&connect); err != nil { - 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. // The reason is that, In case of attachable network in swarm scope, the actual local network // may not be available at the time. At the same time, inside daemon `ConnectContainerToNetwork` @@ -279,22 +262,15 @@ func (n *networkRouter) postNetworkConnect(ctx context.Context, w http.ResponseW } func (n *networkRouter) postNetworkDisconnect(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - var disconnect types.NetworkDisconnect if err := httputils.ParseForm(r); err != nil { return err } - if err := httputils.CheckForJSON(r); err != nil { + var disconnect types.NetworkDisconnect + if err := httputils.ReadJSON(r, &disconnect); err != nil { return err } - if err := json.NewDecoder(r.Body).Decode(&disconnect); err != nil { - 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) } diff --git a/api/server/router/plugin/plugin_routes.go b/api/server/router/plugin/plugin_routes.go index 8c36b51ecc..c82996679d 100644 --- a/api/server/router/plugin/plugin_routes.go +++ b/api/server/router/plugin/plugin_routes.go @@ -4,7 +4,6 @@ import ( "context" "encoding/base64" "encoding/json" - "io" "net/http" "strconv" "strings" @@ -13,7 +12,6 @@ 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" @@ -96,12 +94,8 @@ func (pr *pluginRouter) upgradePlugin(ctx context.Context, w http.ResponseWriter } var privileges types.PluginPrivileges - dec := json.NewDecoder(r.Body) - if err := dec.Decode(&privileges); err != nil { - return errors.Wrap(err, "failed to parse privileges") - } - if dec.More() { - return errors.New("invalid privileges") + if err := httputils.ReadJSON(r, &privileges); err != nil { + return err } metaHeaders, authConfig := parseHeaders(r.Header) @@ -135,12 +129,8 @@ func (pr *pluginRouter) pullPlugin(ctx context.Context, w http.ResponseWriter, r } var privileges types.PluginPrivileges - dec := json.NewDecoder(r.Body) - if err := dec.Decode(&privileges); err != nil { - return errors.Wrap(err, "failed to parse privileges") - } - if dec.More() { - return errors.New("invalid privileges") + if err := httputils.ReadJSON(r, &privileges); err != nil { + return err } metaHeaders, authConfig := parseHeaders(r.Header) @@ -277,11 +267,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &args); err != nil { + return err } if err := pr.backend.Set(vars["name"], args); err != nil { return err diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go index 0751617450..05c10082a1 100644 --- a/api/server/router/swarm/cluster_routes.go +++ b/api/server/router/swarm/cluster_routes.go @@ -2,9 +2,7 @@ package swarm // import "github.com/docker/docker/api/server/router/swarm" import ( "context" - "encoding/json" "fmt" - "io" "net/http" "strconv" @@ -21,11 +19,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &req); err != nil { + return err } version := httputils.VersionFromContext(ctx) @@ -48,11 +43,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &req); err != nil { + return err } return sr.backend.Join(req) } @@ -78,11 +70,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &swarm); err != nil { + return err } rawVersion := r.URL.Query().Get("version") @@ -132,11 +121,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &req); err != nil { + return err } if err := sr.backend.UnlockSwarm(req); err != nil { @@ -216,11 +202,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &service); err != nil { + return err } // Get returns "" if the header does not exist @@ -243,11 +226,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &service); err != nil { + return err } rawVersion := r.URL.Query().Get("version") @@ -341,11 +321,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &node); err != nil { + return err } rawVersion := r.URL.Query().Get("version") @@ -423,11 +400,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &secret); err != nil { + return err } version := httputils.VersionFromContext(ctx) if secret.Templating != nil && versions.LessThan(version, "1.37") { @@ -464,11 +438,8 @@ 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) + if err := httputils.ReadJSON(r, &secret); err != nil { + return err } rawVersion := r.URL.Query().Get("version") @@ -500,11 +471,8 @@ 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 { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &config); err != nil { + return err } version := httputils.VersionFromContext(ctx) @@ -542,11 +510,8 @@ 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) + if err := httputils.ReadJSON(r, &config); err != nil { + return err } rawVersion := r.URL.Query().Get("version") diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index e08e724579..195bc8b409 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -2,14 +2,11 @@ package volume // import "github.com/docker/docker/api/server/router/volume" import ( "context" - "encoding/json" - "io" "net/http" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types/filters" volumetypes "github.com/docker/docker/api/types/volume" - "github.com/docker/docker/errdefs" "github.com/docker/docker/volume/service/opts" "github.com/pkg/errors" ) @@ -47,16 +44,9 @@ func (v *volumeRouter) postVolumesCreate(ctx context.Context, w http.ResponseWri return err } - if err := httputils.CheckForJSON(r); err != nil { - return err - } - var req volumetypes.VolumeCreateBody - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - if err == io.EOF { - return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) - } - return errdefs.InvalidParameter(err) + if err := httputils.ReadJSON(r, &req); err != nil { + return err } volume, err := v.backend.Create(ctx, req.Name, req.Driver, opts.WithCreateOptions(req.DriverOpts), opts.WithCreateLabels(req.Labels)) diff --git a/integration-cli/docker_api_exec_test.go b/integration-cli/docker_api_exec_test.go index 3bf5d003f8..5d4dd2d2ab 100644 --- a/integration-cli/docker_api_exec_test.go +++ b/integration-cli/docker_api_exec_test.go @@ -18,6 +18,7 @@ import ( "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/testutil/request" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/poll" ) @@ -56,7 +57,7 @@ func (s *DockerSuite) TestExecAPICreateNoValidContentType(c *testing.T) { } b, err := request.ReadBody(body) assert.NilError(c, err) - assert.Assert(c, strings.Contains(getErrorMessage(c, b), "Content-Type specified"), "Expected message when creating exec command with invalid Content-Type specified") + assert.Assert(c, is.Contains(getErrorMessage(c, b), "unsupported Content-Type header (test/plain): must be 'application/json'")) } func (s *DockerSuite) TestExecAPICreateContainerPaused(c *testing.T) { diff --git a/integration/container/container_test.go b/integration/container/container_test.go index 0402828a77..06dce0f776 100644 --- a/integration/container/container_test.go +++ b/integration/container/container_test.go @@ -2,6 +2,7 @@ package container // import "github.com/docker/docker/integration/container" import ( "net/http" + "runtime" "testing" "github.com/docker/docker/testutil/request" @@ -9,34 +10,69 @@ import ( is "gotest.tools/v3/assert/cmp" ) +// TestContainerInvalidJSON tests that POST endpoints that expect a body return +// the correct error when sending invalid JSON requests. func TestContainerInvalidJSON(t *testing.T) { defer setupTest(t)() + // POST endpoints that accept / expect a JSON body; endpoints := []string{ "/containers/foobar/exec", + "/containers/foobar/update", "/exec/foobar/start", } + // windows doesnt support API < v1.24 + 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) + ) + } + for _, ep := range endpoints { ep := ep - t.Run(ep, func(t *testing.T) { + t.Run(ep[1:], 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) + t.Run("invalid content type", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{}"), request.ContentType("text/plain")) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unsupported Content-Type header (text/plain): must be 'application/json'")) + }) - res, body, err = request.Post(ep, request.JSON) - assert.NilError(t, err) - assert.Equal(t, res.StatusCode, http.StatusBadRequest) + t.Run("invalid JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "invalid JSON: invalid character 'i' looking for beginning of object key string")) + }) + + t.Run("extra content after JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString(`{} trailing content`), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(res.StatusCode, http.StatusBadRequest)) + + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unexpected content after JSON")) + }) + + t.Run("empty body", func(t *testing.T) { + // empty body should not produce an 500 internal server error, or + // any 5XX error (this is assuming the request does not produce + // an internal server error for another reason, but it shouldn't) + res, _, err := request.Post(ep, request.RawString(``), request.JSON) + assert.NilError(t, err) + assert.Check(t, res.StatusCode < http.StatusInternalServerError) + }) }) } } diff --git a/integration/network/network_test.go b/integration/network/network_test.go index ec0546aa0e..00603094a2 100644 --- a/integration/network/network_test.go +++ b/integration/network/network_test.go @@ -62,9 +62,12 @@ func TestRunContainerWithBridgeNone(t *testing.T) { assert.Check(t, is.Equal(stdout.String(), result.Combined()), "The network namespace of container should be the same with host when --net=host and bridge network is disabled") } +// TestNetworkInvalidJSON tests that POST endpoints that expect a body return +// the correct error when sending invalid JSON requests. func TestNetworkInvalidJSON(t *testing.T) { defer setupTest(t)() + // POST endpoints that accept / expect a JSON body; endpoints := []string{ "/networks/create", "/networks/bridge/connect", @@ -73,24 +76,47 @@ func TestNetworkInvalidJSON(t *testing.T) { for _, ep := range endpoints { ep := ep - t.Run(ep, func(t *testing.T) { + t.Run(ep[1:], 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) + t.Run("invalid content type", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{}"), request.ContentType("text/plain")) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unsupported Content-Type header (text/plain): must be 'application/json'")) + }) - res, body, err = request.Post(ep, request.JSON) - assert.NilError(t, err) - assert.Equal(t, res.StatusCode, http.StatusBadRequest) + t.Run("invalid JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "invalid JSON: invalid character 'i' looking for beginning of object key string")) + }) + + t.Run("extra content after JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString(`{} trailing content`), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(res.StatusCode, http.StatusBadRequest)) + + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unexpected content after JSON")) + }) + + t.Run("empty body", func(t *testing.T) { + // empty body should not produce an 500 internal server error, or + // any 5XX error (this is assuming the request does not produce + // an internal server error for another reason, but it shouldn't) + res, _, err := request.Post(ep, request.RawString(``), request.JSON) + assert.NilError(t, err) + assert.Check(t, res.StatusCode < http.StatusInternalServerError) + }) }) } } diff --git a/integration/plugin/common/plugin_test.go b/integration/plugin/common/plugin_test.go index fb37a5e174..37ef0b3374 100644 --- a/integration/plugin/common/plugin_test.go +++ b/integration/plugin/common/plugin_test.go @@ -29,31 +29,61 @@ import ( "gotest.tools/v3/skip" ) +// TestPluginInvalidJSON tests that POST endpoints that expect a body return +// the correct error when sending invalid JSON requests. func TestPluginInvalidJSON(t *testing.T) { defer setupTest(t)() - endpoints := []string{"/plugins/foobar/set"} + // POST endpoints that accept / expect a JSON body; + endpoints := []string{ + "/plugins/foobar/set", + "/plugins/foobar/upgrade", + "/plugins/pull", + } for _, ep := range endpoints { ep := ep - t.Run(ep, func(t *testing.T) { + t.Run(ep[1:], 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) + t.Run("invalid content type", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("[]"), request.ContentType("text/plain")) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unsupported Content-Type header (text/plain): must be 'application/json'")) + }) - res, body, err = request.Post(ep, request.JSON) - assert.NilError(t, err) - assert.Equal(t, res.StatusCode, http.StatusBadRequest) + t.Run("invalid JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "invalid JSON: invalid character 'i' looking for beginning of object key string")) + }) + + t.Run("extra content after JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString(`[] trailing content`), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(res.StatusCode, http.StatusBadRequest)) + + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unexpected content after JSON")) + }) + + t.Run("empty body", func(t *testing.T) { + // empty body should not produce an 500 internal server error, or + // any 5XX error (this is assuming the request does not produce + // an internal server error for another reason, but it shouldn't) + res, _, err := request.Post(ep, request.RawString(``), request.JSON) + assert.NilError(t, err) + assert.Check(t, res.StatusCode < http.StatusInternalServerError) + }) }) } } diff --git a/integration/volume/volume_test.go b/integration/volume/volume_test.go index f465e47b05..395c51e1f4 100644 --- a/integration/volume/volume_test.go +++ b/integration/volume/volume_test.go @@ -104,31 +104,57 @@ func TestVolumesInspect(t *testing.T) { assert.Check(t, createdAt.Unix()-now.Unix() < 60, "CreatedAt (%s) exceeds creation time (%s) 60s", createdAt, now) } +// TestVolumesInvalidJSON tests that POST endpoints that expect a body return +// the correct error when sending invalid JSON requests. func TestVolumesInvalidJSON(t *testing.T) { defer setupTest(t)() + // POST endpoints that accept / expect a JSON body; endpoints := []string{"/volumes/create"} for _, ep := range endpoints { ep := ep - t.Run(ep, func(t *testing.T) { + t.Run(ep[1:], 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) + t.Run("invalid content type", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{}"), request.ContentType("text/plain")) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unsupported Content-Type header (text/plain): must be 'application/json'")) + }) - res, body, err = request.Post(ep, request.JSON) - assert.NilError(t, err) - assert.Equal(t, res.StatusCode, http.StatusBadRequest) + t.Run("invalid JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(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")) + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "invalid JSON: invalid character 'i' looking for beginning of object key string")) + }) + + t.Run("extra content after JSON", func(t *testing.T) { + res, body, err := request.Post(ep, request.RawString(`{} trailing content`), request.JSON) + assert.NilError(t, err) + assert.Check(t, is.Equal(res.StatusCode, http.StatusBadRequest)) + + buf, err := request.ReadBody(body) + assert.NilError(t, err) + assert.Check(t, is.Contains(string(buf), "unexpected content after JSON")) + }) + + t.Run("empty body", func(t *testing.T) { + // empty body should not produce an 500 internal server error, or + // any 5XX error (this is assuming the request does not produce + // an internal server error for another reason, but it shouldn't) + res, _, err := request.Post(ep, request.RawString(``), request.JSON) + assert.NilError(t, err) + assert.Check(t, res.StatusCode < http.StatusInternalServerError) + }) }) } }