This repository has been archived by the owner on Oct 13, 2023. It is now read-only.
forked from moby/moby
-
Notifications
You must be signed in to change notification settings - Fork 424
[18.09 backport] API: properly handle invalid JSON to return a 400 status #110
Merged
andrewhsu
merged 7 commits into
docker-archive:18.09
from
thaJeztah:18.09_backport_handle_invalid_json
Nov 27, 2018
Merged
[18.09 backport] API: properly handle invalid JSON to return a 400 status #110
andrewhsu
merged 7 commits into
docker-archive:18.09
from
thaJeztah:18.09_backport_handle_invalid_json
Nov 27, 2018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add windows CI entrypoint script. Signed-off-by: John Howard <jhoward@microsoft.com> Signed-off-by: Vincent Demeester <vincent@sbr.pm> Signed-off-by: Daniel Nephin <dnephin@docker.com> Signed-off-by: Vincent Demeester <vincent@sbr.pm> (cherry picked from commit d3cc071) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Salahuddin Khan <salah@docker.com> (cherry picked from commit 4c8b1fd) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Deep Debroy <ddebroy@docker.com> (cherry picked from commit 7d1c1a4) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some improvements in this test; - use the volume-information that's returned by VolumeCreate as "expected" - don't use an explict name for the volume, as it was only used to reference the volume for inspection - improve the test-output on failure, so that "expected" and "actual" values are printed Without this patch applied; === RUN TestVolumesInspect --- FAIL: TestVolumesInspect (0.02s) volume_test.go:108: assertion failed: false (bool) != true (true bool): Time Volume is CreatedAt not equal to current time FAIL With this patch applied; === RUN TestVolumesInspect --- FAIL: TestVolumesInspect (0.02s) volume_test.go:95: assertion failed: expression is false: createdAt.Truncate(time.Minute).Equal(now.Truncate(time.Minute)): CreatedAt (2018-11-01 16:15:20 +0000 UTC) not equal to creation time (2018-11-01 16:15:20.2421166 +0000 UTC m=+13.733512701) FAIL Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit 8e8cac8) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit 05e1842) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These tests don't seem to have anything Linux-specific, so enable them on Windows Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit b334198) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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> (cherry picked from commit c7b488f) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test is really flaky; https://jenkins.dockerproject.org/job/Docker-PRs-s390x/11948/console tracked through moby#32673, and being investigated in moby#37833
|
ping @kolyshkin @cpuguy83 PTAL |
andrewhsu
approved these changes
Nov 27, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of moby#38141 for 18.09
This PR depends on #109 (which on itself depends on #80) for a clean cherry-pick, so this was built on top of that
cherry-pick was clean; no conflicts
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;
Empty request:
After this change, a 400 is returned;
Empty request:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)