From cdcea6f492f8554086726037ca91f213e7101669 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 3 Aug 2018 16:41:47 +0200 Subject: [PATCH] Handle correct status codes for distribution errors This assists to address a regression where distribution errors were not properly handled, resulting in a generic 500 (internal server error) to be returned for `/distribution/name/json` if you weren't authenticated, whereas it should return a 40x (401). This patch attempts to extract the HTTP status-code that was returned by the distribution code, and falls back to returning a 500 status if unable to match. Before this change: curl -v --unix-socket /var/run/docker.sock http://localhost/distribution/name/json * Trying /var/run/docker.sock... * Connected to localhost (/var/run/docker.sock) port 80 (#0) > GET /distribution/name/json HTTP/1.1 > Host: localhost > User-Agent: curl/7.52.1 > Accept: */* > < HTTP/1.1 500 Internal Server Error < Api-Version: 1.37 < Content-Type: application/json < Docker-Experimental: false < Ostype: linux < Server: Docker/dev (linux) < Date: Tue, 03 Jul 2018 15:52:53 GMT < Content-Length: 115 < {"message":"errors:\ndenied: requested access to the resource is denied\nunauthorized: authentication required\n"} * Curl_http_done: called premature == 0 * Connection #0 to host localhost left intact daemon logs: DEBU[2018-07-03T15:52:51.424950601Z] Calling GET /distribution/name/json DEBU[2018-07-03T15:52:53.179895572Z] FIXME: Got an API for which error does not match any expected type!!!: errors: denied: requested access to the resource is denied unauthorized: authentication required error_type=errcode.Errors module=api ERRO[2018-07-03T15:52:53.179942783Z] Handler for GET /distribution/name/json returned error: errors: denied: requested access to the resource is denied unauthorized: authentication required With this patch applied: curl -v --unix-socket /var/run/docker.sock http://localhost/distribution/name/json * Trying /var/run/docker.sock... * Connected to localhost (/var/run/docker.sock) port 80 (#0) > GET /distribution/name/json HTTP/1.1 > Host: localhost > User-Agent: curl/7.52.1 > Accept: */* > < HTTP/1.1 403 Forbidden < Api-Version: 1.38 < Content-Type: application/json < Docker-Experimental: false < Ostype: linux < Server: Docker/dev (linux) < Date: Fri, 03 Aug 2018 14:58:09 GMT < Content-Length: 115 < {"message":"errors:\ndenied: requested access to the resource is denied\nunauthorized: authentication required\n"} * Curl_http_done: called premature == 0 * Connection #0 to host localhost left intact daemon logs: DEBU[2018-08-03T14:58:08.018726228Z] Calling GET /distribution/name/json Signed-off-by: Sebastiaan van Stijn --- api/server/httputils/errors.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/api/server/httputils/errors.go b/api/server/httputils/errors.go index 88e83139d1..2abdce2bcf 100644 --- a/api/server/httputils/errors.go +++ b/api/server/httputils/errors.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/versions" "github.com/docker/docker/errdefs" @@ -54,7 +55,10 @@ func GetHTTPErrorStatusCode(err error) int { if statusCode != http.StatusInternalServerError { return statusCode } - + statusCode = statusCodeFromDistributionError(err) + if statusCode != http.StatusInternalServerError { + return statusCode + } if e, ok := err.(causer); ok { return GetHTTPErrorStatusCode(e.Cause()) } @@ -129,3 +133,24 @@ func statusCodeFromGRPCError(err error) int { return http.StatusInternalServerError } } + +// statusCodeFromDistributionError returns status code according to registry errcode +// code is loosely based on errcode.ServeJSON() in docker/distribution +func statusCodeFromDistributionError(err error) int { + switch errs := err.(type) { + case errcode.Errors: + if len(errs) < 1 { + return http.StatusInternalServerError + } + if _, ok := errs[0].(errcode.ErrorCoder); ok { + return statusCodeFromDistributionError(errs[0]) + } + case errcode.ErrorCoder: + return errs.ErrorCode().Descriptor().HTTPStatusCode + default: + if e, ok := err.(causer); ok { + return statusCodeFromDistributionError(e.Cause()) + } + } + return http.StatusInternalServerError +}