Browse code

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 <github@gone.nl>

Sebastiaan van Stijn authored on 2018/08/03 23:41:47
Showing 1 changed files
... ...
@@ -4,6 +4,7 @@ import (
4 4
 	"fmt"
5 5
 	"net/http"
6 6
 
7
+	"github.com/docker/distribution/registry/api/errcode"
7 8
 	"github.com/docker/docker/api/types"
8 9
 	"github.com/docker/docker/api/types/versions"
9 10
 	"github.com/docker/docker/errdefs"
... ...
@@ -54,7 +55,10 @@ func GetHTTPErrorStatusCode(err error) int {
54 54
 		if statusCode != http.StatusInternalServerError {
55 55
 			return statusCode
56 56
 		}
57
-
57
+		statusCode = statusCodeFromDistributionError(err)
58
+		if statusCode != http.StatusInternalServerError {
59
+			return statusCode
60
+		}
58 61
 		if e, ok := err.(causer); ok {
59 62
 			return GetHTTPErrorStatusCode(e.Cause())
60 63
 		}
... ...
@@ -129,3 +133,24 @@ func statusCodeFromGRPCError(err error) int {
129 129
 		return http.StatusInternalServerError
130 130
 	}
131 131
 }
132
+
133
+// statusCodeFromDistributionError returns status code according to registry errcode
134
+// code is loosely based on errcode.ServeJSON() in docker/distribution
135
+func statusCodeFromDistributionError(err error) int {
136
+	switch errs := err.(type) {
137
+	case errcode.Errors:
138
+		if len(errs) < 1 {
139
+			return http.StatusInternalServerError
140
+		}
141
+		if _, ok := errs[0].(errcode.ErrorCoder); ok {
142
+			return statusCodeFromDistributionError(errs[0])
143
+		}
144
+	case errcode.ErrorCoder:
145
+		return errs.ErrorCode().Descriptor().HTTPStatusCode
146
+	default:
147
+		if e, ok := err.(causer); ok {
148
+			return statusCodeFromDistributionError(e.Cause())
149
+		}
150
+	}
151
+	return http.StatusInternalServerError
152
+}