cmd/go/internal/web: minor api cleanup

- rename PasswordRedacted to Redacted
- move URL into Response in redacted form, remove from Get result list
- add Response.Err to construct non-200 errors
  (otherwise GetBytes is not just a wrapper)
- make 404/410 errors satisfy Is(err, os.ErrNotExist)

Change-Id: Id15899c1e3dfd30cffb1a75ba79a9a1999913258
Reviewed-on: https://go-review.googlesource.com/c/go/+/173717
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

src/cmd/go/internal/get/vcs.go

@@ -782,7 +782,7 @@ func repoRootForImportDynamic(importPath string, mod ModuleMode, security web.Se
 	if err != nil {
 		return nil, err
 	}
-	url, resp, err := web.Get(security, url)
+	resp, err := web.Get(security, url)
 	if err != nil {
 		msg := "https fetch: %v"
 		if security == web.Insecure {
@@ -802,7 +802,7 @@ func repoRootForImportDynamic(importPath string, mod ModuleMode, security web.Se
 		if _, ok := err.(ImportMismatchError); !ok {
 			return nil, fmt.Errorf("parse %s: %v", url, err)
 		}
-		return nil, fmt.Errorf("parse %s: no go-import meta tags (%s)", url, err)
+		return nil, fmt.Errorf("parse %s: no go-import meta tags (%s)", resp.URL, err)
 	}
 	if cfg.BuildV {
 		log.Printf("get %q: found meta tag %#v at %s", importPath, mmi, url)
@@ -817,7 +817,6 @@ func repoRootForImportDynamic(importPath string, mod ModuleMode, security web.Se
 		if cfg.BuildV {
 			log.Printf("get %q: verifying non-authoritative meta tag", importPath)
 		}
-		url0 := *url
 		var imports []metaImport
 		url, imports, err = metaImportsForPrefix(mmi.Prefix, mod, security)
 		if err != nil {
@@ -825,16 +824,16 @@ func repoRootForImportDynamic(importPath string, mod ModuleMode, security web.Se
 		}
 		metaImport2, err := matchGoImport(imports, importPath)
 		if err != nil || mmi != metaImport2 {
-			return nil, fmt.Errorf("%s and %s disagree about go-import for %s", &url0, url, mmi.Prefix)
+			return nil, fmt.Errorf("%s and %s disagree about go-import for %s", resp.URL, url, mmi.Prefix)
 		}
 	}
 
 	if err := validateRepoRoot(mmi.RepoRoot); err != nil {
-		return nil, fmt.Errorf("%s: invalid repo root %q: %v", url, mmi.RepoRoot, err)
+		return nil, fmt.Errorf("%s: invalid repo root %q: %v", resp.URL, mmi.RepoRoot, err)
 	}
 	vcs := vcsByCmd(mmi.VCS)
 	if vcs == nil && mmi.VCS != "mod" {
-		return nil, fmt.Errorf("%s: unknown vcs %q", url, mmi.VCS)
+		return nil, fmt.Errorf("%s: unknown vcs %q", resp.URL, mmi.VCS)
 	}
 
 	rr := &RepoRoot{
@@ -894,15 +893,15 @@ func metaImportsForPrefix(importPrefix string, mod ModuleMode, security web.Secu
 		if err != nil {
 			return setCache(fetchResult{err: err})
 		}
-		url, resp, err := web.Get(security, url)
+		resp, err := web.Get(security, url)
 		if err != nil {
-			return setCache(fetchResult{url: url, err: fmt.Errorf("fetch %s: %v", url, err)})
+			return setCache(fetchResult{url: url, err: fmt.Errorf("fetch %s: %v", resp.URL, err)})
 		}
 		body := resp.Body
 		defer body.Close()
 		imports, err := parseMetaGoImports(body, mod)
 		if err != nil {
-			return setCache(fetchResult{url: url, err: fmt.Errorf("parsing %s: %v", url, err)})
+			return setCache(fetchResult{url: url, err: fmt.Errorf("parsing %s: %v", resp.URL, err)})
 		}
 		if len(imports) == 0 {
 			err = fmt.Errorf("fetch %s: no go-import meta tag", url)

src/cmd/go/internal/modfetch/proxy.go

@@ -124,11 +124,11 @@ func newProxyRepo(baseURL, path string) (Repo, error) {
 	switch url.Scheme {
 	case "file":
 		if *url != (urlpkg.URL{Scheme: url.Scheme, Path: url.Path, RawPath: url.RawPath}) {
-			return nil, fmt.Errorf("proxy URL %q uses file scheme with non-path elements", web.PasswordRedacted(url))
+			return nil, fmt.Errorf("proxy URL %q uses file scheme with non-path elements", web.Redacted(url))
 		}
 	case "http", "https":
 	case "":
-		return nil, fmt.Errorf("proxy URL %q missing scheme", web.PasswordRedacted(url))
+		return nil, fmt.Errorf("proxy URL %q missing scheme", web.Redacted(url))
 	default:
 		return nil, fmt.Errorf("unsupported proxy scheme %q", url.Scheme)
 	}
@@ -171,12 +171,12 @@ func (p *proxyRepo) getBody(path string) (io.ReadCloser, error) {
 	url.Path = fullPath
 	url.RawPath = pathpkg.Join(url.RawPath, pathEscape(path))
 
-	_, resp, err := web.Get(web.DefaultSecurity, url)
+	resp, err := web.Get(web.DefaultSecurity, url)
 	if err != nil {
 		return nil, err
 	}
 	if resp.StatusCode != 200 {
-		return nil, fmt.Errorf("unexpected status (%s): %v", web.PasswordRedacted(url), resp.Status)
+		return nil, fmt.Errorf("unexpected status (%s): %v", web.Redacted(url), resp.Status)
 	}
 	return resp.Body, nil
 }

src/cmd/go/internal/web/api.go

@@ -13,7 +13,8 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	urlpkg "net/url"
+	"net/url"
+	"os"
 )
 
 // SecurityMode specifies whether a function should make network
@@ -27,50 +28,65 @@ const (
 	Insecure                            // Allow plain HTTP if not explicitly HTTPS; skip HTTPS validation.
 )
 
+// An HTTPError describes an HTTP error response (non-200 result).
 type HTTPError struct {
-	status     string
+	URL        string // redacted
+	Status     string
 	StatusCode int
-	url        *urlpkg.URL
 }
 
 func (e *HTTPError) Error() string {
-	return fmt.Sprintf("%s: %s", e.url, e.status)
+	return fmt.Sprintf("reading %s: %v", e.URL, e.Status)
+}
+
+func (e *HTTPError) Is(target error) bool {
+	return target == os.ErrNotExist && (e.StatusCode == 404 || e.StatusCode == 410)
 }
 
 // GetBytes returns the body of the requested resource, or an error if the
-// response status was not http.StatusOk.
+// response status was not http.StatusOK.
 //
-// GetBytes is a convenience wrapper around Get.
-func GetBytes(url *urlpkg.URL) ([]byte, error) {
-	url, resp, err := Get(DefaultSecurity, url)
+// GetBytes is a convenience wrapper around Get and Response.Err.
+func GetBytes(u *url.URL) ([]byte, error) {
+	resp, err := Get(DefaultSecurity, u)
 	if err != nil {
 		return nil, err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode != 200 {
-		err := &HTTPError{status: resp.Status, StatusCode: resp.StatusCode, url: url}
+	if err := resp.Err(); err != nil {
 		return nil, err
 	}
 	b, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("%s: %v", url, err)
+		return nil, fmt.Errorf("reading %s: %v", Redacted(u), err)
 	}
 	return b, nil
 }
 
 type Response struct {
+	URL        string // redacted
 	Status     string
 	StatusCode int
 	Header     map[string][]string
 	Body       io.ReadCloser
 }
 
+// Err returns an *HTTPError corresponding to the response r.
+// It returns nil if the response r has StatusCode 200 or 0 (unset).
+func (r *Response) Err() error {
+	if r.StatusCode == 200 || r.StatusCode == 0 {
+		return nil
+	}
+	return &HTTPError{URL: r.URL, Status: r.Status, StatusCode: r.StatusCode}
+}
+
 // Get returns the body of the HTTP or HTTPS resource specified at the given URL.
 //
 // If the URL does not include an explicit scheme, Get first tries "https".
 // If the server does not respond under that scheme and the security mode is
 // Insecure, Get then tries "http".
-// The returned URL indicates which scheme was actually used.
+// The URL included in the response indicates which scheme was actually used,
+// and it is a redacted URL suitable for use in error messages.
 //
 // For the "https" scheme only, credentials are attached using the
 // cmd/go/internal/auth package. If the URL itself includes a username and
@@ -79,21 +95,23 @@ type Response struct {
 //
 // Get returns a non-nil error only if the request did not receive a response
 // under any applicable scheme. (A non-2xx response does not cause an error.)
-func Get(security SecurityMode, url *urlpkg.URL) (*urlpkg.URL, *Response, error) {
-	return get(security, url)
+func Get(security SecurityMode, u *url.URL) (*Response, error) {
+	return get(security, u)
 }
 
-// PasswordRedacted returns url directly if it does not encode a password,
-// or else a copy of url with the password redacted.
-func PasswordRedacted(url *urlpkg.URL) *urlpkg.URL {
-	if url.User != nil {
-		if _, ok := url.User.Password(); ok {
-			redacted := *url
-			redacted.User = urlpkg.UserPassword(url.User.Username(), "[redacted]")
-			return &redacted
+// Redacted returns a redacted string form of the URL,
+// suitable for printing in error messages.
+// The string form replaces any non-empty password
+// in the original URL with "[redacted]".
+func Redacted(u *url.URL) string {
+	if u.User != nil {
+		if _, ok := u.User.Password(); ok {
+			redacted := *u
+			redacted.User = url.UserPassword(u.User.Username(), "[redacted]")
+			u = &redacted
 		}
 	}
-	return url
+	return u.String()
 }
 
 // OpenBrowser attempts to open the requested URL in a web browser.

src/cmd/go/internal/web/bootstrap.go

@@ -16,8 +16,8 @@ import (
 	urlpkg "net/url"
 )
 
-func get(security SecurityMode, url *urlpkg.URL) (*urlpkg.URL, *Response, error) {
-	return nil, nil, errors.New("no http in bootstrap go command")
+func get(security SecurityMode, url *urlpkg.URL) (*Response, error) {
+	return nil, errors.New("no http in bootstrap go command")
 }
 
 func openBrowser(url string) bool { return false }

src/cmd/go/internal/web/http.go

@@ -49,7 +49,7 @@ var securityPreservingHTTPClient = &http.Client{
 	},
 }
 
-func get(security SecurityMode, url *urlpkg.URL) (*urlpkg.URL, *Response, error) {
+func get(security SecurityMode, url *urlpkg.URL) (*Response, error) {
 	fetch := func(url *urlpkg.URL) (*urlpkg.URL, *http.Response, error) {
 		if cfg.BuildV {
 			log.Printf("Fetching %s", url)
@@ -90,7 +90,7 @@ func get(security SecurityMode, url *urlpkg.URL) (*urlpkg.URL, *Response, error)
 			if security != Insecure || url.Scheme == "https" {
 				// HTTPS failed, and we can't fall back to plain HTTP.
 				// Report the error from the HTTPS attempt.
-				return nil, nil, err
+				return nil, err
 			}
 		}
 	}
@@ -99,42 +99,44 @@ func get(security SecurityMode, url *urlpkg.URL) (*urlpkg.URL, *Response, error)
 		switch url.Scheme {
 		case "http":
 			if security == SecureOnly {
-				return nil, nil, fmt.Errorf("URL %q is not secure", PasswordRedacted(url))
+				return nil, fmt.Errorf("insecure URL: %s", Redacted(url))
 			}
 		case "":
 			if security != Insecure {
 				panic("should have returned after HTTPS failure")
 			}
 		default:
-			return nil, nil, fmt.Errorf("unsupported scheme %s", url.Scheme)
+			return nil, fmt.Errorf("unsupported scheme: %s", Redacted(url))
 		}
 
 		insecure := new(urlpkg.URL)
 		*insecure = *url
 		insecure.Scheme = "http"
 		if insecure.User != nil && security != Insecure {
-			return nil, nil, fmt.Errorf("refusing to pass credentials to insecure URL %q", PasswordRedacted(insecure))
+			return nil, fmt.Errorf("refusing to pass credentials to insecure URL: %s", Redacted(insecure))
 		}
 
 		fetched, res, err = fetch(insecure)
 		if err != nil {
 			// HTTP failed, and we already tried HTTPS if applicable.
 			// Report the error from the HTTP attempt.
-			return nil, nil, err
+			return nil, err
 		}
 	}
 
 	// Note: accepting a non-200 OK here, so people can serve a
 	// meta import in their http 404 page.
 	if cfg.BuildV {
-		log.Printf("Parsing meta tags from %s (status code %d)", PasswordRedacted(fetched), res.StatusCode)
+		log.Printf("Parsing meta tags from %s (status code %d)", Redacted(fetched), res.StatusCode)
 	}
-	return fetched, &Response{
+	r := &Response{
+		URL:        Redacted(fetched),
 		Status:     res.Status,
 		StatusCode: res.StatusCode,
 		Header:     map[string][]string(res.Header),
 		Body:       res.Body,
-	}, nil
+	}
+	return r, nil
 }
 
 func openBrowser(url string) bool { return browser.Open(url) }