From 864f095c43a3bb73d56c30eadc704dddc956178c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 12 Apr 2024 12:00:43 +0800 Subject: [PATCH 1/5] fix: mirrorRemoteAddress --- modules/templates/util_misc.go | 32 ++++++++++------------------ templates/repo/settings/options.tmpl | 2 +- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/modules/templates/util_misc.go b/modules/templates/util_misc.go index 6c1b4ab240e34..2d2f48fe6de56 100644 --- a/modules/templates/util_misc.go +++ b/modules/templates/util_misc.go @@ -142,33 +142,23 @@ type remoteAddress struct { Password string } -func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string, ignoreOriginalURL bool) remoteAddress { +func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress { a := remoteAddress{} - remoteURL := m.OriginalURL - if ignoreOriginalURL || 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 { + // the URL stored in the git repo which contains authentication + if remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName); err != nil { + log.Error("GetRemoteURL %v", err) + return a + } else if u, err := giturl.Parse(remoteURL); err != nil { log.Error("giturl.Parse %v", err) return a + } else if u.User != nil { + a.Username = u.User.Username() + a.Password, _ = u.User.Password() } - if u.Scheme != "ssh" && u.Scheme != "file" { - if u.User != nil { - a.Username = u.User.Username() - a.Password, _ = u.User.Password() - } - u.User = nil - } - a.Address = u.String() + // the URL stored in the database which doesn't contain authentication + a.Address = m.OriginalURL return a } diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index b8fa4759b1326..df6ccbf6bc2fb 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -156,7 +156,7 @@ - {{$address := MirrorRemoteAddress $.Context .Repository .PullMirror.GetRemoteName false}} + {{$address := MirrorRemoteAddress $.Context .Repository .PullMirror.GetRemoteName}}
From 750b1fd84f8f480b6839d2e125541d1cd716171d Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 12 Apr 2024 12:22:50 +0800 Subject: [PATCH 2/5] fix: erase authentication before storing in database --- services/mirror/mirror_pull.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 2a38d4ba55d3a..4a9b5c268b6bd 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -6,6 +6,7 @@ package mirror import ( "context" "fmt" + "net/url" "strings" "time" @@ -30,10 +31,15 @@ const gitShortEmptySha = "0000000" // UpdateAddress writes new address to Git repository and database func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error { + u, err := url.Parse(addr) + if err != nil { + return fmt.Errorf("invalid addr: %v", err) + } + remoteName := m.GetRemoteName() repoPath := m.GetRepository(ctx).RepoPath() // Remove old remote - _, _, err := git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: repoPath}) + _, _, err = git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return err } @@ -70,7 +76,9 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error } } - m.Repo.OriginalURL = addr + // erase authentication before storing in database + u.User = nil + m.Repo.OriginalURL = u.String() return repo_model.UpdateRepositoryCols(ctx, m.Repo, "original_url") } From bb78637b1cd565b166728368f0f68798f88e3815 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 12 Apr 2024 13:18:28 +0800 Subject: [PATCH 3/5] fix: mirrorRemoteAddress --- modules/templates/util_misc.go | 36 ++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/modules/templates/util_misc.go b/modules/templates/util_misc.go index 2d2f48fe6de56..d5e85af2dca3d 100644 --- a/modules/templates/util_misc.go +++ b/modules/templates/util_misc.go @@ -143,24 +143,34 @@ type remoteAddress struct { } func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress { - a := remoteAddress{} - - // the URL stored in the git repo which contains authentication - if remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName); err != nil { + ret := remoteAddress{} + remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName) + if err != nil { log.Error("GetRemoteURL %v", err) - return a - } else if u, err := giturl.Parse(remoteURL); err != nil { + return ret + } + + u, err := giturl.Parse(remoteURL) + if err != nil { log.Error("giturl.Parse %v", err) - return a + return ret } else if u.User != nil { - a.Username = u.User.Username() - a.Password, _ = u.User.Password() + ret.Username = u.User.Username() + ret.Password, _ = u.User.Password() } - // the URL stored in the database which doesn't contain authentication - a.Address = m.OriginalURL - - return a + // The URL stored in the git repo could contain authentication, + // erase it, or it will be shown in the UI. + u.User = nil + ret.Address = u.String() + // Why not use m.OriginalURL to set ret.Address? + // It should be OK to use it, since m.OriginalURL should be the same as the authentication-erased URL from the Git repository. + // However, the old code has already stored authentication in m.OriginalURL when updating mirror settings. + // That means we need to use "giturl.Parse" for m.OriginalURL again to ensure authentication is erased. + // Instead of doing this, why not directly use the authentication-erased URL from the Git repository? + // It should be the same as long as there are no bugs. + + return ret } func FilenameIsImage(filename string) bool { From 06414484470b2767fd98d800b8a4e8281098f0fa Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 12 Apr 2024 13:23:18 +0800 Subject: [PATCH 4/5] fix: use giturl.Parse --- services/mirror/mirror_pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 4a9b5c268b6bd..cc1ac47d02b4c 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -6,7 +6,6 @@ package mirror import ( "context" "fmt" - "net/url" "strings" "time" @@ -14,6 +13,7 @@ import ( system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" + giturl "code.gitea.io/gitea/modules/git/url" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" @@ -31,7 +31,7 @@ const gitShortEmptySha = "0000000" // UpdateAddress writes new address to Git repository and database func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error { - u, err := url.Parse(addr) + u, err := giturl.Parse(addr) if err != nil { return fmt.Errorf("invalid addr: %v", err) } From 7c666f0ee0fa3bc9527013a4e28ddad2f3a4bb25 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 12 Apr 2024 13:33:38 +0800 Subject: [PATCH 5/5] fix: update mirrorRemoteAddress --- modules/templates/util_misc.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/modules/templates/util_misc.go b/modules/templates/util_misc.go index d5e85af2dca3d..774385483b4c6 100644 --- a/modules/templates/util_misc.go +++ b/modules/templates/util_misc.go @@ -154,9 +154,13 @@ func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteNa if err != nil { log.Error("giturl.Parse %v", err) return ret - } else if u.User != nil { - ret.Username = u.User.Username() - ret.Password, _ = u.User.Password() + } + + if u.Scheme != "ssh" && u.Scheme != "file" { + if u.User != nil { + ret.Username = u.User.Username() + ret.Password, _ = u.User.Password() + } } // The URL stored in the git repo could contain authentication,