From: Russ Cox Date: Tue, 23 Apr 2019 20:51:28 +0000 (-0400) Subject: cmd/go/internal/web: minor api cleanup X-Git-Tag: go1.13beta1~556 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5fa14a31b0b4bd95cf10a1394d2322db110b25b4;p=gostls13.git 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick Reviewed-by: Bryan C. Mills --- diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go index bb1845e315..262bf2979e 100644 --- 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 ec9caf1556..97064cd1ce 100644 --- 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 5dc81de9b6..0012cfc6f4 100644 --- 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 84e9d35644..781702100a 100644 --- 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 0711f81209..e126b03273 100644 --- 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) }