diff options
author | Sebastiaan van Stijn <github@gone.nl> | 2022-04-05 11:43:06 +0200 |
---|---|---|
committer | Sebastiaan van Stijn <github@gone.nl> | 2022-04-11 21:37:51 +0200 |
commit | 0c9ff0b45a70061d43655672b2e8227e1b51684f (patch) | |
tree | 2ccaa9d7d55e858a9eccdaa204f5f0854316893f /integration/plugin | |
parent | ef490cae4520ff1c8aa765ec9107885417a2583b (diff) | |
download | docker-0c9ff0b45a70061d43655672b2e8227e1b51684f.tar.gz |
api/server/httputils: add ReadJSON() utility
Implement a ReadJSON() utility to help reduce some code-duplication,
and to make sure we handle JSON requests consistently (e.g. always
check for the content-type).
Differences compared to current handling:
- prevent possible panic if request.Body is nil ("should never happen")
- always require Content-Type to be "application/json"
- be stricter about additional content after JSON (previously ignored)
- but, allow the body to be empty (an empty body is not invalid);
update TestContainerInvalidJSON accordingly, which was testing the
wrong expectation.
- close body after reading (some code did this)
We should consider to add a "max body size" on this function, similar to
https://github.com/moby/moby/blob/7b9275c0da707b030e62c96b679a976f31f929d3/api/server/middleware/debug.go#L27-L40
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Diffstat (limited to 'integration/plugin')
-rw-r--r-- | integration/plugin/common/plugin_test.go | 64 |
1 files changed, 47 insertions, 17 deletions
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) - - 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")) + 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), "unsupported Content-Type header (text/plain): must be 'application/json'")) + }) + + 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), "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) + }) }) } } |