From: Bryan C. Mills Date: Thu, 5 Sep 2019 18:27:27 +0000 (-0400) Subject: cmd/go/internal/modfetch/codehost: treat nonexistent repositories as “not found” X-Git-Tag: go1.14beta1~1136 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f6c691e0e1b1434a02301c39e6d66e21699a98a8;p=gostls13.git cmd/go/internal/modfetch/codehost: treat nonexistent repositories as “not found” If a go-import directive refers to a nonexistent repository, today we treat that as an error fetching a module that actually exists. That makes the HTTP server responsible for determining which repositories do or do not exist, which may in general depend on the user's separately-stored credentials, and imposes significant complexity on such a server, which can otherwise be very simple. Instead, check the repository URL and/or error message to try to determine whether the repository exists at all. If the repo does not exist, treat its absence as a “not found” error — as if the server had not returned it in the first place. Updates #34094 Change-Id: I142619ff43b96d0de428cdd0b01cca828c9ba234 Reviewed-on: https://go-review.googlesource.com/c/go/+/194561 Reviewed-by: Jay Conrod --- diff --git a/src/cmd/go/internal/modfetch/codehost/git.go b/src/cmd/go/internal/modfetch/codehost/git.go index d382e8ac9a..df895ec91b 100644 --- a/src/cmd/go/internal/modfetch/codehost/git.go +++ b/src/cmd/go/internal/modfetch/codehost/git.go @@ -6,9 +6,11 @@ package codehost import ( "bytes" + "errors" "fmt" "io" "io/ioutil" + "net/url" "os" "os/exec" "path/filepath" @@ -21,6 +23,7 @@ import ( "cmd/go/internal/lockedfile" "cmd/go/internal/par" "cmd/go/internal/semver" + "cmd/go/internal/web" ) // GitRepo returns the code repository at the given Git remote reference. @@ -34,6 +37,15 @@ func LocalGitRepo(remote string) (Repo, error) { return newGitRepoCached(remote, true) } +// A notExistError wraps another error to retain its original text +// but makes it opaquely equivalent to os.ErrNotExist. +type notExistError struct { + err error +} + +func (e notExistError) Error() string { return e.err.Error() } +func (notExistError) Is(err error) bool { return err == os.ErrNotExist } + const gitWorkDirType = "git3" var gitRepoCache par.Cache @@ -85,8 +97,9 @@ func newGitRepo(remote string, localOK bool) (Repo, error) { os.RemoveAll(r.dir) return nil, err } - r.remote = "origin" } + r.remoteURL = r.remote + r.remote = "origin" } else { // Local path. // Disallow colon (not in ://) because sometimes @@ -113,9 +126,9 @@ func newGitRepo(remote string, localOK bool) (Repo, error) { } type gitRepo struct { - remote string - local bool - dir string + remote, remoteURL string + local bool + dir string mu lockedfile.Mutex // protects fetchLevel and git repo state @@ -166,14 +179,25 @@ func (r *gitRepo) loadRefs() { // The git protocol sends all known refs and ls-remote filters them on the client side, // so we might as well record both heads and tags in one shot. // Most of the time we only care about tags but sometimes we care about heads too. - out, err := Run(r.dir, "git", "ls-remote", "-q", r.remote) - if err != nil { - if rerr, ok := err.(*RunError); ok { + out, gitErr := Run(r.dir, "git", "ls-remote", "-q", r.remote) + if gitErr != nil { + if rerr, ok := gitErr.(*RunError); ok { if bytes.Contains(rerr.Stderr, []byte("fatal: could not read Username")) { rerr.HelpText = "Confirm the import path was entered correctly.\nIf this is a private repository, see https://golang.org/doc/faq#git_https for additional information." } } - r.refsErr = err + + // If the remote URL doesn't exist at all, ideally we should treat the whole + // repository as nonexistent by wrapping the error in a notExistError. + // For HTTP and HTTPS, that's easy to detect: we'll try to fetch the URL + // ourselves and see what code it serves. + if u, err := url.Parse(r.remoteURL); err == nil && (u.Scheme == "http" || u.Scheme == "https") { + if _, err := web.GetBytes(u); errors.Is(err, os.ErrNotExist) { + gitErr = notExistError{gitErr} + } + } + + r.refsErr = gitErr return } diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go index 569ef3a57a..a3a27abf77 100644 --- a/src/cmd/go/internal/modfetch/proxy.go +++ b/src/cmd/go/internal/modfetch/proxy.go @@ -150,13 +150,28 @@ func TryProxies(f func(proxy string) error) error { return f("off") } + var lastAttemptErr error for _, proxy := range proxies { err = f(proxy) if !errors.Is(err, os.ErrNotExist) { + lastAttemptErr = err break } + + // The error indicates that the module does not exist. + // In general we prefer to report the last such error, + // because it indicates the error that occurs after all other + // options have been exhausted. + // + // However, for modules in the NOPROXY list, the most useful error occurs + // first (with proxy set to "noproxy"), and the subsequent errors are all + // errNoProxy (which is not particularly helpful). Do not overwrite a more + // useful error with errNoproxy. + if lastAttemptErr == nil || !errors.Is(err, errNoproxy) { + lastAttemptErr = err + } } - return err + return lastAttemptErr } type proxyRepo struct { diff --git a/src/cmd/go/testdata/script/mod_missing_repo.txt b/src/cmd/go/testdata/script/mod_missing_repo.txt new file mode 100644 index 0000000000..8dae85fa88 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_missing_repo.txt @@ -0,0 +1,15 @@ +# Regression test for golang.org/issue/34094: modules hosted within gitlab.com +# subgroups could not be fetched because the server returned bogus go-import +# tags for prefixes of the module path. + +[!net] skip +[!exec:git] skip + +env GO111MODULE=on +env GOPROXY=direct +env GOSUMDB=off + +! go get -d vcs-test.golang.org/go/missingrepo/missingrepo-git +stderr 'vcs-test.golang.org/go/missingrepo/missingrepo-git: git ls-remote .*: exit status .*' + +go get -d vcs-test.golang.org/go/missingrepo/missingrepo-git/notmissing