Refactor RepoRefByType (#32413)

1. clarify the "filepath" could(should) contain "{ref}"
2. remove unclear RepoRefLegacy and RepoRefAny, use RepoRefUnknown to guess
3. by the way, avoid using AppURL

routers/api/v1/repo/file.go

@@ -56,12 +56,12 @@ func GetRawFile(ctx *context.APIContext) {
 	//   required: true
 	// - name: filepath
 	//   in: path
-	//   description: filepath of the file to get
+	//   description: path of the file to get, it should be "{ref}/{filepath}". If there is no ref could be inferred, it will be treated as the default branch
 	//   type: string
 	//   required: true
 	// - name: ref
 	//   in: query
-	//   description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)"
+	//   description: "The name of the commit/branch/tag. Default the repository’s default branch"
 	//   type: string
 	//   required: false
 	// responses:
@@ -109,12 +109,12 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
 	//   required: true
 	// - name: filepath
 	//   in: path
-	//   description: filepath of the file to get
+	//   description: path of the file to get, it should be "{ref}/{filepath}". If there is no ref could be inferred, it will be treated as the default branch
 	//   type: string
 	//   required: true
 	// - name: ref
 	//   in: query
-	//   description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)"
+	//   description: "The name of the commit/branch/tag. Default the repository’s default branch"
 	//   type: string
 	//   required: false
 	// responses:

routers/web/goget.go

@@ -65,7 +65,7 @@ func goGet(ctx *context.Context) {
 		insecure = "--insecure "
 	}
 
-	goGetImport := context.ComposeGoGetImport(ownerName, trimmedRepoName)
+	goGetImport := context.ComposeGoGetImport(ctx, ownerName, trimmedRepoName)
 
 	var cloneURL string
 	if setting.Repository.GoGetCloneURLProtocol == "ssh" {

routers/web/web.go

@@ -1323,7 +1323,7 @@ func registerRoutes(m *web.Router) {
 			m.Get(".rss", feedEnabled, repo.TagsListFeedRSS)
 			m.Get(".atom", feedEnabled, repo.TagsListFeedAtom)
 		}, ctxDataSet("EnableFeed", setting.Other.EnableFeed),
-			repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, true))
+			repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true}))
 		m.Post("/tags/delete", repo.DeleteTag, reqSignIn,
 			repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef())
 	}, ignSignIn, context.RepoAssignment, reqRepoCodeReader)
@@ -1337,7 +1337,7 @@ func registerRoutes(m *web.Router) {
 			m.Get(".rss", feedEnabled, repo.ReleasesFeedRSS)
 			m.Get(".atom", feedEnabled, repo.ReleasesFeedAtom)
 		}, ctxDataSet("EnableFeed", setting.Other.EnableFeed),
-			repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, true))
+			repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true}))
 		m.Get("/releases/attachments/{uuid}", repo.MustBeNotEmpty, repo.GetAttachment)
 		m.Get("/releases/download/{vTag}/{fileName}", repo.MustBeNotEmpty, repo.RedirectDownload)
 		m.Group("/releases", func() {
@@ -1535,7 +1535,7 @@ func registerRoutes(m *web.Router) {
 			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownloadOrLFS)
 			m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByIDOrLFS)
 			// "/*" route is deprecated, and kept for backward compatibility
-			m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownloadOrLFS)
+			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownloadOrLFS)
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/raw", func() {
@@ -1544,7 +1544,7 @@ func registerRoutes(m *web.Router) {
 			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload)
 			m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByID)
 			// "/*" route is deprecated, and kept for backward compatibility
-			m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownload)
+			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownload)
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/render", func() {
@@ -1559,7 +1559,7 @@ func registerRoutes(m *web.Router) {
 			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefCommits)
 			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefCommits)
 			// "/*" route is deprecated, and kept for backward compatibility
-			m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.RefCommits)
+			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.RefCommits)
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/blame", func() {
@@ -1582,7 +1582,7 @@ func registerRoutes(m *web.Router) {
 			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home)
 			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home)
 			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home)
-			m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.Home) // "/*" route is deprecated, and kept for backward compatibility
+			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.Home) // "/*" route is deprecated, and kept for backward compatibility
 		}, repo.SetEditorconfigIfExists)
 
 		m.Get("/forks", context.RepoRef(), repo.Forks)

services/context/api.go

@@ -14,7 +14,6 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/cache"
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/httpcache"
 	"code.gitea.io/gitea/modules/log"
@@ -306,24 +305,8 @@ func RepoRefForAPI(next http.Handler) http.Handler {
 			return
 		}
 
