Skip to content

ggcr: retryError consumes HTTP response body #2125

@ryanbrainard

Description

@ryanbrainard

Describe the bug

The retryError function in pkg/v1/remote/transport/error.go consumes the HTTP response body by calling io.ReadAll(resp.Body), but doesn't restore the body for subsequent callers. This makes the response body unavailable for retry logic or other error handling that might need to read the body content. This also means that the resulting error is different when retries are used or not used.

To Reproduce

package main

import (
	"fmt"
	"github.com/google/go-containerregistry/pkg/name"
	"github.com/google/go-containerregistry/pkg/v1/remote"
	"net/http"
)

func main() {
	const ref = "index.docker.io/docker/labs-k8s-toolkit-extension:0.0.46-bad-version"

	fmt.Println("Without 404 retry:")
	if _, err := remote.Get(name.MustParseReference(ref)); err != nil {
		fmt.Println(err)
	}

	fmt.Println("With 404 retry:")
	if _, err := remote.Get(name.MustParseReference(ref), remote.WithRetryStatusCodes(http.StatusNotFound)); err != nil {
		fmt.Println(err)
	}
}

Output:

Notice how the error message returned is different. Namely, the MANIFEST_UNKNOWN is swallowed by the retry handler.

Without 404 retry:
GET https://index.docker.io/v2/docker/labs-k8s-toolkit-extension/manifests/0.0.46-bad-version: MANIFEST_UNKNOWN: manifest unknown; unknown tag=0.0.46-bad-version
With 404 retry:
GET https://index.docker.io/v2/docker/labs-k8s-toolkit-extension/manifests/0.0.46-bad-version: unexpected status code 404 Not Found

Expected behavior

Both cases should show the same detailed error message with the registry's structured error response (MANIFEST_UNKNOWN: manifest unknown; unknown tag=0.0.46-bad-version). The retry case loses the structured error details because the response body was consumed by retryError and is no longer available for proper error parsing.

Additional context

Location: pkg/v1/remote/transport/error.go, line ~185 in retryError function

Root cause: The retryError function calls io.ReadAll(resp.Body) which consumes the body, making it unavailable for subsequent error parsing that would extract the structured registry error details.

Proposed fix:

func retryError(resp *http.Response) error {
	b, err := io.ReadAll(resp.Body)
	if err != nil {
		return err
	}
	resp.Body.Close()
	resp.Body = io.NopCloser(bytes.NewReader(b))  // Restore readable body
	
	rerr := makeError(resp, b)
	rerr.temporary = true
	return rerr
}

This ensures the response body can be read multiple times by creating a new io.ReadCloser from the buffered data.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions