API: properly handle invalid JSON to return a 400 status

The API did not treat invalid JSON payloads as a 400 error, as a result
returning a 500 error;

Before this change, an invalid JSON body would return a 500 error;

```bash
curl -v \
  --unix-socket /var/run/docker.sock \
  -X POST \
  "http://localhost/v1.30/networks/create" \
  -H "Content-Type: application/json" \
  -d '{invalid json'
```

```
> POST /v1.30/networks/create HTTP/1.1
> Host: localhost
> User-Agent: curl/7.52.1
> Accept: */*
> Content-Type: application/json
> Content-Length: 13
>
* upload completely sent off: 13 out of 13 bytes
< HTTP/1.1 500 Internal Server Error
< Api-Version: 1.40
< Content-Type: application/json
< Docker-Experimental: false
< Ostype: linux
< Server: Docker/dev (linux)
< Date: Mon, 05 Nov 2018 11:55:20 GMT
< Content-Length: 79
<
{"message":"invalid character 'i' looking for beginning of object key string"}
```

Empty request:

```bash
curl -v \
  --unix-socket /var/run/docker.sock \
  -X POST \
  "http://localhost/v1.30/networks/create" \
  -H "Content-Type: application/json"
```

```
> POST /v1.30/networks/create HTTP/1.1
> Host: localhost
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Type: application/json
>
< HTTP/1.1 500 Internal Server Error
< Api-Version: 1.38
< Content-Length: 18
< Content-Type: application/json
< Date: Mon, 05 Nov 2018 12:00:18 GMT
< Docker-Experimental: true
< Ostype: linux
< Server: Docker/18.06.1-ce (linux)
<
{"message":"EOF"}
```

After this change, a 400 is returned;

```bash
curl -v \
  --unix-socket /var/run/docker.sock \
  -X POST \
  "http://localhost/v1.30/networks/create" \
  -H "Content-Type: application/json" \
  -d '{invalid json'
```

```
> POST /v1.30/networks/create HTTP/1.1
> Host: localhost
> User-Agent: curl/7.52.1
> Accept: */*
> Content-Type: application/json
> Content-Length: 13
>
* upload completely sent off: 13 out of 13 bytes
< HTTP/1.1 400 Bad Request
< Api-Version: 1.40
< Content-Type: application/json
< Docker-Experimental: false
< Ostype: linux
< Server: Docker/dev (linux)
< Date: Mon, 05 Nov 2018 11:57:15 GMT
< Content-Length: 79
<
{"message":"invalid character 'i' looking for beginning of object key string"}
```

Empty request:

```bash
curl -v \
  --unix-socket /var/run/docker.sock \
  -X POST \
  "http://localhost/v1.30/networks/create" \
  -H "Content-Type: application/json"
```

```
> POST /v1.30/networks/create HTTP/1.1
> Host: localhost
> User-Agent: curl/7.52.1
> Accept: */*
> Content-Type: application/json
>
< HTTP/1.1 400 Bad Request
< Api-Version: 1.40
< Content-Type: application/json
< Docker-Experimental: false
< Ostype: linux
< Server: Docker/dev (linux)
< Date: Mon, 05 Nov 2018 11:59:22 GMT
< Content-Length: 49
<
{"message":"got EOF while reading request body"}
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2018-11-05 14:50:33 +01:00
parent 2f902930e9
commit c7b488fbc8
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
11 changed files with 248 additions and 17 deletions

View File

@ -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 == "" {

View File

@ -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 {

View File

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

View File

@ -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

View File

@ -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)
}
nodeID, err := sr.backend.Init(req)
if err != nil {
@ -34,7 +38,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)
}
@ -61,7 +68,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")
@ -112,7 +122,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 {
@ -175,7 +188,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
@ -207,7 +223,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")
@ -309,7 +328,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")
@ -388,7 +410,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") {
@ -426,6 +451,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)
}
@ -459,7 +487,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)
@ -498,6 +529,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)
}

View File

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

View File

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

View File

@ -3,6 +3,7 @@ package network // import "github.com/docker/docker/integration/network"
import (
"bytes"
"context"
"net/http"
"os/exec"
"strings"
"testing"
@ -10,6 +11,7 @@ import (
"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"))
})
}
}

View File

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

View File

@ -0,0 +1,38 @@
package common // import "github.com/docker/docker/integration/plugin/common"
import (
"net/http"
"testing"
"github.com/docker/docker/internal/test/request"
"gotest.tools/assert"
is "gotest.tools/assert/cmp"
)
func TestPluginInvalidJSON(t *testing.T) {
defer setupTest(t)()
endpoints := []string{"/plugins/foobar/set"}
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"))
})
}
}

View File

@ -2,6 +2,7 @@ package volume
import (
"context"
"net/http"
"path/filepath"
"strings"
"testing"
@ -95,6 +96,34 @@ func TestVolumesInspect(t *testing.T) {
assert.Check(t, createdAt.Truncate(time.Minute).Equal(now.Truncate(time.Minute)), "CreatedAt (%s) not equal to creation time (%s)", createdAt, now)
}
func TestVolumesInvalidJSON(t *testing.T) {
defer setupTest(t)()
endpoints := []string{"/volumes/create"}
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"))
})
}
}
func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) {
if testEnv.OSType == "windows" {
return "c:", `\`