-		if ref := ctx.FormTrim("ref"); len(ref) > 0 {
+		// NOTICE: the "ref" here for internal usage only (e.g. woodpecker)
-			commit, err := ctx.Repo.GitRepo.GetCommit(ref)
+		refName, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.FormTrim("ref"))
-			if err != nil {
-				if git.IsErrNotExist(err) {
-					ctx.NotFound()
-				} else {
-					ctx.Error(http.StatusInternalServerError, "GetCommit", err)
-				}
-				return
-			}
-			ctx.Repo.Commit = commit
-			ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
-			ctx.Repo.TreePath = ctx.PathParam("*")
-			next.ServeHTTP(w, req)
-			return
-		}
-
-		refName := getRefName(ctx.Base, ctx.Repo, RepoRefAny)
 		var err error
 
 		if ctx.Repo.GitRepo.IsBranchExist(refName) {

services/context/repo.go

@@ -25,6 +25,7 @@ import (
 	"code.gitea.io/gitea/modules/cache"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
+	"code.gitea.io/gitea/modules/httplib"
 	code_indexer "code.gitea.io/gitea/modules/indexer/code"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
@@ -306,11 +307,9 @@ func RetrieveTemplateRepo(ctx *Context, repo *repo_model.Repository) {
 }
 
 // ComposeGoGetImport returns go-get-import meta content.
-func ComposeGoGetImport(owner, repo string) string {
+func ComposeGoGetImport(ctx context.Context, owner, repo string) string {
-	/// setting.AppUrl is guaranteed to be parse as url
+	curAppURL, _ := url.Parse(httplib.GuessCurrentAppURL(ctx))
-	appURL, _ := url.Parse(setting.AppURL)
+	return path.Join(curAppURL.Host, setting.AppSubURL, url.PathEscape(owner), url.PathEscape(repo))
-
-	return path.Join(appURL.Host, setting.AppSubURL, url.PathEscape(owner), url.PathEscape(repo))
 }
 
 // EarlyResponseForGoGetMeta responses appropriate go-get meta with status 200
@@ -332,7 +331,7 @@ func EarlyResponseForGoGetMeta(ctx *Context) {
 	} else {
 		cloneURL = repo_model.ComposeHTTPSCloneURL(username, reponame)
 	}
-	goImportContent := fmt.Sprintf("%s git %s", ComposeGoGetImport(username, reponame), cloneURL)
+	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)
 }
@@ -744,7 +743,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc {
 	}
 
 	if ctx.FormString("go-get") == "1" {
-		ctx.Data["GoGetImport"] = ComposeGoGetImport(owner.Name, repo.Name)
+		ctx.Data["GoGetImport"] = ComposeGoGetImport(ctx, owner.Name, repo.Name)
 		fullURLPrefix := repo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(ctx.Repo.BranchName)
 		ctx.Data["GoDocDirectory"] = fullURLPrefix + "{/dir}"
 		ctx.Data["GoDocFile"] = fullURLPrefix + "{/dir}/{file}#L{line}"
@@ -756,19 +755,11 @@ func RepoAssignment(ctx *Context) context.CancelFunc {
 type RepoRefType int
 
 const (
-	// RepoRefLegacy unknown type, make educated guess and redirect.
+	// RepoRefUnknown is for legacy support, makes the code to "guess" the ref type
-	// for backward compatibility with previous URL scheme
+	RepoRefUnknown RepoRefType = iota
-	RepoRefLegacy RepoRefType = iota
-	// RepoRefAny is for usage where educated guess is needed
-	// but redirect can not be made
-	RepoRefAny
-	// RepoRefBranch branch
 	RepoRefBranch
-	// RepoRefTag tag
 	RepoRefTag
-	// RepoRefCommit commit
 	RepoRefCommit
-	// RepoRefBlob blob
 	RepoRefBlob
 )
 
@@ -781,22 +772,6 @@ func RepoRef() func(*Context) context.CancelFunc {
 	return RepoRefByType(RepoRefBranch)
 }
 
-// RefTypeIncludesBranches returns true if ref type can be a branch
-func (rt RepoRefType) RefTypeIncludesBranches() bool {
-	if rt == RepoRefLegacy || rt == RepoRefAny || rt == RepoRefBranch {
-		return true
-	}
-	return false
-}
-
-// RefTypeIncludesTags returns true if ref type can be a tag
-func (rt RepoRefType) RefTypeIncludesTags() bool {
-	if rt == RepoRefLegacy || rt == RepoRefAny || rt == RepoRefTag {
-		return true
-	}
-	return false
-}
-
 func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool) string {
 	refName := ""
 	parts := strings.Split(path, "/")
@@ -810,28 +785,50 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool
 	return ""
 }
 
-func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string {
+func isStringLikelyCommitID(objFmt git.ObjectFormat, s string, minLength ...int) bool {
-	path := ctx.PathParam("*")
+	minLen := util.OptionalArg(minLength, objFmt.FullLength())
-	switch pathType {
+	if len(s) < minLen || len(s) > objFmt.FullLength() {
-	case RepoRefLegacy, RepoRefAny:
+		return false
-		if refName := getRefName(ctx, repo, RepoRefBranch); len(refName) > 0 {
-			return refName
 	}
-		if refName := getRefName(ctx, repo, RepoRefTag); len(refName) > 0 {
+	for _, c := range s {
-			return refName
+		isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')
+		if !isHex {
+			return false
+		}
+	}
+	return true
+}
+
+func getRefNameLegacy(ctx *Base, repo *Repository, optionalExtraRef ...string) (string, RepoRefType) {
+	extraRef := util.OptionalArg(optionalExtraRef)
+	reqPath := ctx.PathParam("*")
+	reqPath = path.Join(extraRef, reqPath)
+
+	if refName := getRefName(ctx, repo, RepoRefBranch); refName != "" {
+		return refName, RepoRefBranch
+	}
+	if refName := getRefName(ctx, repo, RepoRefTag); refName != "" {
+		return refName, RepoRefTag
 	}
-		// For legacy and API support only full commit sha
-		parts := strings.Split(path, "/")
 
-		if len(parts) > 0 && len(parts[0]) == git.ObjectFormatFromName(repo.Repository.ObjectFormatName).FullLength() {
+	// For legacy support only full commit sha
+	parts := strings.Split(reqPath, "/")
+	if isStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), parts[0]) {
+		// FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists
 		repo.TreePath = strings.Join(parts[1:], "/")
-			return parts[0]
+		return parts[0], RepoRefCommit
 	}
+
 	if refName := getRefName(ctx, repo, RepoRefBlob); len(refName) > 0 {
-			return refName
+		return refName, RepoRefBlob
 	}
-		repo.TreePath = path
+	repo.TreePath = reqPath
-		return repo.Repository.DefaultBranch
+	return repo.Repository.DefaultBranch, RepoRefBranch
+}
+
+func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string {
+	path := ctx.PathParam("*")
+	switch pathType {
 	case RepoRefBranch:
 		ref := getRefNameFromPath(repo, path, repo.GitRepo.IsBranchExist)
 		if len(ref) == 0 {
@@ -866,13 +863,13 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string {
 		return getRefNameFromPath(repo, path, repo.GitRepo.IsTagExist)
 	case RepoRefCommit:
 		parts := strings.Split(path, "/")
-
+		if isStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) {
-		if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= repo.GetObjectFormat().FullLength() {
+			// FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists
 			repo.TreePath = strings.Join(parts[1:], "/")
 			return parts[0]
 		}
 
-		if len(parts) > 0 && parts[0] == headRefName {
+		if parts[0] == headRefName {
 			// HEAD ref points to last default branch commit
 			commit, err := repo.GitRepo.GetBranchCommit(repo.Repository.DefaultBranch)
 			if err != nil {
@@ -888,15 +885,21 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string {
 		}
 		return path
 	default:
-		log.Error("Unrecognized path type: %v", path)
+		panic(fmt.Sprintf("Unrecognized path type: %v", pathType))
 	}
 	return ""
 }
 
+type RepoRefByTypeOptions struct {
+	IgnoreNotExistErr bool
+}
+
 // RepoRefByType handles repository reference name for a specific type
 // of repository reference
-func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context) context.CancelFunc {
+func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) context.CancelFunc {
+	opt := util.OptionalArg(opts)
 	return func(ctx *Context) (cancel context.CancelFunc) {
+		refType := detectRefType
 		// Empty repository does not have reference information.
 		if ctx.Repo.Repository.IsEmpty {
 			// assume the user is viewing the (non-existent) default branch
@@ -955,8 +958,13 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 				return cancel
 			}
 			ctx.Repo.IsViewBranch = true
+		} else {
+			guessLegacyPath := refType == RepoRefUnknown
+			if guessLegacyPath {
+				refName, refType = getRefNameLegacy(ctx.Base, ctx.Repo)
 			} else {
 				refName = getRefName(ctx.Base, ctx.Repo, refType)
+			}
 			ctx.Repo.RefName = refName
 			isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool)
 			if isRenamedBranch && has {
@@ -967,7 +975,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 				return cancel
 			}
 
-			if refType.RefTypeIncludesBranches() && ctx.Repo.GitRepo.IsBranchExist(refName) {
+			if refType == RepoRefBranch && ctx.Repo.GitRepo.IsBranchExist(refName) {
 				ctx.Repo.IsViewBranch = true
 				ctx.Repo.BranchName = refName
 
@@ -977,7 +985,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 					return cancel
 				}
 				ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
-			} else if refType.RefTypeIncludesTags() && ctx.Repo.GitRepo.IsTagExist(refName) {
+			} else if refType == RepoRefTag && ctx.Repo.GitRepo.IsTagExist(refName) {
 				ctx.Repo.IsViewTag = true
 				ctx.Repo.TagName = refName
 
@@ -991,7 +999,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 					return cancel
 				}
 				ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
-			} else if len(refName) >= 7 && len(refName) <= ctx.Repo.GetObjectFormat().FullLength() {
+			} else if isStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refName, 7) {
 				ctx.Repo.IsViewCommit = true
 				ctx.Repo.CommitID = refName
 
@@ -1002,18 +1010,18 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 				}
 				// If short commit ID add canonical link header
 				if len(refName) < ctx.Repo.GetObjectFormat().FullLength() {
-					ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"",
+					canonicalURL := util.URLJoin(httplib.GuessCurrentAppURL(ctx), strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1))
-						util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1))))
+					ctx.RespHeader().Set("Link", fmt.Sprintf(`<%s>; rel="canonical"`, canonicalURL))
 				}
 			} else {
-				if len(ignoreNotExistErr) > 0 && ignoreNotExistErr[0] {
+				if opt.IgnoreNotExistErr {
 					return cancel
 				}
 				ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName))
 				return cancel
 			}
 
-			if refType == RepoRefLegacy {
+			if guessLegacyPath {
 				// redirect from old URL scheme to new URL scheme
 				prefix := strings.TrimPrefix(setting.AppSubURL+strings.ToLower(strings.TrimSuffix(ctx.Req.URL.Path, ctx.PathParam("*"))), strings.ToLower(ctx.Repo.RepoLink))
 				redirect := path.Join(

templates/swagger/v1_json.tmpl

@@ -10727,14 +10727,14 @@
           },
           {
             "type": "string",
-            "description": "filepath of the file to get",
+            "description": "path of the file to get, it should be \"{ref}/{filepath}\". If there is no ref could be inferred, it will be treated as the default branch",
             "name": "filepath",
             "in": "path",
             "required": true
           },
           {
             "type": "string",
-            "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)",
+            "description": "The name of the commit/branch/tag. Default the repository’s default branch",
             "name": "ref",
             "in": "query"
           }
@@ -12818,14 +12818,14 @@
           },
           {
             "type": "string",
-            "description": "filepath of the file to get",
+            "description": "path of the file to get, it should be \"{ref}/{filepath}\". If there is no ref could be inferred, it will be treated as the default branch",
             "name": "filepath",
             "in": "path",
             "required": true
           },
           {
             "type": "string",
-            "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)",
+            "description": "The name of the commit/branch/tag. Default the repository’s default branch",
             "name": "ref",
             "in": "query"
           }

tests/integration/links_test.go

@@ -49,18 +49,18 @@ func TestLinksNoLogin(t *testing.T) {
 func TestRedirectsNoLogin(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	redirects := map[string]string{
+	redirects := []struct{ from, to string }{
-		"/user2/repo1/commits/master":                "/user2/repo1/commits/branch/master",
+		{"/user2/repo1/commits/master", "/user2/repo1/commits/branch/master"},
-		"/user2/repo1/src/master":                    "/user2/repo1/src/branch/master",
+		{"/user2/repo1/src/master", "/user2/repo1/src/branch/master"},
-		"/user2/repo1/src/master/file.txt":           "/user2/repo1/src/branch/master/file.txt",
+		{"/user2/repo1/src/master/file.txt", "/user2/repo1/src/branch/master/file.txt"},
-		"/user2/repo1/src/master/directory/file.txt": "/user2/repo1/src/branch/master/directory/file.txt",
+		{"/user2/repo1/src/master/directory/file.txt", "/user2/repo1/src/branch/master/directory/file.txt"},
-		"/user/avatar/Ghost/-1":                      "/assets/img/avatar_default.png",
+		{"/user/avatar/Ghost/-1", "/assets/img/avatar_default.png"},
-		"/api/v1/swagger":                            "/api/swagger",
+		{"/api/v1/swagger", "/api/swagger"},
-	}
+	}
-	for link, redirectLink := range redirects {
+	for _, c := range redirects {
-		req := NewRequest(t, "GET", link)
+		req := NewRequest(t, "GET", c.from)
 		resp := MakeRequest(t, req, http.StatusSeeOther)
-		assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp))
+		assert.EqualValues(t, path.Join(setting.AppSubURL, c.to), test.RedirectURL(resp))
 	}
 }