From 4f04ab25867ee1b4f20241798bf90a588083ec6b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Dec 2023 14:18:12 +0800 Subject: [PATCH] Refactor SSH clone URL generation code (#28421) Refactor the code and add tests, keep the old logic. --- models/repo/repo.go | 24 +++++++++++------------- models/repo/repo_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 59f68df996..f739ada307 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -606,25 +606,23 @@ func ComposeHTTPSCloneURL(owner, repo string) string { func ComposeSSHCloneURL(ownerName, repoName string) string { sshUser := setting.SSH.User - - // if we have a ipv6 literal we need to put brackets around it - // for the git cloning to work. sshDomain := setting.SSH.Domain - ip := net.ParseIP(setting.SSH.Domain) - if ip != nil && ip.To4() == nil { - sshDomain = "[" + setting.SSH.Domain + "]" + + // non-standard port, it must use full URI + if setting.SSH.Port != 22 { + sshHost := net.JoinHostPort(sshDomain, strconv.Itoa(setting.SSH.Port)) + return fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, sshHost, url.PathEscape(ownerName), url.PathEscape(repoName)) } - if setting.SSH.Port != 22 { - return fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, - net.JoinHostPort(setting.SSH.Domain, strconv.Itoa(setting.SSH.Port)), - url.PathEscape(ownerName), - url.PathEscape(repoName)) + // for standard port, it can use a shorter URI (without the port) + sshHost := sshDomain + if ip := net.ParseIP(sshHost); ip != nil && ip.To4() == nil { + sshHost = "[" + sshHost + "]" // for IPv6 address, wrap it with brackets } if setting.Repository.UseCompatSSHURI { - return fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, sshDomain, url.PathEscape(ownerName), url.PathEscape(repoName)) + return fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, sshHost, url.PathEscape(ownerName), url.PathEscape(repoName)) } - return fmt.Sprintf("%s@%s:%s/%s.git", sshUser, sshDomain, url.PathEscape(ownerName), url.PathEscape(repoName)) + return fmt.Sprintf("%s@%s:%s/%s.git", sshUser, sshHost, url.PathEscape(ownerName), url.PathEscape(repoName)) } func (repo *Repository) cloneLink(isWiki bool) *CloneLink { diff --git a/models/repo/repo_test.go b/models/repo/repo_test.go index 4bbfeb634a..ca9209d751 100644 --- a/models/repo/repo_test.go +++ b/models/repo/repo_test.go @@ -12,6 +12,8 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -186,3 +188,32 @@ func TestGetRepositoryByURL(t *testing.T) { test(t, "try.gitea.io:user2/repo2.git") }) } + +func TestComposeSSHCloneURL(t *testing.T) { + defer test.MockVariableValue(&setting.SSH, setting.SSH)() + defer test.MockVariableValue(&setting.Repository, setting.Repository)() + + setting.SSH.User = "git" + + // test SSH_DOMAIN + setting.SSH.Domain = "domain" + setting.SSH.Port = 22 + setting.Repository.UseCompatSSHURI = false + assert.Equal(t, "git@domain:user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + setting.Repository.UseCompatSSHURI = true + assert.Equal(t, "ssh://git@domain/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + // test SSH_DOMAIN while use non-standard SSH port + setting.SSH.Port = 123 + setting.Repository.UseCompatSSHURI = false + assert.Equal(t, "ssh://git@domain:123/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + setting.Repository.UseCompatSSHURI = true + assert.Equal(t, "ssh://git@domain:123/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + + // test IPv6 SSH_DOMAIN + setting.Repository.UseCompatSSHURI = false + setting.SSH.Domain = "::1" + setting.SSH.Port = 22 + assert.Equal(t, "git@[::1]:user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) + setting.SSH.Port = 123 + assert.Equal(t, "ssh://git@[::1]:123/user/repo.git", repo_model.ComposeSSHCloneURL("user", "repo")) +}