diff --git a/cmd/zoekt-mirror-gerrit/main.go b/cmd/zoekt-mirror-gerrit/main.go index 4a9b513d4..5b441c031 100644 --- a/cmd/zoekt-mirror-gerrit/main.go +++ b/cmd/zoekt-mirror-gerrit/main.go @@ -18,6 +18,7 @@ package main import ( "bytes" "context" + "errors" "flag" "fmt" "io" @@ -113,10 +114,13 @@ func main() { if *httpCrendentialsPath != "" { creds, err := os.ReadFile(*httpCrendentialsPath) if err != nil { - log.Print("Cannot read gerrit http credentials, going Anonymous") + log.Printf("Cannot read gerrit http credentials (%v), going Anonymous", err) } else { - splitCreds := strings.Split(strings.TrimSpace(string(creds)), ":") - rootURL.User = url.UserPassword(splitCreds[0], splitCreds[1]) + user, err := parseHTTPCredentials(string(creds)) + if err != nil { + log.Fatalf("%s: %v", *httpCrendentialsPath, err) + } + rootURL.User = user } } @@ -141,16 +145,7 @@ func main() { log.Fatalf("GetServerInfo: %v", err) } - var projectURL string - for _, s := range []string{"http", "anonymous http"} { - if schemeInfo, ok := info.Download.Schemes[s]; ok { - projectURL = schemeInfo.URL - if s == "http" && schemeInfo.IsAuthRequired { - projectURL = addPassword(projectURL, rootURL.User) - } - break - } - } + projectURL, needsAuth := selectProjectURL(info) if projectURL == "" { log.Fatalf("project URL is empty, got Schemes %#v", info.Download.Schemes) } @@ -180,9 +175,9 @@ func main() { continue } - cloneURL, err := url.Parse(strings.Replace(projectURL, "${project}", k, 1)) + cloneURL, err := buildCloneURL(projectURL, k, needsAuth, rootURL.User) if err != nil { - log.Fatalf("url.Parse: %v", err) + log.Fatalf("buildCloneURL: %v", err) } name := filepath.Join(cloneURL.Host, cloneURL.Path) @@ -267,10 +262,34 @@ func anonymousURL(u *url.URL) string { return anon.String() } -func addPassword(u string, user *url.Userinfo) string { - password, _ := user.Password() - username := user.Username() - return strings.Replace(u, fmt.Sprintf("://%s@", username), fmt.Sprintf("://%s:%s@", username, password), 1) +func selectProjectURL(info *gerrit.ServerInfo) (string, bool) { + for _, s := range []string{"http", "anonymous http"} { + if schemeInfo, ok := info.Download.Schemes[s]; ok { + return schemeInfo.URL, s == "http" && schemeInfo.IsAuthRequired + } + } + return "", false +} + +func buildCloneURL(projectURL, project string, needsAuth bool, user *url.Userinfo) (*url.URL, error) { + u, err := url.Parse(strings.Replace(projectURL, "${project}", project, 1)) + if err != nil { + return nil, err + } + // Leave the download scheme's own userinfo alone when no credentials were + // configured: git can still resolve them via a credential helper or .netrc. + if needsAuth && user != nil { + u.User = user + } + return u, nil +} + +func parseHTTPCredentials(creds string) (*url.Userinfo, error) { + split := strings.SplitN(strings.TrimSpace(creds), ":", 2) + if len(split) != 2 { + return nil, errors.New("expected format 'username:password'") + } + return url.UserPassword(split[0], split[1]), nil } func addMetaConfigFetch(repoDir string) error { diff --git a/cmd/zoekt-mirror-gerrit/main_test.go b/cmd/zoekt-mirror-gerrit/main_test.go new file mode 100644 index 000000000..24544e48d --- /dev/null +++ b/cmd/zoekt-mirror-gerrit/main_test.go @@ -0,0 +1,123 @@ +package main + +import ( + "net/url" + "testing" + + gerrit "github.com/andygrunwald/go-gerrit" +) + +func TestSelectProjectURL(t *testing.T) { + httpScheme := gerrit.DownloadSchemeInfo{URL: "https://admin@gerrit.example.com/a/${project}"} + anonScheme := gerrit.DownloadSchemeInfo{URL: "https://gerrit.example.com/a/${project}"} + + for _, tc := range []struct { + name string + schemes map[string]gerrit.DownloadSchemeInfo + wantURL string + wantNeedsAuth bool + }{ + {name: "http auth required", schemes: map[string]gerrit.DownloadSchemeInfo{"http": {URL: httpScheme.URL, IsAuthRequired: true}}, wantURL: httpScheme.URL, wantNeedsAuth: true}, + {name: "http no auth required", schemes: map[string]gerrit.DownloadSchemeInfo{"http": {URL: httpScheme.URL}}, wantURL: httpScheme.URL}, + {name: "anonymous http only", schemes: map[string]gerrit.DownloadSchemeInfo{"anonymous http": anonScheme}, wantURL: anonScheme.URL}, + {name: "no supported scheme", schemes: map[string]gerrit.DownloadSchemeInfo{"ssh": anonScheme}}, + } { + t.Run(tc.name, func(t *testing.T) { + info := &gerrit.ServerInfo{Download: gerrit.DownloadInfo{Schemes: tc.schemes}} + gotURL, gotNeedsAuth := selectProjectURL(info) + if gotURL != tc.wantURL || gotNeedsAuth != tc.wantNeedsAuth { + t.Fatalf("selectProjectURL() = (%q, %v), want (%q, %v)", gotURL, gotNeedsAuth, tc.wantURL, tc.wantNeedsAuth) + } + }) + } +} + +// Regression for the reported bug: passwords containing '/' previously broke +// url.Parse because they were spliced into the raw URL string. +func TestBuildCloneURLRoundTripsPasswords(t *testing.T) { + const template = "https://admin@gerrit.example.com/a/${project}" + for _, pw := range []string{"secret", "abc/def", "ZOSOKjgV/kgEkN0bzPJp+oGeJLqpXykqWFJpon/Ckg", "p@ss#1?x"} { + t.Run("password "+pw, func(t *testing.T) { + u, err := buildCloneURL(template, "legacy/modules", true, url.UserPassword("admin", pw)) + if err != nil { + t.Fatalf("buildCloneURL: %v", err) + } + + // git is handed cloneURL.String(), so assert on the serialized form + // rather than on the fields buildCloneURL just assigned. + got, err := url.Parse(u.String()) + if err != nil { + t.Fatalf("url.Parse(%q): %v", u.String(), err) + } + if pass, ok := got.User.Password(); !ok || pass != pw { + t.Fatalf("round-trip password = (%q, %v), want (%q, true)", pass, ok, pw) + } + if name := got.User.Username(); name != "admin" { + t.Fatalf("round-trip username = %q, want \"admin\"", name) + } + if got.Host != "gerrit.example.com" || got.Path != "/a/legacy/modules" { + t.Fatalf("host/path changed: %q/%q", got.Host, got.Path) + } + }) + } +} + +func TestBuildCloneURLAnonymous(t *testing.T) { + u, err := buildCloneURL("https://gerrit.example.com/a/${project}", "p", false, nil) + if err != nil { + t.Fatal(err) + } + if u.User != nil { + t.Fatalf("user = %v, want nil", u.User) + } +} + +// Without --http-credentials rootURL.User is nil. The clone URL must keep the +// username Gerrit put in the download scheme so git can still resolve the +// password via a credential helper or .netrc. +func TestBuildCloneURLKeepsSchemeUserWithoutCredentials(t *testing.T) { + u, err := buildCloneURL("https://admin@gerrit.example.com/a/${project}", "legacy/modules", true, nil) + if err != nil { + t.Fatalf("buildCloneURL: %v", err) + } + if u.User == nil || u.User.Username() != "admin" { + t.Fatalf("user = %v, want admin", u.User) + } + if _, ok := u.User.Password(); ok { + t.Fatalf("password set, want none: %q", u.String()) + } +} + +func TestParseHTTPCredentials(t *testing.T) { + for _, tc := range []struct { + name string + creds string + wantUser string + wantPass string + wantError bool + }{ + {name: "simple", creds: "admin:secret", wantUser: "admin", wantPass: "secret"}, + {name: "password containing colon", creds: "admin:pa:ss", wantUser: "admin", wantPass: "pa:ss"}, + {name: "password containing slash", creds: "admin:abc/def", wantUser: "admin", wantPass: "abc/def"}, + {name: "surrounding whitespace", creds: "\n admin:pa:ss\n", wantUser: "admin", wantPass: "pa:ss"}, + {name: "empty password", creds: "admin:", wantUser: "admin", wantPass: ""}, + {name: "no colon", creds: "adminonly", wantError: true}, + } { + t.Run(tc.name, func(t *testing.T) { + user, err := parseHTTPCredentials(tc.creds) + if tc.wantError { + if err == nil { + t.Fatalf("parseHTTPCredentials(%q) succeeded, want error", tc.creds) + } + return + } + if err != nil { + t.Fatalf("parseHTTPCredentials(%q): %v", tc.creds, err) + } + pass, ok := user.Password() + if user.Username() != tc.wantUser || !ok || pass != tc.wantPass { + t.Fatalf("got (%q, %q, %v), want (%q, %q, true)", user.Username(), pass, ok, tc.wantUser, tc.wantPass) + } + }) + } +}