fix: correct "go get" URL ssh scheme, fix form binding (#39674, #39528) (#39676)

* Backport #39674
* Fix form binding errors (found by #39528)
* Fix #39680

---------

Co-authored-by: Roland Singer <10167163+r0l1@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

custom/conf/app.example.ini

@@ -1098,8 +1098,9 @@ LEVEL = Info
 ;; Force ssh:// clone url instead of scp-style uri when default SSH port is used
 ;USE_COMPAT_SSH_URI = false
 ;;
-;; Value for the "go get" request returns the repository url as https or ssh, default is https
+;; Scheme of the returned URL for the "go get" response.
-;GO_GET_CLONE_URL_PROTOCOL = https
+;; Default is "https" if DISABLE_HTTP_GIT=false or SSH is disabled, otherwise "ssh".
+;GO_GET_CLONE_URL_PROTOCOL =
 ;;
 ;; Close issues as long as a commit on any branch marks it as fixed
 ;DEFAULT_CLOSE_ISSUES_VIA_COMMITS_IN_ANY_BRANCH = false

models/repo/repo.go

@@ -616,6 +616,15 @@ func ComposeHTTPSCloneURL(ctx context.Context, owner, repo string) string {
 
 // ComposeSSHCloneURL returns SSH clone URL based on the given owner and repository name.
 func ComposeSSHCloneURL(doer *user_model.User, ownerName, repoName string) string {
+	return composeSSHCloneURL(doer, ownerName, repoName, setting.Repository.UseCompatSSHURI)
+}
+
+// ComposeSSHCloneURI is like ComposeSSHCloneURL but always returns the "ssh://" form, because "go get" rejects scp-style addresses
+func ComposeSSHCloneURI(doer *user_model.User, ownerName, repoName string) string {
+	return composeSSHCloneURL(doer, ownerName, repoName, true)
+}
+
+func composeSSHCloneURL(doer *user_model.User, ownerName, repoName string, useURI bool) string {
 	sshUser := setting.SSH.User
 	sshDomain := setting.SSH.Domain
 
@@ -642,7 +651,7 @@ func ComposeSSHCloneURL(doer *user_model.User, ownerName, repoName string) strin
 	if ip := net.ParseIP(sshHost); ip != nil && ip.To4() == nil {
 		sshHost = "[" + sshHost + "]" // for IPv6 address, wrap it with brackets
 	}
-	if setting.Repository.UseCompatSSHURI {
+	if useURI {
 		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, sshHost, url.PathEscape(ownerName), url.PathEscape(repoName))

models/repo/repo_test.go

@@ -185,6 +185,8 @@ func TestComposeSSHCloneURL(t *testing.T) {
 	assert.Equal(t, "git@domain:user/repo.git", ComposeSSHCloneURL(&user_model.User{Name: "doer"}, "user", "repo"))
 	setting.Repository.UseCompatSSHURI = true
 	assert.Equal(t, "ssh://git@domain/user/repo.git", ComposeSSHCloneURL(&user_model.User{Name: "doer"}, "user", "repo"))
+	setting.Repository.UseCompatSSHURI = false
+	assert.Equal(t, "ssh://git@domain/user/repo.git", ComposeSSHCloneURI(nil, "user", "repo"))
 	// test SSH_DOMAIN while use non-standard SSH port
 	setting.SSH.Port = 123
 	setting.Repository.UseCompatSSHURI = false

modules/setting/repository.go

@@ -303,7 +303,7 @@ func loadRepositoryFrom(rootCfg ConfigProvider) {
 	sec := rootCfg.Section("repository")
 	Repository.DisableHTTPGit = sec.Key("DISABLE_HTTP_GIT").MustBool()
 	Repository.UseCompatSSHURI = sec.Key("USE_COMPAT_SSH_URI").MustBool()
-	Repository.GoGetCloneURLProtocol = sec.Key("GO_GET_CLONE_URL_PROTOCOL").MustString("https")
+	Repository.GoGetCloneURLProtocol = sec.Key("GO_GET_CLONE_URL_PROTOCOL").String()
 	// MAX_CREATION_LIMIT is a shortcut that sets the default for the two per-type limits below.
 	// USER_/ORG_MAX_CREATION_LIMIT take precedence when explicitly set.
 	Repository.MaxCreationLimit = sec.Key("MAX_CREATION_LIMIT").MustInt(-1)                                   // FIXME: INI-MUST-SIDE-EFFECT

routers/web/goget.go

@@ -69,13 +69,7 @@ func goGet(ctx *context.Context) {
 
 	goGetImport := context.ComposeGoGetImport(ctx, ownerName, trimmedRepoName)
 
-	var cloneURL string
+	goImportContent := fmt.Sprintf("%s git %s", goGetImport, context.ComposeGoGetCloneURL(ctx, ownerName, trimmedRepoName))
-	if setting.Repository.GoGetCloneURLProtocol == "ssh" {
-		cloneURL = repo_model.ComposeSSHCloneURL(ctx.Doer, ownerName, repoName)
-	} else {
-		cloneURL = repo_model.ComposeHTTPSCloneURL(ctx, ownerName, repoName)
-	}
-	goImportContent := fmt.Sprintf("%s git %s", goGetImport, cloneURL /*CloneLink*/)
 	goSourceContent := fmt.Sprintf("%s _ %s %s", goGetImport, prefix+"{/dir}" /*GoDocDirectory*/, prefix+"{/dir}/{file}#L{line}" /*GoDocFile*/)
 	goGetCli := fmt.Sprintf("go get %s%s", insecure, goGetImport)
 

services/context/repo.go

@@ -389,6 +389,15 @@ func ComposeGoGetImport(ctx context.Context, owner, repo string) string {
 	return path.Join(curAppURL.Host, setting.AppSubURL, url.PathEscape(owner), url.PathEscape(repo))
 }
 
+// ComposeGoGetCloneURL returns the clone URL for the go-import meta content.
+func ComposeGoGetCloneURL(ctx *Context, owner, repo string) string {
+	useSSH := setting.Repository.GoGetCloneURLProtocol == "ssh" || (setting.Repository.DisableHTTPGit && !setting.SSH.Disabled)
+	if useSSH {
+		return repo_model.ComposeSSHCloneURI(ctx.Doer, owner, repo)
+	}
+	return repo_model.ComposeHTTPSCloneURL(ctx, owner, repo)
+}
+
 // EarlyResponseForGoGetMeta responses appropriate go-get meta with status 200
 // if user does not have actual access to the requested repository,
 // or the owner or repository does not exist at all.
@@ -402,13 +411,7 @@ func EarlyResponseForGoGetMeta(ctx *Context) {
 		return
 	}
 
-	var cloneURL string
+	goImportContent := fmt.Sprintf("%s git %s", ComposeGoGetImport(ctx, username, reponame), ComposeGoGetCloneURL(ctx, username, reponame))
-	if setting.Repository.GoGetCloneURLProtocol == "ssh" {
-		cloneURL = repo_model.ComposeSSHCloneURL(ctx.Doer, username, reponame)
-	} else {
-		cloneURL = repo_model.ComposeHTTPSCloneURL(ctx, username, reponame)
-	}
-	goImportContent := fmt.Sprintf("%s git %s", ComposeGoGetImport(ctx, username, reponame), cloneURL)
 	htmlMeta := fmt.Sprintf(`<meta name="go-import" content="%s">`, html.EscapeString(goImportContent))
 	ctx.PlainText(http.StatusOK, htmlMeta)
 }

services/forms/admin.go

@@ -65,6 +65,6 @@ type AdminEditUserForm struct {
 // AdminDashboardForm form for admin dashboard operations
 type AdminDashboardForm struct {
 	middleware.FormDefaultValidator
-	Op   string `binding:"required"`
+	Op   string `binding:"Required"`
 	From string
 }

services/forms/repo_form.go

@@ -571,12 +571,6 @@ type AddTimeManuallyForm struct {
 	Minutes int `binding:"Range(0,1000)"`
 }
 
-// SaveTopicForm form for save topics for repository
-type SaveTopicForm struct {
-	middleware.FormDefaultValidator
-	Topics []string `binding:"topics;Required;"`
-}
-
 // AddDeployTokenForm form for adding a deploy token to a repository
 type AddDeployTokenForm struct {
 	middleware.FormDefaultValidator

tests/integration/goget_test.go

@@ -58,6 +58,11 @@ func TestGoGetForSSH(t *testing.T) {
 </html>`, setting.AppDomain, setting.HTTPPort, setting.AppURL, setting.SSH.Domain, setting.SSH.Port)
 
 	assert.Equal(t, expected, resp.Body.String())
+
+	// go rejects scp-style addresses, so the standard port must still produce an ssh:// URL
+	defer test.MockVariableValue(&setting.SSH.Port, 22)()
+	resp = MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Body.String(), fmt.Sprintf(`git ssh://git@%s/blah/glah.git">`, setting.SSH.Domain))
 }
 
 // TestGoGetPrivateRepoBranchNotLeaked ensures the go-get meta endpoint does not disclose a