]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/modfetch/codehost: treat nonexistent repositories as “not found”
authorBryan C. Mills <bcmills@google.com>
Thu, 5 Sep 2019 18:27:27 +0000 (14:27 -0400)
committerBryan C. Mills <bcmills@google.com>
Wed, 11 Sep 2019 14:02:43 +0000 (14:02 +0000)
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 <jayconrod@google.com>
src/cmd/go/internal/modfetch/codehost/git.go
src/cmd/go/internal/modfetch/proxy.go
src/cmd/go/testdata/script/mod_missing_repo.txt [new file with mode: 0644]

index d382e8ac9a9232c7ff5f7ac006eb39918ccabf66..df895ec91bed7ad2d75613f6934b441b471f387b 100644 (file)
@@ -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
        }
 
index 569ef3a57a60a39682d00c8a38b0c54113307cca..a3a27abf77d2d9c99bd0e2f41792f0a6af170a07 100644 (file)
@@ -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 (file)
index 0000000..8dae85f
--- /dev/null
@@ -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