diff --git a/registry/client/auth/session.go b/registry/client/auth/session.go index fe212831..b0879fb4 100644 --- a/registry/client/auth/session.go +++ b/registry/client/auth/session.go @@ -360,8 +360,7 @@ func (th *tokenHandler) fetchTokenWithOAuth(ctx context.Context, realm *url.URL, } defer resp.Body.Close() - if !client.SuccessStatus(resp.StatusCode) { - err := client.HandleErrorResponse(resp) + if err := client.HandleHTTPResponseError(resp); err != nil { return "", time.Time{}, err } @@ -443,8 +442,7 @@ func (th *tokenHandler) fetchTokenWithBasicAuth(ctx context.Context, realm *url. } defer resp.Body.Close() - if !client.SuccessStatus(resp.StatusCode) { - err := client.HandleErrorResponse(resp) + if err := client.HandleHTTPResponseError(resp); err != nil { return "", time.Time{}, err } diff --git a/registry/client/blob_writer.go b/registry/client/blob_writer.go index bff0e08b..f85c84f7 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 HandleHTTPResponseError(resp) } func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) { @@ -51,8 +51,8 @@ func (hbu *httpBlobUpload) ReadFrom(r io.Reader) (n int64, err error) { } defer resp.Body.Close() - if !SuccessStatus(resp.StatusCode) { - return 0, hbu.handleErrorResponse(resp) + if err := hbu.handleErrorResponse(resp); err != nil { + return 0, err } hbu.uuid = resp.Header.Get("Docker-Upload-UUID") @@ -87,8 +87,8 @@ func (hbu *httpBlobUpload) Write(p []byte) (n int, err error) { } defer resp.Body.Close() - if !SuccessStatus(resp.StatusCode) { - return 0, hbu.handleErrorResponse(resp) + if err := hbu.handleErrorResponse(resp); err != nil { + return 0, err } hbu.uuid = resp.Header.Get("Docker-Upload-UUID") @@ -137,8 +137,8 @@ func (hbu *httpBlobUpload) Commit(ctx context.Context, desc distribution.Descrip } defer resp.Body.Close() - if !SuccessStatus(resp.StatusCode) { - return distribution.Descriptor{}, hbu.handleErrorResponse(resp) + if err := hbu.handleErrorResponse(resp); err != nil { + return distribution.Descriptor{}, err } return hbu.statter.Stat(ctx, desc.Digest) @@ -155,7 +155,7 @@ func (hbu *httpBlobUpload) Cancel(ctx context.Context) error { } defer resp.Body.Close() - if resp.StatusCode == http.StatusNotFound || SuccessStatus(resp.StatusCode) { + if resp.StatusCode == http.StatusNotFound { return nil } return hbu.handleErrorResponse(resp) diff --git a/registry/client/errors.go b/registry/client/errors.go index 30c76926..9d20b8b7 100644 --- a/registry/client/errors.go +++ b/registry/client/errors.go @@ -116,11 +116,16 @@ func mergeErrors(err1, err2 error) error { return errcode.Errors(append(makeErrorList(err1), makeErrorList(err2)...)) } -// 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 { +// HandleHTTPResponseError returns error parsed from HTTP response, if any. +// It returns nil if no error occurred (HTTP status 200-399), or an error +// for unsuccessful HTTP response codes (in the range 400 - 499 inclusive). +// If possible, it returns a typed error, but an UnexpectedHTTPStatusError +// is returned for response code outside the expected range (HTTP status < 200 +// and > 500). +func HandleHTTPResponseError(resp *http.Response) error { + if resp.StatusCode >= 200 && resp.StatusCode <= 399 { + return nil + } if resp.StatusCode >= 400 && resp.StatusCode < 500 { // Check for OAuth errors within the `WWW-Authenticate` header first // See https://tools.ietf.org/html/rfc6750#section-3 @@ -153,8 +158,23 @@ func HandleErrorResponse(resp *http.Response) error { return &UnexpectedHTTPStatusError{Status: resp.Status} } +// 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. +// +// Deprecated: use [HandleHTTPResponseError] and check the error. +func HandleErrorResponse(resp *http.Response) error { + if resp.StatusCode >= 200 && resp.StatusCode <= 399 { + return &UnexpectedHTTPStatusError{Status: resp.Status} + } + return HandleHTTPResponseError(resp) +} + // SuccessStatus returns true if the argument is a successful HTTP response // code (in the range 200 - 399 inclusive). +// +// Deprecated: use [HandleHTTPResponseError] and check the error. func SuccessStatus(status int) bool { return status >= 200 && status <= 399 } diff --git a/registry/client/errors_test.go b/registry/client/errors_test.go index 135608c7..432e2bcc 100644 --- a/registry/client/errors_test.go +++ b/registry/client/errors_test.go @@ -14,7 +14,18 @@ type nopCloser struct { func (nopCloser) Close() error { return nil } -func TestHandleErrorResponse401ValidBody(t *testing.T) { +func TestHandleHTTPResponseError200ValidBody(t *testing.T) { + response := &http.Response{ + Status: "200 OK", + StatusCode: 200, + } + err := HandleHTTPResponseError(response) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } +} + +func TestHandleHTTPResponseError401ValidBody(t *testing.T) { json := `{"errors":[{"code":"UNAUTHORIZED","message":"action requires authentication"}]}` response := &http.Response{ Status: "401 Unauthorized", @@ -22,7 +33,7 @@ func TestHandleErrorResponse401ValidBody(t *testing.T) { Body: nopCloser{bytes.NewBufferString(json)}, Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := "unauthorized: action requires authentication" if !strings.Contains(err.Error(), expectedMsg) { @@ -30,7 +41,7 @@ func TestHandleErrorResponse401ValidBody(t *testing.T) { } } -func TestHandleErrorResponse401WithInvalidBody(t *testing.T) { +func TestHandleHTTPResponseError401WithInvalidBody(t *testing.T) { json := "{invalid json}" response := &http.Response{ Status: "401 Unauthorized", @@ -38,7 +49,7 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) { Body: nopCloser{bytes.NewBufferString(json)}, Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := "unauthorized: authentication required" if !strings.Contains(err.Error(), expectedMsg) { @@ -46,7 +57,7 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) { } } -func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) { +func TestHandleHTTPResponseErrorExpectedStatusCode400ValidBody(t *testing.T) { json := `{"errors":[{"code":"DIGEST_INVALID","message":"provided digest does not match"}]}` response := &http.Response{ Status: "400 Bad Request", @@ -54,7 +65,7 @@ func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) { Body: nopCloser{bytes.NewBufferString(json)}, Header: http.Header{"Content-Type": []string{"application/json"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := "digest invalid: provided digest does not match" if !strings.Contains(err.Error(), expectedMsg) { @@ -62,7 +73,7 @@ func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) { } } -func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) { +func TestHandleHTTPResponseErrorExpectedStatusCode404EmptyErrorSlice(t *testing.T) { json := `{"randomkey": "randomvalue"}` response := &http.Response{ Status: "404 Not Found", @@ -70,7 +81,7 @@ func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) { Body: nopCloser{bytes.NewBufferString(json)}, Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := `error parsing HTTP 404 response body: no error details found in HTTP response body: "{\"randomkey\": \"randomvalue\"}"` if !strings.Contains(err.Error(), expectedMsg) { @@ -78,7 +89,7 @@ func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) { } } -func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) { +func TestHandleHTTPResponseErrorExpectedStatusCode404InvalidBody(t *testing.T) { json := "{invalid json}" response := &http.Response{ Status: "404 Not Found", @@ -86,7 +97,7 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) { Body: nopCloser{bytes.NewBufferString(json)}, Header: http.Header{"Content-Type": []string{"application/json"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := "error parsing HTTP 404 response body: invalid character 'i' looking for beginning of object key string: \"{invalid json}\"" if !strings.Contains(err.Error(), expectedMsg) { @@ -94,14 +105,14 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) { } } -func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) { +func TestHandleHTTPResponseErrorUnexpectedStatusCode501(t *testing.T) { response := &http.Response{ Status: "501 Not Implemented", StatusCode: 501, Body: nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")}, Header: http.Header{"Content-Type": []string{"application/json"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := "received unexpected HTTP status: 501 Not Implemented" if !strings.Contains(err.Error(), expectedMsg) { @@ -109,7 +120,7 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) { } } -func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) { +func TestHandleHTTPResponseErrorInsufficientPrivileges403(t *testing.T) { json := `{"details":"requesting higher privileges than access token allows"}` response := &http.Response{ Status: "403 Forbidden", @@ -117,7 +128,7 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) { Body: nopCloser{bytes.NewBufferString(json)}, Header: http.Header{"Content-Type": []string{"application/json"}}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) expectedMsg := "denied: requesting higher privileges than access token allows" if !strings.Contains(err.Error(), expectedMsg) { @@ -125,14 +136,14 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) { } } -func TestHandleErrorResponseNonJson(t *testing.T) { +func TestHandleHTTPResponseErrorNonJson(t *testing.T) { msg := `{"details":"requesting higher privileges than access token allows"}` response := &http.Response{ Status: "403 Forbidden", StatusCode: 403, Body: nopCloser{bytes.NewBufferString(msg)}, } - err := HandleErrorResponse(response) + err := HandleHTTPResponseError(response) if !strings.Contains(err.Error(), msg) { t.Errorf("Expected %q, got: %q", msg, err.Error()) diff --git a/registry/client/repository.go b/registry/client/repository.go index 58e763a4..d39af256 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -89,8 +89,6 @@ type registry struct { // of the slice, starting at the value provided in 'last'. The number of entries will be returned along with io.EOF if there // are no more entries func (r *registry) Repositories(ctx context.Context, entries []string, last string) (int, error) { - var numFilled int - var returnErr error values := buildCatalogValues(len(entries), last) u, err := r.ub.BuildCatalogURL(values) @@ -108,28 +106,27 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - var ctlg struct { - Repositories []string `json:"repositories"` - } - decoder := json.NewDecoder(resp.Body) - - if err := decoder.Decode(&ctlg); err != nil { - return 0, err - } - - copy(entries, ctlg.Repositories) - numFilled = len(ctlg.Repositories) - - link := resp.Header.Get("Link") - if link == "" { - returnErr = io.EOF - } - } else { - return 0, HandleErrorResponse(resp) + if err := HandleHTTPResponseError(resp); err != nil { + return 0, err } - return numFilled, returnErr + var ctlg struct { + Repositories []string `json:"repositories"` + } + decoder := json.NewDecoder(resp.Body) + + if err := decoder.Decode(&ctlg); err != nil { + return 0, err + } + + copy(entries, ctlg.Repositories) + numFilled := len(ctlg.Repositories) + + if resp.Header.Get("Link") == "" { + return numFilled, io.EOF + } + + return numFilled, nil } // NewRepository creates a new Repository for the given repository name and base URL. @@ -200,18 +197,17 @@ type tags struct { // All returns all tags func (t *tags) All(ctx context.Context) ([]string, error) { - var tags []string - listURLStr, err := t.ub.BuildTagsURL(t.name) if err != nil { - return tags, err + return nil, err } listURL, err := url.Parse(listURLStr) if err != nil { - return tags, err + return nil, err } + var allTags []string for { req, err := http.NewRequestWithContext(ctx, http.MethodGet, listURL.String(), nil) if err != nil { @@ -219,36 +215,36 @@ func (t *tags) All(ctx context.Context) ([]string, error) { } resp, err := t.client.Do(req) if err != nil { - return tags, err + return allTags, err } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - b, err := io.ReadAll(resp.Body) + if err := HandleHTTPResponseError(resp); err != nil { + return allTags, err + } + + b, err := io.ReadAll(resp.Body) + if err != nil { + return allTags, err + } + + tagsResponse := struct { + Tags []string `json:"tags"` + }{} + if err := json.Unmarshal(b, &tagsResponse); err != nil { + return allTags, err + } + allTags = append(allTags, tagsResponse.Tags...) + if link := resp.Header.Get("Link"); link != "" { + firsLink, _, _ := strings.Cut(link, ";") + linkURL, err := url.Parse(strings.Trim(firsLink, "<>")) if err != nil { - return tags, err + return allTags, err } - tagsResponse := struct { - Tags []string `json:"tags"` - }{} - if err := json.Unmarshal(b, &tagsResponse); err != nil { - return tags, err - } - tags = append(tags, tagsResponse.Tags...) - if link := resp.Header.Get("Link"); link != "" { - firsLink, _, _ := strings.Cut(link, ";") - linkURL, err := url.Parse(strings.Trim(firsLink, "<>")) - if err != nil { - return tags, err - } - - listURL = listURL.ResolveReference(linkURL) - } else { - return tags, nil - } + listURL = listURL.ResolveReference(linkURL) } else { - return tags, HandleErrorResponse(resp) + return allTags, nil } } } @@ -345,7 +341,7 @@ func (t *tags) Get(ctx context.Context, tag string) (distribution.Descriptor, er if resp.StatusCode >= 200 && resp.StatusCode < 400 { return descriptorFromResponse(resp) } - return distribution.Descriptor{}, HandleErrorResponse(resp) + return distribution.Descriptor{}, HandleHTTPResponseError(resp) } } @@ -378,10 +374,7 @@ func (t *tags) Untag(ctx context.Context, tag string) error { } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - return nil - } - return HandleErrorResponse(resp) + return HandleHTTPResponseError(resp) } type manifests struct { @@ -411,12 +404,14 @@ func (ms *manifests) Exists(ctx context.Context, dgst digest.Digest) (bool, erro } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - return true, nil - } else if resp.StatusCode == http.StatusNotFound { + if resp.StatusCode == http.StatusNotFound { return false, nil } - return false, HandleErrorResponse(resp) + + if err := HandleHTTPResponseError(resp); err != nil { + return false, err + } + return true, nil } // AddEtagToTag allows a client to supply an eTag to Get which will be @@ -517,25 +512,27 @@ func (ms *manifests) Get(ctx context.Context, dgst digest.Digest, options ...dis defer resp.Body.Close() if resp.StatusCode == http.StatusNotModified { return nil, distribution.ErrManifestNotModified - } else if SuccessStatus(resp.StatusCode) { - if contentDgst != nil { - dgst, err := digest.Parse(resp.Header.Get("Docker-Content-Digest")) - if err == nil { - *contentDgst = dgst - } - } - mt := resp.Header.Get("Content-Type") - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - m, _, err := distribution.UnmarshalManifest(mt, body) - if err != nil { - return nil, err - } - return m, nil } - return nil, HandleErrorResponse(resp) + if err := HandleHTTPResponseError(resp); err != nil { + return nil, err + } + + if contentDgst != nil { + dgst, err := digest.Parse(resp.Header.Get("Docker-Content-Digest")) + if err == nil { + *contentDgst = dgst + } + } + mt := resp.Header.Get("Content-Type") + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + m, _, err := distribution.UnmarshalManifest(mt, body) + if err != nil { + return nil, err + } + return m, nil } // Put puts a manifest. A tag can be specified using an options parameter which uses some shared state to hold the @@ -594,17 +591,16 @@ func (ms *manifests) Put(ctx context.Context, m distribution.Manifest, options . } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - dgstHeader := resp.Header.Get("Docker-Content-Digest") - dgst, err := digest.Parse(dgstHeader) - if err != nil { - return "", err - } - - return dgst, nil + if err := HandleHTTPResponseError(resp); err != nil { + return "", err } - return "", HandleErrorResponse(resp) + dgst, err := digest.Parse(resp.Header.Get("Docker-Content-Digest")) + if err != nil { + return "", err + } + + return dgst, nil } func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error { @@ -627,10 +623,7 @@ func (ms *manifests) Delete(ctx context.Context, dgst digest.Digest) error { } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - return nil - } - return HandleErrorResponse(resp) + return HandleHTTPResponseError(resp) } // todo(richardscothern): Restore interface and implementation with merge of #1050 @@ -689,7 +682,7 @@ func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (io.ReadSeekClose if resp.StatusCode == http.StatusNotFound { return distribution.ErrBlobUnknown } - return HandleErrorResponse(resp) + return HandleHTTPResponseError(resp) }), nil } @@ -732,13 +725,11 @@ func (bs *blobs) Put(ctx context.Context, mediaType string, p []byte) (distribut return distribution.Descriptor{}, fmt.Errorf("short copy: wrote %d of %d", n, len(p)) } - desc := distribution.Descriptor{ + return writer.Commit(ctx, distribution.Descriptor{ MediaType: mediaType, Size: int64(len(p)), Digest: dgstr.Digest(), - } - - return writer.Commit(ctx, desc) + }) } type optionFunc func(interface{}) error @@ -827,7 +818,7 @@ func (bs *blobs) Create(ctx context.Context, options ...distribution.BlobCreateO location: location, }, nil default: - return nil, HandleErrorResponse(resp) + return nil, HandleHTTPResponseError(resp) } } @@ -877,26 +868,29 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - lengthHeader := resp.Header.Get("Content-Length") - if lengthHeader == "" { - return distribution.Descriptor{}, fmt.Errorf("missing content-length header for request: %s", u) - } - - length, err := strconv.ParseInt(lengthHeader, 10, 64) - if err != nil { - return distribution.Descriptor{}, fmt.Errorf("error parsing content-length: %v", err) - } - - return distribution.Descriptor{ - MediaType: resp.Header.Get("Content-Type"), - Size: length, - Digest: dgst, - }, nil - } else if resp.StatusCode == http.StatusNotFound { + if resp.StatusCode == http.StatusNotFound { return distribution.Descriptor{}, distribution.ErrBlobUnknown } - return distribution.Descriptor{}, HandleErrorResponse(resp) + + if err := HandleHTTPResponseError(resp); err != nil { + return distribution.Descriptor{}, err + } + + lengthHeader := resp.Header.Get("Content-Length") + if lengthHeader == "" { + return distribution.Descriptor{}, fmt.Errorf("missing content-length header for request: %s", u) + } + + length, err := strconv.ParseInt(lengthHeader, 10, 64) + if err != nil { + return distribution.Descriptor{}, fmt.Errorf("error parsing content-length: %v", err) + } + + return distribution.Descriptor{ + MediaType: resp.Header.Get("Content-Type"), + Size: length, + Digest: dgst, + }, nil } func buildCatalogValues(maxEntries int, last string) url.Values { @@ -934,10 +928,7 @@ func (bs *blobStatter) Clear(ctx context.Context, dgst digest.Digest) error { } defer resp.Body.Close() - if SuccessStatus(resp.StatusCode) { - return nil - } - return HandleErrorResponse(resp) + return HandleHTTPResponseError(resp) } func (bs *blobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error {