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 <art27@cantab.net>

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"`

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)
 }

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
+}

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)
+		})
+	}
+}

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
 	}
 
+	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
+	}
 	a.Address = u.String()
 
 	return a

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
 		}

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)

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)

templates/repo/header.tmpl

@@ -37,7 +37,9 @@
 						{{end}}
 					</div>
 				</div>
-				{{if .IsMirror}}<div class="fork-flag">{{$.i18n.Tr "repo.mirror_from"}} <a target="_blank" rel="noopener noreferrer" href="{{if .SanitizedOriginalURL}}{{.SanitizedOriginalURL}}{{else}}{{(MirrorRemoteAddress $.Context $.Mirror).Address}}{{end}}">{{if .SanitizedOriginalURL}}{{.SanitizedOriginalURL}}{{else}}{{(MirrorRemoteAddress $.Context $.Mirror).Address}}{{end}}</a></div>{{end}}
+				{{if .IsMirror}}
+				{{$address := MirrorRemoteAddress $.Context . $.Mirror.GetRemoteName}}
+				<div class="fork-flag">{{$.i18n.Tr "repo.mirror_from"}} <a target="_blank" rel="noopener noreferrer" href="{{$address.Address}}">{{$address.Address}}</a></div>{{end}}
 				{{if .IsFork}}<div class="fork-flag">{{$.i18n.Tr "repo.forked_from"}} <a href="{{.BaseRepo.Link}}">{{.BaseRepo.FullName}}</a></div>{{end}}
 				{{if .IsGenerated}}<div class="fork-flag">{{$.i18n.Tr "repo.generated_from"}} <a href="{{.TemplateRepo.Link}}">{{.TemplateRepo.FullName}}</a></div>{{end}}
 			</div>

templates/repo/settings/options.tmpl

@@ -91,7 +91,7 @@
 					{{if .Repository.IsMirror}}
 					<tbody>
 						<tr>
-							<td>{{(MirrorRemoteAddress $.Context .Mirror).Address}}</td>
+							<td>{{(MirrorRemoteAddress $.Context .Repository .Mirror.GetRemoteName).Address}}</td>
 							<td>{{$.i18n.Tr "repo.settings.mirror_settings.direction.pull"}}</td>
 							<td>{{.Mirror.UpdatedUnix.AsTime}}</td>
 							<td class="right aligned">
@@ -119,7 +119,7 @@
 										<label for="interval">{{.i18n.Tr "repo.mirror_interval" .MinimumMirrorInterval}}</label>
 										<input id="interval" name="interval" value="{{.MirrorInterval}}">
 									</div>
-									{{$address := MirrorRemoteAddress $.Context .Mirror}}
+									{{$address := MirrorRemoteAddress $.Context .Repository .Mirror.GetRemoteName}}
 									<div class="field {{if .Err_MirrorAddress}}error{{end}}">
 										<label for="mirror_address">{{.i18n.Tr "repo.mirror_address"}}</label>
 										<input id="mirror_address" name="mirror_address" value="{{$address.Address}}" required>
@@ -168,7 +168,7 @@
 					<tbody>
 						{{range .PushMirrors}}
 						<tr>
-							{{$address := MirrorRemoteAddress $.Context .}}
+							{{$address := MirrorRemoteAddress $.Context $.Repository .GetRemoteName}}
 							<td>{{$address.Address}}</td>
 							<td>{{$.i18n.Tr "repo.settings.mirror_settings.direction.push"}}</td>
 							<td>{{if .LastUpdateUnix}}{{.LastUpdateUnix.AsTime}}{{else}}{{$.i18n.Tr "never"}}{{end}} {{if .LastError}}<div class="ui red label tooltip" data-content="{{.LastError}}">{{$.i18n.Tr "error"}}</div>{{end}}</td>