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>

diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go
index bb1845e315e9c777d849f251c5ea19d97748c158..262bf2979e5b7eaff6d1c56332e1debd45b0b4f2 100644 (file)
--- a/src/cmd/go/internal/get/vcs.go
+++ b/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)

diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go
index ec9caf1556c11889d1cea43e87473519871e7f60..97064cd1ce491a96c3100385806ab89c3a11f774 100644 (file)
--- a/src/cmd/go/internal/modfetch/proxy.go
+++ b/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
 }

diff --git a/src/cmd/go/internal/web/api.go b/src/cmd/go/internal/web/api.go
index 5dc81de9b66110fcefe9c4448d41d3dfc691c921..0012cfc6f4e71e62f77d41049a4b46590be7974a 100644 (file)
--- a/src/cmd/go/internal/web/api.go
+++ b/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.

diff --git a/src/cmd/go/internal/web/bootstrap.go b/src/cmd/go/internal/web/bootstrap.go
index 84e9d356448117013f612ce50ce6bb639b9f8be0..781702100a09e8b596ce9d3a9095e7dc88ada578 100644 (file)
--- a/src/cmd/go/internal/web/bootstrap.go
+++ b/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 }

diff --git a/src/cmd/go/internal/web/http.go b/src/cmd/go/internal/web/http.go
index 0711f812091a79e02c74d8f89609837018fc5f91..e126b03273a8d024b453a8ab48600c5172a366c1 100644 (file)
--- a/src/cmd/go/internal/web/http.go
+++ b/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) }