From e28f0d92991e60ac3174b2ebf224f37be22c8fad Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 24 Jun 2019 12:29:28 -0400 Subject: [PATCH] cmd/go/internal/modfetch: return structured errors from proxy operations CL 181881 added structured error types for direct fetches. Use those same structured errors to format proxy errors consistently. Also ensure that an empty @v/list is treated as equivalent to the module not existing at all. Updates #27173 Updates #32715 Change-Id: I203fd8259bc4f28b3389745f1a1fde936b0fa24d Reviewed-on: https://go-review.googlesource.com/c/go/+/183619 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod --- .../go/internal/modfetch/codehost/codehost.go | 14 ++++- src/cmd/go/internal/modfetch/codehost/git.go | 2 +- src/cmd/go/internal/modfetch/proxy.go | 52 +++++++++++++------ src/cmd/go/internal/modload/query.go | 9 +++- src/cmd/go/internal/module/module.go | 5 +- .../go/testdata/script/mod_query_empty.txt | 52 +++++++++++++++++++ 6 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_query_empty.txt diff --git a/src/cmd/go/internal/modfetch/codehost/codehost.go b/src/cmd/go/internal/modfetch/codehost/codehost.go index ab9287b541..a4e50d692a 100644 --- a/src/cmd/go/internal/modfetch/codehost/codehost.go +++ b/src/cmd/go/internal/modfetch/codehost/codehost.go @@ -116,8 +116,20 @@ type UnknownRevisionError struct { func (e *UnknownRevisionError) Error() string { return "unknown revision " + e.Rev } +func (UnknownRevisionError) Is(err error) bool { + return err == os.ErrNotExist +} + +// ErrNoCommits is an error equivalent to os.ErrNotExist indicating that a given +// repository or module contains no commits. +var ErrNoCommits error = noCommitsError{} -func (e *UnknownRevisionError) Is(err error) bool { +type noCommitsError struct{} + +func (noCommitsError) Error() string { + return "no commits" +} +func (noCommitsError) Is(err error) bool { return err == os.ErrNotExist } diff --git a/src/cmd/go/internal/modfetch/codehost/git.go b/src/cmd/go/internal/modfetch/codehost/git.go index 83e694dfe8..d382e8ac9a 100644 --- a/src/cmd/go/internal/modfetch/codehost/git.go +++ b/src/cmd/go/internal/modfetch/codehost/git.go @@ -222,7 +222,7 @@ func (r *gitRepo) Latest() (*RevInfo, error) { return nil, r.refsErr } if r.refs["HEAD"] == "" { - return nil, fmt.Errorf("no commits") + return nil, ErrNoCommits } return r.Stat(r.refs["HEAD"]) } diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go index 426499baa9..6235ad3d6e 100644 --- a/src/cmd/go/internal/modfetch/proxy.go +++ b/src/cmd/go/internal/modfetch/proxy.go @@ -196,6 +196,26 @@ func (p *proxyRepo) ModulePath() string { return p.path } +// versionError returns err wrapped in a ModuleError for p.path. +func (p *proxyRepo) versionError(version string, err error) error { + if version != "" && version != module.CanonicalVersion(version) { + return &module.ModuleError{ + Path: p.path, + Err: &module.InvalidVersionError{ + Version: version, + Pseudo: IsPseudoVersion(version), + Err: err, + }, + } + } + + return &module.ModuleError{ + Path: p.path, + Version: version, + Err: err, + } +} + func (p *proxyRepo) getBytes(path string) ([]byte, error) { body, err := p.getBody(path) if err != nil { @@ -226,7 +246,7 @@ func (p *proxyRepo) getBody(path string) (io.ReadCloser, error) { func (p *proxyRepo) Versions(prefix string) ([]string, error) { data, err := p.getBytes("@v/list") if err != nil { - return nil, err + return nil, p.versionError("", err) } var list []string for _, line := range strings.Split(string(data), "\n") { @@ -242,7 +262,7 @@ func (p *proxyRepo) Versions(prefix string) ([]string, error) { func (p *proxyRepo) latest() (*RevInfo, error) { data, err := p.getBytes("@v/list") if err != nil { - return nil, err + return nil, p.versionError("", err) } var best time.Time var bestVersion string @@ -257,7 +277,7 @@ func (p *proxyRepo) latest() (*RevInfo, error) { } } if bestVersion == "" { - return nil, fmt.Errorf("no commits") + return nil, p.versionError("", codehost.ErrNoCommits) } info := &RevInfo{ Version: bestVersion, @@ -271,21 +291,21 @@ func (p *proxyRepo) latest() (*RevInfo, error) { func (p *proxyRepo) Stat(rev string) (*RevInfo, error) { encRev, err := module.EncodeVersion(rev) if err != nil { - return nil, err + return nil, p.versionError(rev, err) } data, err := p.getBytes("@v/" + encRev + ".info") if err != nil { - return nil, err + return nil, p.versionError(rev, err) } info := new(RevInfo) if err := json.Unmarshal(data, info); err != nil { - return nil, err + return nil, p.versionError(rev, 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 // proxy must return either exactly that version or an error — not some // arbitrary other version. - return nil, fmt.Errorf("requested canonical version %s, but proxy returned info for version %s", rev, info.Version) + return nil, p.versionError(rev, fmt.Errorf("proxy returned info for version %s instead of requested version", info.Version)) } return info, nil } @@ -298,48 +318,48 @@ func (p *proxyRepo) Latest() (*RevInfo, error) { } info := new(RevInfo) if err := json.Unmarshal(data, info); err != nil { - return nil, err + return nil, p.versionError("", err) } return info, nil } func (p *proxyRepo) GoMod(version string) ([]byte, error) { if version != module.CanonicalVersion(version) { - return nil, fmt.Errorf("version %s is not canonical", version) + return nil, p.versionError(version, fmt.Errorf("internal error: version passed to GoMod is not canonical")) } encVer, err := module.EncodeVersion(version) if err != nil { - return nil, err + return nil, p.versionError(version, err) } data, err := p.getBytes("@v/" + encVer + ".mod") if err != nil { - return nil, err + return nil, p.versionError(version, err) } return data, nil } func (p *proxyRepo) Zip(dst io.Writer, version string) error { if version != module.CanonicalVersion(version) { - return fmt.Errorf("version %s is not canonical", version) + return p.versionError(version, fmt.Errorf("internal error: version passed to Zip is not canonical")) } encVer, err := module.EncodeVersion(version) if err != nil { - return err + return p.versionError(version, err) } body, err := p.getBody("@v/" + encVer + ".zip") if err != nil { - return err + 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 { - return err + return p.versionError(version, err) } if lr.N <= 0 { - return fmt.Errorf("downloaded zip file too large") + return p.versionError(version, fmt.Errorf("downloaded zip file too large")) } return nil } diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go index 1e55992777..8ce61c0a1d 100644 --- a/src/cmd/go/internal/modload/query.go +++ b/src/cmd/go/internal/modload/query.go @@ -243,8 +243,13 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version) if mayUseLatest { // Special case for "latest": if no tags match, use latest commit in repo, // provided it is not excluded. - if latest, err := repo.Latest(); err == nil && allowed(module.Version{Path: path, Version: latest.Version}) { - return lookup(latest.Version) + latest, err := repo.Latest() + if err == nil { + if allowed(module.Version{Path: path, Version: latest.Version}) { + return lookup(latest.Version) + } + } else if !errors.Is(err, os.ErrNotExist) { + return nil, err } } diff --git a/src/cmd/go/internal/module/module.go b/src/cmd/go/internal/module/module.go index 4a313f99f9..3e0baba15b 100644 --- a/src/cmd/go/internal/module/module.go +++ b/src/cmd/go/internal/module/module.go @@ -536,7 +536,10 @@ func EncodePath(path string) (encoding string, err error) { // and not contain exclamation marks. func EncodeVersion(v string) (encoding string, err error) { if err := checkElem(v, true); err != nil || strings.Contains(v, "!") { - return "", fmt.Errorf("disallowed version string %q", v) + return "", &InvalidVersionError{ + Version: v, + Err: fmt.Errorf("disallowed version string"), + } } return encodeString(v) } diff --git a/src/cmd/go/testdata/script/mod_query_empty.txt b/src/cmd/go/testdata/script/mod_query_empty.txt new file mode 100644 index 0000000000..4e27c1ee5c --- /dev/null +++ b/src/cmd/go/testdata/script/mod_query_empty.txt @@ -0,0 +1,52 @@ +env GO111MODULE=on +env GOSUMDB=off + +go mod download example.com/join@v1.1.0 + +# If the proxy serves a bogus result for the @latest version, +# reading that version should cause 'go get' to fail. +env GOPROXY=file:///$WORK/badproxy +cp go.mod.orig go.mod +! go get -d example.com/join/subpkg +stderr 'go get example.com/join/subpkg: example.com/join/subpkg@v0.0.0-20190624000000-123456abcdef: .*' + +# If @v/list is empty, the 'go' command should still try to resolve +# other module paths. +env GOPROXY=file:///$WORK/emptysub +cp go.mod.orig go.mod +go get -d example.com/join/subpkg +go list -m example.com/join/... +! stdout 'example.com/join/subpkg' +stdout 'example.com/join v1.1.0' + +# If @v/list includes a version that the proxy does not actually serve, +# that version is treated as nonexistent. +env GOPROXY=file:///$WORK/notfound +cp go.mod.orig go.mod +go get -d example.com/join/subpkg +go list -m example.com/join/... +! stdout 'example.com/join/subpkg' +stdout 'example.com/join v1.1.0' + +-- go.mod.orig -- +module example.com/othermodule +go 1.13 +-- $WORK/badproxy/example.com/join/subpkg/@v/list -- +v0.0.0-20190624000000-123456abcdef +-- $WORK/badproxy/example.com/join/subpkg/@v/v0.0.0-20190624000000-123456abcdef.info -- +This file is not valid JSON. +-- $WORK/badproxy/example.com/join/@v/list -- +v1.1.0 +-- $WORK/badproxy/example.com/join/@v/v1.1.0.info -- +{"Version": "v1.1.0"} +-- $WORK/emptysub/example.com/join/subpkg/@v/list -- +-- $WORK/emptysub/example.com/join/@v/list -- +v1.1.0 +-- $WORK/emptysub/example.com/join/@v/v1.1.0.info -- +{"Version": "v1.1.0"} +-- $WORK/notfound/example.com/join/subpkg/@v/list -- +v1.0.0-does-not-exist +-- $WORK/notfound/example.com/join/@v/list -- +v1.1.0 +-- $WORK/notfound/example.com/join/@v/v1.1.0.info -- +{"Version": "v1.1.0"} -- 2.50.0