diff --git a/registry/client/auth/session.go b/registry/client/auth/session.go index 6c92fc343..9819b3cb8 100644 --- a/registry/client/auth/session.go +++ b/registry/client/auth/session.go @@ -240,7 +240,8 @@ func (th *tokenHandler) fetchToken(params map[string]string) (token *tokenRespon defer resp.Body.Close() if !client.SuccessStatus(resp.StatusCode) { - return nil, fmt.Errorf("token auth attempt for registry: %s request failed with status: %d %s", req.URL, resp.StatusCode, http.StatusText(resp.StatusCode)) + err := client.HandleErrorResponse(resp) + return nil, err } decoder := json.NewDecoder(resp.Body) diff --git a/registry/client/blob_writer.go b/registry/client/blob_writer.go index c7eee4e8c..21a018dc3 100644 --- a/registry/client/blob_writer.go +++ b/registry/client/blob_writer.go @@ -33,7 +33,7 @@ func (hbu *httpBlobUpload) handleErrorResponse(resp *http.Response) error { if resp.StatusCode == http.StatusNotFound { return distribution.ErrBlobUploadUnknown } - return handleErrorResponse(resp) + return HandleErrorResponse(resp) } func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) { diff --git a/registry/client/errors.go b/registry/client/errors.go index 7305c021c..8e3cb1084 100644 --- a/registry/client/errors.go +++ b/registry/client/errors.go @@ -47,7 +47,11 @@ func parseHTTPErrorResponse(r io.Reader) error { return errors } -func handleErrorResponse(resp *http.Response) error { +// HandleErrorResponse returns error parsed from HTTP response for an +// unsuccessful HTTP response code (in the range 400 - 499 inclusive). An +// UnexpectedHTTPStatusError returned for response code outside of expected +// range. +func HandleErrorResponse(resp *http.Response) error { if resp.StatusCode == 401 { err := parseHTTPErrorResponse(resp.Body) if uErr, ok := err.(*UnexpectedHTTPResponseError); ok { diff --git a/registry/client/errors_test.go b/registry/client/errors_test.go new file mode 100644 index 000000000..80241a5a4 --- /dev/null +++ b/registry/client/errors_test.go @@ -0,0 +1,89 @@ +package client + +import ( + "bytes" + "io" + "net/http" + "strings" + "testing" +) + +type nopCloser struct { + io.Reader +} + +func (nopCloser) Close() error { return nil } + +func TestHandleErrorResponse401ValidBody(t *testing.T) { + json := "{\"errors\":[{\"code\":\"UNAUTHORIZED\",\"message\":\"action requires authentication\"}]}" + response := &http.Response{ + Status: "401 Unauthorized", + StatusCode: 401, + Body: nopCloser{bytes.NewBufferString(json)}, + } + err := HandleErrorResponse(response) + + expectedMsg := "unauthorized: action requires authentication" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) + } +} + +func TestHandleErrorResponse401WithInvalidBody(t *testing.T) { + json := "{invalid json}" + response := &http.Response{ + Status: "401 Unauthorized", + StatusCode: 401, + Body: nopCloser{bytes.NewBufferString(json)}, + } + err := HandleErrorResponse(response) + + expectedMsg := "unauthorized: authentication required" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) + } +} + +func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) { + json := "{\"errors\":[{\"code\":\"DIGEST_INVALID\",\"message\":\"provided digest does not match\"}]}" + response := &http.Response{ + Status: "400 Bad Request", + StatusCode: 400, + Body: nopCloser{bytes.NewBufferString(json)}, + } + err := HandleErrorResponse(response) + + expectedMsg := "digest invalid: provided digest does not match" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) + } +} + +func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) { + json := "{invalid json}" + response := &http.Response{ + Status: "404 Not Found", + StatusCode: 404, + Body: nopCloser{bytes.NewBufferString(json)}, + } + err := HandleErrorResponse(response) + + expectedMsg := "Error parsing HTTP response: invalid character 'i' looking for beginning of object key string: \"{invalid json}\"" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) + } +} + +func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) { + response := &http.Response{ + Status: "501 Not Implemented", + StatusCode: 501, + Body: nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")}, + } + err := HandleErrorResponse(response) + + expectedMsg := "Received unexpected HTTP status: 501 Not Implemented" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) + } +} diff --git a/registry/client/repository.go b/registry/client/repository.go index 9d489dd58..758c6e5e3 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -91,7 +91,7 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri returnErr = io.EOF } } else { - return 0, handleErrorResponse(resp) + return 0, HandleErrorResponse(resp) } return numFilled, returnErr @@ -203,7 +203,7 @@ func (t *tags) All(ctx context.Context) ([]string, error) { tags = tagsResponse.Tags return tags, nil } - return tags, handleErrorResponse(resp) + return tags, HandleErrorResponse(resp) } func descriptorFromResponse(response *http.Response) (distribution.Descriptor, error) { @@ -276,7 +276,7 @@ check: } goto check default: - return distribution.Descriptor{}, handleErrorResponse(resp) + return distribution.Descriptor{}, HandleErrorResponse(resp) } } @@ -315,7 +315,7 @@ func (ms *manifests) Exists(ctx context.Context, dgst digest.Digest) (bool, erro } else if resp.StatusCode == http.StatusNotFound { return false, nil } - return false, handleErrorResponse(resp) + return false, HandleErrorResponse(resp) } // AddEtagToTag allows a client to supply an eTag to Get which will be @@ -395,7 +395,7 @@ func (ms *manifests) Get(ctx context.Context, dgst digest.Digest, options ...dis } return m, nil } - return nil, handleErrorResponse(resp) + return nil, HandleErrorResponse(resp) } // WithTag allows a tag to be passed into Put which enables the client @@ -462,7 +462,7 @@ func (ms *manifests) Put(ctx context.Context, m distribution.Manifest, options . return dgst, nil } - return "", handleErrorResponse(resp) + return "", HandleErrorResponse(resp) } func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error { @@ -484,7 +484,7 @@ func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error { if SuccessStatus(resp.StatusCode) { return nil } - return handleErrorResponse(resp) + return HandleErrorResponse(resp) } // todo(richardscothern): Restore interface and implementation with merge of #1050 @@ -541,7 +541,7 @@ func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.Rea if resp.StatusCode == http.StatusNotFound { return distribution.ErrBlobUnknown } - return handleErrorResponse(resp) + return HandleErrorResponse(resp) }), nil } @@ -597,7 +597,7 @@ func (bs *blobs) Create(ctx context.Context) (distribution.BlobWriter, error) { location: location, }, nil } - return nil, handleErrorResponse(resp) + return nil, HandleErrorResponse(resp) } func (bs *blobs) Resume(ctx context.Context, id string) (distribution.BlobWriter, error) { @@ -645,7 +645,7 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi } else if resp.StatusCode == http.StatusNotFound { return distribution.Descriptor{}, distribution.ErrBlobUnknown } - return distribution.Descriptor{}, handleErrorResponse(resp) + return distribution.Descriptor{}, HandleErrorResponse(resp) } func buildCatalogValues(maxEntries int, last string) url.Values { @@ -682,7 +682,7 @@ func (bs *blobStatter) Clear(ctx context.Context, dgst digest.Digest) error { if SuccessStatus(resp.StatusCode) { return nil } - return handleErrorResponse(resp) + return HandleErrorResponse(resp) } func (bs *blobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error {