Skip to content

Commit c7b488f

Browse files
thaJeztahthaJeztah
committed
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>
1 parent 2f90293 commit c7b488f

File tree

11 files changed

+248
-17
lines changed

11 files changed

+248
-17
lines changed

‎api/server/router/container/copy.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import (
66
"context"
77
"encoding/base64"
88
"encoding/json"
9+
"errors"
910
"io"
1011
"net/http"
1112

1213
"github.com/docker/docker/api/server/httputils"
1314
"github.com/docker/docker/api/types"
1415
"github.com/docker/docker/api/types/versions"
16+
"github.com/docker/docker/errdefs"
1517
gddohttputil "github.com/golang/gddo/httputil"
1618
)
1719

@@ -37,7 +39,10 @@ func (s *containerRouter) postContainersCopy(ctx context.Context, w http.Respons
3739

3840
cfg := types.CopyConfig{}
3941
if err := json.NewDecoder(r.Body).Decode(&cfg); err != nil {
40-
return err
42+
if err == io.EOF {
43+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
44+
}
45+
return errdefs.InvalidParameter(err)
4146
}
4247

4348
if cfg.Resource == "" {

‎api/server/router/container/exec.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package container // import "github.com/docker/docker/api/server/router/containe
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"io"
89
"net/http"
@@ -44,7 +45,10 @@ func (s *containerRouter) postContainerExecCreate(ctx context.Context, w http.Re
4445

4546
execConfig := &types.ExecConfig{}
4647
if err := json.NewDecoder(r.Body).Decode(execConfig); err != nil {
47-
return err
48+
if err == io.EOF {
49+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
50+
}
51+
return errdefs.InvalidParameter(err)
4852
}
4953

5054
if len(execConfig.Cmd) == 0 {
@@ -84,7 +88,10 @@ func (s *containerRouter) postContainerExecStart(ctx context.Context, w http.Res
8488

8589
execStartCheck := &types.ExecStartCheck{}
8690
if err := json.NewDecoder(r.Body).Decode(execStartCheck); err != nil {
87-
return err
91+
if err == io.EOF {
92+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
93+
}
94+
return errdefs.InvalidParameter(err)
8895
}
8996

9097
if exists, err := s.backend.ExecExists(execName); !exists {

‎api/server/router/network/network_routes.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package network // import "github.com/docker/docker/api/server/router/network"
33
import (
44
"context"
55
"encoding/json"
6+
"io"
67
"net/http"
78
"strconv"
89
"strings"
@@ -215,7 +216,10 @@ func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWr
215216
}
216217

217218
if err := json.NewDecoder(r.Body).Decode(&create); err != nil {
218-
return err
219+
if err == io.EOF {
220+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
221+
}
222+
return errdefs.InvalidParameter(err)
219223
}
220224

221225
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
261265
}
262266

263267
if err := json.NewDecoder(r.Body).Decode(&connect); err != nil {
264-
return err
268+
if err == io.EOF {
269+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
270+
}
271+
return errdefs.InvalidParameter(err)
265272
}
266273

267274
// 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
282289
}
283290

284291
if err := json.NewDecoder(r.Body).Decode(&disconnect); err != nil {
285-
return err
292+
if err == io.EOF {
293+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
294+
}
295+
return errdefs.InvalidParameter(err)
286296
}
287297

288298
return n.backend.DisconnectContainerFromNetwork(disconnect.Container, vars["id"], disconnect.Force)

‎api/server/router/plugin/plugin_routes.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/base64"
66
"encoding/json"
7+
"io"
78
"net/http"
89
"strconv"
910
"strings"
@@ -12,6 +13,7 @@ import (
1213
"github.com/docker/docker/api/server/httputils"
1314
"github.com/docker/docker/api/types"
1415
"github.com/docker/docker/api/types/filters"
16+
"github.com/docker/docker/errdefs"
1517
"github.com/docker/docker/pkg/ioutils"
1618
"github.com/docker/docker/pkg/streamformatter"
1719
"github.com/pkg/errors"
@@ -276,7 +278,10 @@ func (pr *pluginRouter) pushPlugin(ctx context.Context, w http.ResponseWriter, r
276278
func (pr *pluginRouter) setPlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
277279
var args []string
278280
if err := json.NewDecoder(r.Body).Decode(&args); err != nil {
279-
return err
281+
if err == io.EOF {
282+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
283+
}
284+
return errdefs.InvalidParameter(err)
280285
}
281286
if err := pr.backend.Set(vars["name"], args); err != nil {
282287
return err

‎api/server/router/swarm/cluster_routes.go‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"io"
78
"net/http"
89
"strconv"
910

@@ -21,7 +22,10 @@ import (
2122
func (sr *swarmRouter) initCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
2223
var req types.InitRequest
2324
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
24-
return err
25+
if err == io.EOF {
26+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
27+
}
28+
return errdefs.InvalidParameter(err)
2529
}
2630
nodeID, err := sr.backend.Init(req)
2731
if err != nil {
@@ -34,7 +38,10 @@ func (sr *swarmRouter) initCluster(ctx context.Context, w http.ResponseWriter, r
3438
func (sr *swarmRouter) joinCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
3539
var req types.JoinRequest
3640
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
37-
return err
41+
if err == io.EOF {
42+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
43+
}
44+
return errdefs.InvalidParameter(err)
3845
}
3946
return sr.backend.Join(req)
4047
}
@@ -61,7 +68,10 @@ func (sr *swarmRouter) inspectCluster(ctx context.Context, w http.ResponseWriter
6168
func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
6269
var swarm types.Spec
6370
if err := json.NewDecoder(r.Body).Decode(&swarm); err != nil {
64-
return err
71+
if err == io.EOF {
72+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
73+
}
74+
return errdefs.InvalidParameter(err)
6575
}
6676

6777
rawVersion := r.URL.Query().Get("version")
@@ -112,7 +122,10 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter,
112122
func (sr *swarmRouter) unlockCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
113123
var req types.UnlockRequest
114124
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
115-
return err
125+
if err == io.EOF {
126+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
127+
}
128+
return errdefs.InvalidParameter(err)
116129
}
117130

118131
if err := sr.backend.UnlockSwarm(req); err != nil {
@@ -175,7 +188,10 @@ func (sr *swarmRouter) getService(ctx context.Context, w http.ResponseWriter, r
175188
func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
176189
var service types.ServiceSpec
177190
if err := json.NewDecoder(r.Body).Decode(&service); err != nil {
178-
return err
191+
if err == io.EOF {
192+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
193+
}
194+
return errdefs.InvalidParameter(err)
179195
}
180196

181197
// Get returns "" if the header does not exist
@@ -207,7 +223,10 @@ func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter,
207223
func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
208224
var service types.ServiceSpec
209225
if err := json.NewDecoder(r.Body).Decode(&service); err != nil {
210-
return err
226+
if err == io.EOF {
227+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
228+
}
229+
return errdefs.InvalidParameter(err)
211230
}
212231

213232
rawVersion := r.URL.Query().Get("version")
@@ -309,7 +328,10 @@ func (sr *swarmRouter) getNode(ctx context.Context, w http.ResponseWriter, r *ht
309328
func (sr *swarmRouter) updateNode(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
310329
var node types.NodeSpec
311330
if err := json.NewDecoder(r.Body).Decode(&node); err != nil {
312-
return err
331+
if err == io.EOF {
332+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
333+
}
334+
return errdefs.InvalidParameter(err)
313335
}
314336

315337
rawVersion := r.URL.Query().Get("version")
@@ -388,7 +410,10 @@ func (sr *swarmRouter) getSecrets(ctx context.Context, w http.ResponseWriter, r
388410
func (sr *swarmRouter) createSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
389411
var secret types.SecretSpec
390412
if err := json.NewDecoder(r.Body).Decode(&secret); err != nil {
391-
return err
413+
if err == io.EOF {
414+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
415+
}
416+
return errdefs.InvalidParameter(err)
392417
}
393418
version := httputils.VersionFromContext(ctx)
394419
if secret.Templating != nil && versions.LessThan(version, "1.37") {
@@ -426,6 +451,9 @@ func (sr *swarmRouter) getSecret(ctx context.Context, w http.ResponseWriter, r *
426451
func (sr *swarmRouter) updateSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
427452
var secret types.SecretSpec
428453
if err := json.NewDecoder(r.Body).Decode(&secret); err != nil {
454+
if err == io.EOF {
455+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
456+
}
429457
return errdefs.InvalidParameter(err)
430458
}
431459

@@ -459,7 +487,10 @@ func (sr *swarmRouter) getConfigs(ctx context.Context, w http.ResponseWriter, r
459487
func (sr *swarmRouter) createConfig(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
460488
var config types.ConfigSpec
461489
if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
462-
return err
490+
if err == io.EOF {
491+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
492+
}
493+
return errdefs.InvalidParameter(err)
463494
}
464495

465496
version := httputils.VersionFromContext(ctx)
@@ -498,6 +529,9 @@ func (sr *swarmRouter) getConfig(ctx context.Context, w http.ResponseWriter, r *
498529
func (sr *swarmRouter) updateConfig(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
499530
var config types.ConfigSpec
500531
if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
532+
if err == io.EOF {
533+
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
534+
}
501535
return errdefs.InvalidParameter(err)
502536
}
503537

‎api/server/router/volume/volume_routes.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (v *volumeRouter) postVolumesCreate(ctx context.Context, w http.ResponseWri
5656
if err == io.EOF {
5757
return errdefs.InvalidParameter(errors.New("got EOF while reading request body"))
5858
}
59-
return err
59+
return errdefs.InvalidParameter(err)
6060
}
6161

6262
volume, err := v.backend.Create(ctx, req.Name, req.Driver, opts.WithCreateOptions(req.DriverOpts), opts.WithCreateLabels(req.Labels))

‎integration/container/container_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package container // import "github.com/docker/docker/integration/container"
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
7+
"github.com/docker/docker/internal/test/request"
8+
"gotest.tools/assert"
9+
is "gotest.tools/assert/cmp"
10+
)
11+
12+
func TestContainerInvalidJSON(t *testing.T) {
13+
defer setupTest(t)()
14+
15+
endpoints := []string{
16+
"/containers/foobar/copy",
17+
"/containers/foobar/exec",
18+
"/exec/foobar/start",
19+
}
20+
21+
for _, ep := range endpoints {
22+
t.Run(ep, func(t *testing.T) {
23+
t.Parallel()
24+
25+
res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON)
26+
assert.NilError(t, err)
27+
assert.Equal(t, res.StatusCode, http.StatusBadRequest)
28+
29+
buf, err := request.ReadBody(body)
30+
assert.NilError(t, err)
31+
assert.Check(t, is.Contains(string(buf), "invalid character 'i' looking for beginning of object key string"))
32+
33+
res, body, err = request.Post(ep, request.JSON)
34+
assert.NilError(t, err)
35+
assert.Equal(t, res.StatusCode, http.StatusBadRequest)
36+
37+
buf, err = request.ReadBody(body)
38+
assert.NilError(t, err)
39+
assert.Check(t, is.Contains(string(buf), "got EOF while reading request body"))
40+
})
41+
}
42+
}

‎integration/network/network_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ package network // import "github.com/docker/docker/integration/network"
33
import (
44
"bytes"
55
"context"
6+
"net/http"
67
"os/exec"
78
"strings"
89
"testing"
910

1011
"github.com/docker/docker/api/types"
1112
"github.com/docker/docker/integration/internal/container"
1213
"github.com/docker/docker/internal/test/daemon"
14+
"github.com/docker/docker/internal/test/request"
1315
"gotest.tools/assert"
1416
is "gotest.tools/assert/cmp"
1517
"gotest.tools/skip"
@@ -56,3 +58,35 @@ func TestRunContainerWithBridgeNone(t *testing.T) {
5658
assert.NilError(t, err)
5759
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")
5860
}
61+
62+
func TestNetworkInvalidJSON(t *testing.T) {
63+
defer setupTest(t)()
64+
65+
endpoints := []string{
66+
"/networks/create",
67+
"/networks/bridge/connect",
68+
"/networks/bridge/disconnect",
69+
}
70+
71+
for _, ep := range endpoints {
72+
t.Run(ep, func(t *testing.T) {
73+
t.Parallel()
74+
75+
res, body, err := request.Post(ep, request.RawString("{invalid json"), request.JSON)
76+
assert.NilError(t, err)
77+
assert.Equal(t, res.StatusCode, http.StatusBadRequest)
78+
79+
buf, err := request.ReadBody(body)
80+
assert.NilError(t, err)
81+
assert.Check(t, is.Contains(string(buf), "invalid character 'i' looking for beginning of object key string"))
82+
83+
res, body, err = request.Post(ep, request.JSON)
84+
assert.NilError(t, err)
85+
assert.Equal(t, res.StatusCode, http.StatusBadRequest)
86+
87+
buf, err = request.ReadBody(body)
88+
assert.NilError(t, err)
89+
assert.Check(t, is.Contains(string(buf), "got EOF while reading request body"))
90+
})
91+
}
92+
}

‎integration/plugin/common/main_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package common // import "github.com/docker/docker/integration/plugin/common"
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"testing"
7+
8+
"github.com/docker/docker/internal/test/environment"
9+
)
10+
11+
var testEnv *environment.Execution
12+
13+
func TestMain(m *testing.M) {
14+
var err error
15+
testEnv, err = environment.New()
16+
if err != nil {
17+
fmt.Println(err)
18+
os.Exit(1)
19+
}
20+
testEnv.Print()
21+
os.Exit(m.Run())
22+
}
23+
24+
func setupTest(t *testing.T) func() {
25+
environment.ProtectAll(t, testEnv)
26+
return func() { testEnv.Clean(t) }
27+
}

0 commit comments

Comments
 (0)