Fix raw file API ref handling (#33172) (#33189)

Backport #33172 by wxiaoguang

Fix #33164 and add more tests

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

2 files changed, 18 insertions, 23 deletions

diff --git a/services/context/api.go b/services/context/api.go
index b45e80a329..50ec0ee40b 100644
--- a/services/context/api.go
+++ b/services/context/api.go
@@ -305,8 +305,7 @@ func RepoRefForAPI(next http.Handler) http.Handler {
 			return
 		}
 
-		// NOTICE: the "ref" here for internal usage only (e.g. woodpecker)
-		refName, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.FormTrim("ref"))
+		refName, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref"))
 		var err error
 
 		if ctx.Repo.GitRepo.IsBranchExist(refName) {
diff --git a/services/context/repo.go b/services/context/repo.go
index 1e7c430347..879b3fcb86 100644
--- a/services/context/repo.go
+++ b/services/context/repo.go
@@ -769,35 +769,30 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool
 	return ""
 }
 
-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 != "" {
+func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, RepoRefType) {
+	reqRefPath := path.Join(extraRef, reqPath)
+	reqRefPathParts := strings.Split(reqRefPath, "/")
+	if refName := getRefName(ctx, repo, reqRefPath, RepoRefBranch); refName != "" {
 		return refName, RepoRefBranch
 	}
-	if refName := getRefName(ctx, repo, RepoRefTag); refName != "" {
+	if refName := getRefName(ctx, repo, reqRefPath, RepoRefTag); refName != "" {
 		return refName, RepoRefTag
 	}
-
-	// For legacy support only full commit sha
-	parts := strings.Split(reqPath, "/")
-	if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), parts[0]) {
+	if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), reqRefPathParts[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], RepoRefCommit
+		repo.TreePath = strings.Join(reqRefPathParts[1:], "/")
+		return reqRefPathParts[0], RepoRefCommit
 	}
-
-	if refName := getRefName(ctx, repo, RepoRefBlob); len(refName) > 0 {
+	if refName := getRefName(ctx, repo, reqPath, RepoRefBlob); refName != "" {
 		return refName, RepoRefBlob
 	}
+	// FIXME: the old code falls back to default branch if "ref" doesn't exist, there could be an edge case:
+	// "README?ref=no-such" would read the README file from the default branch, but the user might expect a 404
 	repo.TreePath = reqPath
 	return repo.Repository.DefaultBranch, RepoRefBranch
 }
 
-func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string {
-	path := ctx.PathParam("*")
+func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType) string {
 	switch pathType {
 	case RepoRefBranch:
 		ref := getRefNameFromPath(repo, path, repo.GitRepo.IsBranchExist)
@@ -900,7 +895,8 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 		}
 
 		// Get default branch.
-		if len(ctx.PathParam("*")) == 0 {
+		reqPath := ctx.PathParam("*")
+		if reqPath == "" {
 			refName = ctx.Repo.Repository.DefaultBranch
 			if !ctx.Repo.GitRepo.IsBranchExist(refName) {
 				brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 1)
@@ -925,12 +921,12 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 				return cancel
 			}
 			ctx.Repo.IsViewBranch = true
-		} else {
+		} else { // there is a path in request
 			guessLegacyPath := refType == RepoRefUnknown
 			if guessLegacyPath {
-				refName, refType = getRefNameLegacy(ctx.Base, ctx.Repo)
+				refName, refType = getRefNameLegacy(ctx.Base, ctx.Repo, reqPath, "")
 			} else {
-				refName = getRefName(ctx.Base, ctx.Repo, refType)
+				refName = getRefName(ctx.Base, ctx.Repo, reqPath, refType)
 			}
 			ctx.Repo.RefName = refName
 			isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool)