cmd/go/internal/modfetch: show real URL in response body read errors

CL 233437 added a redactedURL field to proxyRepo, a struct that already
had a field named 'url'. Neither fields were documented, so the similar
names suggest the most natural interpretation that proxyRepo.redactedURL
is equivalent to proxyRepo.url.Redacted() rather than something else.
That's possibly why it was joined with the module version in CL 406675.

It turns out the two URLs differ in more than just redaction: one is the
base proxy URL with (escaped) module path joined, the other is just the
base proxy URL, in redacted form.

Document and rename the fields to make the distinction more clear, and
include all 3 of base module proxy URL + module path + module version
in the reported URL, rather than just the first and third bits as seen
in the errors at https://go.dev/issue/51323#issuecomment-1735812250.

For #51323.
Updates #38680.
Updates #52727.

Change-Id: Ib4b134b548adeec826ee88fe51a2cf580fde0516
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/532035
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go
index 56a6aaa40d9c09ae681aa7bd6069b53b3c06c9a6..e0efb097ecdda9ee27f71c673258ab9f022f1a71 100644 (file)
--- a/src/cmd/go/internal/modfetch/proxy.go
+++ b/src/cmd/go/internal/modfetch/proxy.go
@@ -185,9 +185,9 @@ func TryProxies(f func(proxy string) error) error {
 }
 
 type proxyRepo struct {
-       url         *url.URL
-       path        string
-       redactedURL string
+       url          *url.URL // The combined module proxy URL joined with the module path.
+       path         string   // The module path (unescaped).
+       redactedBase string   // The base module proxy URL in [url.URL.Redacted] form.
 
        listLatestOnce sync.Once
        listLatest     *RevInfo
@@ -195,31 +195,35 @@ type proxyRepo struct {
 }
 
 func newProxyRepo(baseURL, path string) (Repo, error) {
+       // Parse the base proxy URL.
        base, err := url.Parse(baseURL)
        if err != nil {
                return nil, err
        }
+       redactedBase := base.Redacted()
        switch base.Scheme {
        case "http", "https":
                // ok
        case "file":
                if *base != (url.URL{Scheme: base.Scheme, Path: base.Path, RawPath: base.RawPath}) {
-                       return nil, fmt.Errorf("invalid file:// proxy URL with non-path elements: %s", base.Redacted())
+                       return nil, fmt.Errorf("invalid file:// proxy URL with non-path elements: %s", redactedBase)
                }
        case "":
-               return nil, fmt.Errorf("invalid proxy URL missing scheme: %s", base.Redacted())
+               return nil, fmt.Errorf("invalid proxy URL missing scheme: %s", redactedBase)
        default:
-               return nil, fmt.Errorf("invalid proxy URL scheme (must be https, http, file): %s", base.Redacted())
+               return nil, fmt.Errorf("invalid proxy URL scheme (must be https, http, file): %s", redactedBase)
        }
 
+       // Append the module path to the URL.
+       url := base
        enc, err := module.EscapePath(path)
        if err != nil {
                return nil, err
        }
-       redactedURL := base.Redacted()
-       base.Path = strings.TrimSuffix(base.Path, "/") + "/" + enc
-       base.RawPath = strings.TrimSuffix(base.RawPath, "/") + "/" + pathEscape(enc)
-       return &proxyRepo{base, path, redactedURL, sync.Once{}, nil, nil}, nil
+       url.Path = strings.TrimSuffix(base.Path, "/") + "/" + enc
+       url.RawPath = strings.TrimSuffix(base.RawPath, "/") + "/" + pathEscape(enc)
+
+       return &proxyRepo{url, path, redactedBase, sync.Once{}, nil, nil}, nil
 }
 
 func (p *proxyRepo) ModulePath() string {
@@ -253,7 +257,7 @@ func (p *proxyRepo) versionError(version string, err error) error {
 }
 
 func (p *proxyRepo) getBytes(ctx context.Context, path string) ([]byte, error) {
-       body, err := p.getBody(ctx, path)
+       body, redactedURL, err := p.getBody(ctx, path)
        if err != nil {
                return nil, err
        }
@@ -261,14 +265,14 @@ func (p *proxyRepo) getBytes(ctx context.Context, path string) ([]byte, error) {
 
        b, err := io.ReadAll(body)
        if err != nil {
-               // net/http doesn't add context to Body errors, so add it here.
+               // net/http doesn't add context to Body read errors, so add it here.
                // (See https://go.dev/issue/52727.)
-               return b, &url.Error{Op: "read", URL: strings.TrimSuffix(p.redactedURL, "/") + "/" + path, Err: err}
+               return b, &url.Error{Op: "read", URL: redactedURL, Err: err}
        }
        return b, nil
 }
 
-func (p *proxyRepo) getBody(ctx context.Context, path string) (r io.ReadCloser, err error) {
+func (p *proxyRepo) getBody(ctx context.Context, path string) (r io.ReadCloser, redactedURL string, err error) {
        fullPath := pathpkg.Join(p.url.Path, path)
 
        target := *p.url
@@ -277,13 +281,13 @@ func (p *proxyRepo) getBody(ctx context.Context, path string) (r io.ReadCloser,
 
        resp, err := web.Get(web.DefaultSecurity, &target)
        if err != nil {
-               return nil, err
+               return nil, "", err
        }
        if err := resp.Err(); err != nil {
                resp.Body.Close()
-               return nil, err
+               return nil, "", err
        }
-       return resp.Body, nil
+       return resp.Body, resp.URL, nil
 }
 
 func (p *proxyRepo) Versions(ctx context.Context, prefix string) (*Versions, error) {
@@ -370,7 +374,7 @@ func (p *proxyRepo) Stat(ctx context.Context, rev string) (*RevInfo, error) {
        }
        info := new(RevInfo)
        if err := json.Unmarshal(data, info); err != nil {
-               return nil, p.versionError(rev, fmt.Errorf("invalid response from proxy %q: %w", p.redactedURL, err))
+               return nil, p.versionError(rev, fmt.Errorf("invalid response from proxy %q: %w", p.redactedBase, err))
        }
        if info.Version != rev && rev == module.CanonicalVersion(rev) && module.Check(p.path, rev) == nil {
                // If we request a correct, appropriate version for the module path, the
@@ -391,7 +395,7 @@ func (p *proxyRepo) Latest(ctx context.Context) (*RevInfo, error) {
        }
        info := new(RevInfo)
        if err := json.Unmarshal(data, info); err != nil {
-               return nil, p.versionError("", fmt.Errorf("invalid response from proxy %q: %w", p.redactedURL, err))
+               return nil, p.versionError("", fmt.Errorf("invalid response from proxy %q: %w", p.redactedBase, err))
        }
        return info, nil
 }
@@ -422,7 +426,7 @@ func (p *proxyRepo) Zip(ctx context.Context, dst io.Writer, version string) erro
                return p.versionError(version, err)
        }
        path := "@v/" + encVer + ".zip"
-       body, err := p.getBody(ctx, path)
+       body, redactedURL, err := p.getBody(ctx, path)
        if err != nil {
                return p.versionError(version, err)
        }
@@ -430,9 +434,9 @@ func (p *proxyRepo) Zip(ctx context.Context, dst io.Writer, version string) erro
 
        lr := &io.LimitedReader{R: body, N: codehost.MaxZipFile + 1}
        if _, err := io.Copy(dst, lr); err != nil {
-               // net/http doesn't add context to Body errors, so add it here.
+               // net/http doesn't add context to Body read errors, so add it here.
                // (See https://go.dev/issue/52727.)
-               err = &url.Error{Op: "read", URL: strings.TrimSuffix(p.redactedURL, "/") + "/" + path, Err: err}
+               err = &url.Error{Op: "read", URL: redactedURL, Err: err}
                return p.versionError(version, err)
        }
        if lr.N <= 0 {