From ce3dd04c63a048fe791ed864c2023fd37b09e427 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 11 Jun 2022 21:50:14 +0800 Subject: [PATCH] Fix some mirror bugs (#18649) * Fix some mirror bugs * Remove unnecessary code * Fix lint * rename stdard url * Allow more charactors in git ssh protocol url * improve the detection * support ipv6 for git url parse * Fix bug * Fix template * Fix bug * fix template * Fix tmpl * Fix tmpl * Fix parse ssh with interface * Rename functions name Co-authored-by: zeripath --- models/repo/mirror.go | 6 - modules/git/remote.go | 20 +++- modules/git/url/url.go | 90 +++++++++++++++ modules/git/url/url_test.go | 167 +++++++++++++++++++++++++++ modules/templates/helper.go | 34 ++++-- routers/web/repo/setting.go | 14 ++- services/mirror/mirror_pull.go | 5 +- services/mirror/mirror_push.go | 4 +- templates/repo/header.tmpl | 4 +- templates/repo/settings/options.tmpl | 6 +- 10 files changed, 316 insertions(+), 34 deletions(-) create mode 100644 modules/git/url/url.go create mode 100644 modules/git/url/url_test.go diff --git a/models/repo/mirror.go b/models/repo/mirror.go index bd83d2442..8f96e8cee 100644 --- a/models/repo/mirror.go +++ b/models/repo/mirror.go @@ -19,12 +19,6 @@ import ( // ErrMirrorNotExist mirror does not exist error var ErrMirrorNotExist = errors.New("Mirror does not exist") -// RemoteMirrorer defines base methods for pull/push mirrors. -type RemoteMirrorer interface { - GetRepository() *Repository - GetRemoteName() string -} - // Mirror represents mirror information of a repository. type Mirror struct { ID int64 `xorm:"pk autoincr"` diff --git a/modules/git/remote.go b/modules/git/remote.go index b2a2e6d7a..cbb4ac612 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -6,11 +6,12 @@ package git import ( "context" - "net/url" + + giturl "code.gitea.io/gitea/modules/git/url" ) -// GetRemoteAddress returns the url of a specific remote of the repository. -func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (*url.URL, error) { +// GetRemoteAddress returns remote url of git repository in the repoPath with special remote name +func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (string, error) { var cmd *Command if CheckGitVersionAtLeast("2.7") == nil { cmd = NewCommand(ctx, "remote", "get-url", remoteName) @@ -20,11 +21,20 @@ func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (*url.UR result, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) if err != nil { - return nil, err + return "", err } if len(result) > 0 { result = result[:len(result)-1] } - return url.Parse(result) + return result, nil +} + +// GetRemoteURL returns the url of a specific remote of the repository. +func GetRemoteURL(ctx context.Context, repoPath, remoteName string) (*giturl.GitURL, error) { + addr, err := GetRemoteAddress(ctx, repoPath, remoteName) + if err != nil { + return nil, err + } + return giturl.Parse(addr) } diff --git a/modules/git/url/url.go b/modules/git/url/url.go new file mode 100644 index 000000000..b41cfab7e --- /dev/null +++ b/modules/git/url/url.go @@ -0,0 +1,90 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package url + +import ( + "fmt" + stdurl "net/url" + "strings" +) + +// ErrWrongURLFormat represents an error with wrong url format +type ErrWrongURLFormat struct { + URL string +} + +func (err ErrWrongURLFormat) Error() string { + return fmt.Sprintf("git URL %s format is wrong", err.URL) +} + +// GitURL represents a git URL +type GitURL struct { + *stdurl.URL + extraMark int // 0 no extra 1 scp 2 file path with no prefix +} + +// String returns the URL's string +func (u *GitURL) String() string { + switch u.extraMark { + case 0: + return u.URL.String() + case 1: + return fmt.Sprintf("%s@%s:%s", u.User.Username(), u.Host, u.Path) + case 2: + return u.Path + default: + return "" + } +} + +// Parse parse all kinds of git URL +func Parse(remote string) (*GitURL, error) { + if strings.Contains(remote, "://") { + u, err := stdurl.Parse(remote) + if err != nil { + return nil, err + } + return &GitURL{URL: u}, nil + } else if strings.Contains(remote, "@") && strings.Contains(remote, ":") { + url := stdurl.URL{ + Scheme: "ssh", + } + squareBrackets := false + lastIndex := -1 + FOR: + for i := 0; i < len(remote); i++ { + switch remote[i] { + case '@': + url.User = stdurl.User(remote[:i]) + lastIndex = i + 1 + case ':': + if !squareBrackets { + url.Host = strings.ReplaceAll(remote[lastIndex:i], "%25", "%") + if len(remote) <= i+1 { + return nil, ErrWrongURLFormat{URL: remote} + } + url.Path = remote[i+1:] + break FOR + } + case '[': + squareBrackets = true + case ']': + squareBrackets = false + } + } + return &GitURL{ + URL: &url, + extraMark: 1, + }, nil + } + + return &GitURL{ + URL: &stdurl.URL{ + Scheme: "file", + Path: remote, + }, + extraMark: 2, + }, nil +} diff --git a/modules/git/url/url_test.go b/modules/git/url/url_test.go new file mode 100644 index 000000000..611bef867 --- /dev/null +++ b/modules/git/url/url_test.go @@ -0,0 +1,167 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package url + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseGitURLs(t *testing.T) { + kases := []struct { + kase string + expected *GitURL + }{ + { + kase: "git@127.0.0.1:go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "127.0.0.1", + Path: "go-gitea/gitea.git", + }, + extraMark: 1, + }, + }, + { + kase: "git@[fe80:14fc:cec5:c174:d88%2510]:go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "[fe80:14fc:cec5:c174:d88%10]", + Path: "go-gitea/gitea.git", + }, + extraMark: 1, + }, + }, + { + kase: "git@[::1]:go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "[::1]", + Path: "go-gitea/gitea.git", + }, + extraMark: 1, + }, + }, + { + kase: "git@github.com:go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "github.com", + Path: "go-gitea/gitea.git", + }, + extraMark: 1, + }, + }, + { + kase: "ssh://git@github.com/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "github.com", + Path: "/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + { + kase: "ssh://git@[::1]/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "[::1]", + Path: "/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + { + kase: "/repositories/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "file", + Path: "/repositories/go-gitea/gitea.git", + }, + extraMark: 2, + }, + }, + { + kase: "file:///repositories/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "file", + Path: "/repositories/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + { + kase: "https://github.com/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "https", + Host: "github.com", + Path: "/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + { + kase: "https://git:git@github.com/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "https", + Host: "github.com", + User: url.UserPassword("git", "git"), + Path: "/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + { + kase: "https://[fe80:14fc:cec5:c174:d88%2510]:20/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "https", + Host: "[fe80:14fc:cec5:c174:d88%10]:20", + Path: "/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + + { + kase: "git://github.com/go-gitea/gitea.git", + expected: &GitURL{ + URL: &url.URL{ + Scheme: "git", + Host: "github.com", + Path: "/go-gitea/gitea.git", + }, + extraMark: 0, + }, + }, + } + + for _, kase := range kases { + t.Run(kase.kase, func(t *testing.T) { + u, err := Parse(kase.kase) + assert.NoError(t, err) + assert.EqualValues(t, kase.expected.extraMark, u.extraMark) + assert.EqualValues(t, *kase.expected, *u) + }) + } +} diff --git a/modules/templates/helper.go b/modules/templates/helper.go index ef7b70c09..03e0e9899 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -32,6 +32,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/emoji" "code.gitea.io/gitea/modules/git" + giturl "code.gitea.io/gitea/modules/git/url" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" @@ -971,20 +972,35 @@ type remoteAddress struct { Password string } -func mirrorRemoteAddress(ctx context.Context, m repo_model.RemoteMirrorer) remoteAddress { +func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress { a := remoteAddress{} - - u, err := git.GetRemoteAddress(ctx, m.GetRepository().RepoPath(), m.GetRemoteName()) - if err != nil { - log.Error("GetRemoteAddress %v", err) + if !m.IsMirror { return a } - if u.User != nil { - a.Username = u.User.Username() - a.Password, _ = u.User.Password() + remoteURL := m.OriginalURL + if remoteURL == "" { + var err error + remoteURL, err = git.GetRemoteAddress(ctx, m.RepoPath(), remoteName) + if err != nil { + log.Error("GetRemoteURL %v", err) + return a + } + } + + u, err := giturl.Parse(remoteURL) + if err != nil { + log.Error("giturl.Parse %v", err) + return a + } + + if u.Scheme != "ssh" && u.Scheme != "file" { + if u.User != nil { + a.Username = u.User.Username() + a.Password, _ = u.User.Password() + } + u.User = nil } - u.User = nil a.Address = u.String() return a diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index f49ef6e85..6083d17fa 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -215,22 +215,24 @@ func SettingsPost(ctx *context.Context) { return } - u, _ := git.GetRemoteAddress(ctx, ctx.Repo.Repository.RepoPath(), ctx.Repo.Mirror.GetRemoteName()) + u, err := git.GetRemoteURL(ctx, ctx.Repo.Repository.RepoPath(), ctx.Repo.Mirror.GetRemoteName()) + if err != nil { + ctx.Data["Err_MirrorAddress"] = true + handleSettingRemoteAddrError(ctx, err, form) + return + } if u.User != nil && form.MirrorPassword == "" && form.MirrorUsername == u.User.Username() { form.MirrorPassword, _ = u.User.Password() } - address, err := forms.ParseRemoteAddr(form.MirrorAddress, form.MirrorUsername, form.MirrorPassword) - if err == nil { - err = migrations.IsMigrateURLAllowed(address, ctx.Doer) - } + err = migrations.IsMigrateURLAllowed(u.String(), ctx.Doer) if err != nil { ctx.Data["Err_MirrorAddress"] = true handleSettingRemoteAddrError(ctx, err, form) return } - if err := mirror_service.UpdateAddress(ctx, ctx.Repo.Mirror, address); err != nil { + if err := mirror_service.UpdateAddress(ctx, ctx.Repo.Mirror, u.String()); err != nil { ctx.ServerError("UpdateAddress", err) return } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index caa81f0fe..f4c527bbd 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -210,9 +210,10 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } gitArgs = append(gitArgs, m.GetRemoteName()) - remoteAddr, remoteErr := git.GetRemoteAddress(ctx, repoPath, m.GetRemoteName()) + remoteURL, remoteErr := git.GetRemoteURL(ctx, repoPath, m.GetRemoteName()) if remoteErr != nil { log.Error("SyncMirrors [repo: %-v]: GetRemoteAddress Error %v", m.Repo, remoteErr) + return nil, false } stdoutBuilder := strings.Builder{} @@ -291,7 +292,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo if m.LFS && setting.LFS.StartServer { log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) - endpoint := lfs.DetermineEndpoint(remoteAddr.String(), m.LFSEndpoint) + endpoint := lfs.DetermineEndpoint(remoteURL.String(), m.LFSEndpoint) lfsClient := lfs.NewClient(endpoint, nil) if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { log.Error("SyncMirrors [repo: %-v]: failed to synchronize LFS objects for repository: %v", m.Repo, err) diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 138ebb737..2927bed72 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -131,7 +131,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { timeout := time.Duration(setting.Git.Timeout.Mirror) * time.Second performPush := func(path string) error { - remoteAddr, err := git.GetRemoteAddress(ctx, path, m.RemoteName) + remoteURL, err := git.GetRemoteURL(ctx, path, m.RemoteName) if err != nil { log.Error("GetRemoteAddress(%s) Error %v", path, err) return errors.New("Unexpected error") @@ -147,7 +147,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { } defer gitRepo.Close() - endpoint := lfs.DetermineEndpoint(remoteAddr.String(), "") + endpoint := lfs.DetermineEndpoint(remoteURL.String(), "") lfsClient := lfs.NewClient(endpoint, nil) if err := pushAllLFSObjects(ctx, gitRepo, lfsClient); err != nil { return util.SanitizeErrorCredentialURLs(err) diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 2d963d67c..cfac37cd1 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -37,7 +37,9 @@ {{end}} - {{if .IsMirror}}
{{$.i18n.Tr "repo.mirror_from"}} {{if .SanitizedOriginalURL}}{{.SanitizedOriginalURL}}{{else}}{{(MirrorRemoteAddress $.Context $.Mirror).Address}}{{end}}
{{end}} + {{if .IsMirror}} + {{$address := MirrorRemoteAddress $.Context . $.Mirror.GetRemoteName}} +
{{$.i18n.Tr "repo.mirror_from"}} {{$address.Address}}
{{end}} {{if .IsFork}}
{{$.i18n.Tr "repo.forked_from"}} {{.BaseRepo.FullName}}
{{end}} {{if .IsGenerated}}
{{$.i18n.Tr "repo.generated_from"}} {{.TemplateRepo.FullName}}
{{end}} diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 67a98aff4..68cbd4de2 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -91,7 +91,7 @@ {{if .Repository.IsMirror}} - {{(MirrorRemoteAddress $.Context .Mirror).Address}} + {{(MirrorRemoteAddress $.Context .Repository .Mirror.GetRemoteName).Address}} {{$.i18n.Tr "repo.settings.mirror_settings.direction.pull"}} {{.Mirror.UpdatedUnix.AsTime}} @@ -119,7 +119,7 @@ - {{$address := MirrorRemoteAddress $.Context .Mirror}} + {{$address := MirrorRemoteAddress $.Context .Repository .Mirror.GetRemoteName}}
@@ -168,7 +168,7 @@ {{range .PushMirrors}} - {{$address := MirrorRemoteAddress $.Context .}} + {{$address := MirrorRemoteAddress $.Context $.Repository .GetRemoteName}} {{$address.Address}} {{$.i18n.Tr "repo.settings.mirror_settings.direction.push"}} {{if .LastUpdateUnix}}{{.LastUpdateUnix.AsTime}}{{else}}{{$.i18n.Tr "never"}}{{end}} {{if .LastError}}
{{$.i18n.Tr "error"}}
{{end}}