Skip to content

Commit f6c691e

Browse files
author
Bryan C. Mills
committed
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 <[email protected]>
1 parent 0e015e2 commit f6c691e

File tree

3 files changed

+63
-9
lines changed

3 files changed

+63
-9
lines changed

src/cmd/go/internal/modfetch/codehost/git.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ package codehost
66

77
import (
88
"bytes"
9+
"errors"
910
"fmt"
1011
"io"
1112
"io/ioutil"
13+
"net/url"
1214
"os"
1315
"os/exec"
1416
"path/filepath"
@@ -21,6 +23,7 @@ import (
2123
"cmd/go/internal/lockedfile"
2224
"cmd/go/internal/par"
2325
"cmd/go/internal/semver"
26+
"cmd/go/internal/web"
2427
)
2528

2629
// GitRepo returns the code repository at the given Git remote reference.
@@ -34,6 +37,15 @@ func LocalGitRepo(remote string) (Repo, error) {
3437
return newGitRepoCached(remote, true)
3538
}
3639

40+
// A notExistError wraps another error to retain its original text
41+
// but makes it opaquely equivalent to os.ErrNotExist.
42+
type notExistError struct {
43+
err error
44+
}
45+
46+
func (e notExistError) Error() string { return e.err.Error() }
47+
func (notExistError) Is(err error) bool { return err == os.ErrNotExist }
48+
3749
const gitWorkDirType = "git3"
3850

3951
var gitRepoCache par.Cache
@@ -85,8 +97,9 @@ func newGitRepo(remote string, localOK bool) (Repo, error) {
8597
os.RemoveAll(r.dir)
8698
return nil, err
8799
}
88-
r.remote = "origin"
89100
}
101+
r.remoteURL = r.remote
102+
r.remote = "origin"
90103
} else {
91104
// Local path.
92105
// Disallow colon (not in ://) because sometimes
@@ -113,9 +126,9 @@ func newGitRepo(remote string, localOK bool) (Repo, error) {
113126
}
114127

115128
type gitRepo struct {
116-
remote string
117-
local bool
118-
dir string
129+
remote, remoteURL string
130+
local bool
131+
dir string
119132

120133
mu lockedfile.Mutex // protects fetchLevel and git repo state
121134

@@ -166,14 +179,25 @@ func (r *gitRepo) loadRefs() {
166179
// The git protocol sends all known refs and ls-remote filters them on the client side,
167180
// so we might as well record both heads and tags in one shot.
168181
// Most of the time we only care about tags but sometimes we care about heads too.
169-
out, err := Run(r.dir, "git", "ls-remote", "-q", r.remote)
170-
if err != nil {
171-
if rerr, ok := err.(*RunError); ok {
182+
out, gitErr := Run(r.dir, "git", "ls-remote", "-q", r.remote)
183+
if gitErr != nil {
184+
if rerr, ok := gitErr.(*RunError); ok {
172185
if bytes.Contains(rerr.Stderr, []byte("fatal: could not read Username")) {
173186
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."
174187
}
175188
}
176-
r.refsErr = err
189+
190+
// If the remote URL doesn't exist at all, ideally we should treat the whole
191+
// repository as nonexistent by wrapping the error in a notExistError.
192+
// For HTTP and HTTPS, that's easy to detect: we'll try to fetch the URL
193+
// ourselves and see what code it serves.
194+
if u, err := url.Parse(r.remoteURL); err == nil && (u.Scheme == "http" || u.Scheme == "https") {
195+
if _, err := web.GetBytes(u); errors.Is(err, os.ErrNotExist) {
196+
gitErr = notExistError{gitErr}
197+
}
198+
}
199+
200+
r.refsErr = gitErr
177201
return
178202
}
179203

src/cmd/go/internal/modfetch/proxy.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,28 @@ func TryProxies(f func(proxy string) error) error {
150150
return f("off")
151151
}
152152

153+
var lastAttemptErr error
153154
for _, proxy := range proxies {
154155
err = f(proxy)
155156
if !errors.Is(err, os.ErrNotExist) {
157+
lastAttemptErr = err
156158
break
157159
}
160+
161+
// The error indicates that the module does not exist.
162+
// In general we prefer to report the last such error,
163+
// because it indicates the error that occurs after all other
164+
// options have been exhausted.
165+
//
166+
// However, for modules in the NOPROXY list, the most useful error occurs
167+
// first (with proxy set to "noproxy"), and the subsequent errors are all
168+
// errNoProxy (which is not particularly helpful). Do not overwrite a more
169+
// useful error with errNoproxy.
170+
if lastAttemptErr == nil || !errors.Is(err, errNoproxy) {
171+
lastAttemptErr = err
172+
}
158173
}
159-
return err
174+
return lastAttemptErr
160175
}
161176

162177
type proxyRepo struct {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Regression test for golang.org/issue/34094: modules hosted within gitlab.com
2+
# subgroups could not be fetched because the server returned bogus go-import
3+
# tags for prefixes of the module path.
4+
5+
[!net] skip
6+
[!exec:git] skip
7+
8+
env GO111MODULE=on
9+
env GOPROXY=direct
10+
env GOSUMDB=off
11+
12+
! go get -d vcs-test.golang.org/go/missingrepo/missingrepo-git
13+
stderr 'vcs-test.golang.org/go/missingrepo/missingrepo-git: git ls-remote .*: exit status .*'
14+
15+
go get -d vcs-test.golang.org/go/missingrepo/missingrepo-git/notmissing

0 commit comments

Comments
 (0)