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 <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>

Author: dmitshur

Diff for: src/cmd/go/internal/modfetch/proxy.go

@@ -185,41 +185,45 @@ 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
 	listLatestErr  error
 }
 
 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,22 +257,22 @@ 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
 	}
 	defer body.Close()
 
 	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,17 +426,17 @@ 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)
 	}
 	defer body.Close()
 
 	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 {