Merge 0efa2beba64b4afbd730608c9eafc9e04d1909ee into c68925152b1b6c8f92806cdbda9c4672dcc1608f

modules/git/ref.go

@@ -228,13 +228,16 @@ func (ref RefName) RefWebLinkPath() string {
 	return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
 }
 
-func ParseRefSuffix(ref string) (string, string) {
+func ParseRefSuffix(ref string) (refName, refSuffix string) {
 	// Partially support https://git-scm.com/docs/gitrevisions
-	if idx := strings.Index(ref, "@{"); idx != -1 {
-		return ref[:idx], ref[idx:]
+	suffixIdx := -1 // earliest suffix mark, so a combined suffix like "main~2^" stays intact
+	for _, mark := range []string{"@{", "^", "~"} {
+		if idx := strings.Index(ref, mark); idx != -1 && (suffixIdx == -1 || idx < suffixIdx) {
+			suffixIdx = idx
+		}
 	}
-	if idx := strings.Index(ref, "^"); idx != -1 {
-		return ref[:idx], ref[idx:]
+	if suffixIdx == -1 {
+		return ref, ""
 	}
-	return ref, ""
+	return ref[:suffixIdx], ref[suffixIdx:]
 }

modules/git/ref_test.go

@@ -37,3 +37,22 @@ func TestRefWebLinkPath(t *testing.T) {
 	assert.Equal(t, "tag/foo", RefName("refs/tags/foo").RefWebLinkPath())
 	assert.Equal(t, "commit/c0ffee", RefName("c0ffee").RefWebLinkPath())
 }
+
+func TestParseRefSuffix(t *testing.T) {
+	cases := []struct {
+		ref, name, suffix string
+	}{
+		{"main", "main", ""},
+		{"main^", "main", "^"},
+		{"main^2", "main", "^2"},
+		{"main~3", "main", "~3"},
+		{"main@{yesterday}", "main", "@{yesterday}"},
+		{"main~2^", "main", "~2^"},
+		{"main^~2", "main", "^~2"},
+	}
+	for _, c := range cases {
+		name, suffix := ParseRefSuffix(c.ref)
+		assert.Equal(t, c.name, name, "ref: %s", c.ref)
+		assert.Equal(t, c.suffix, suffix, "ref: %s", c.ref)
+	}
+}

routers/api/v1/repo/compare.go

@@ -38,7 +38,7 @@ func CompareDiff(ctx *context.APIContext) {
 	//   required: true
 	// - name: basehead
 	//   in: path
-	//   description: compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs, including branch names that contain slashes.
+	//   description: compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs (including branch names that contain slashes), optionally with a `^` or `~N` revision suffix.
 	//   type: string
 	//   required: true
 	// - name: output

routers/api/v1/repo/pull.go

@@ -1087,12 +1087,6 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git
 	baseRepo := ctx.Repo.Repository
 	compareReq := common.ParseCompareRouterParam(compareParam)
 
-	// remove the check when we support compare with carets
-	if compareReq.BaseOriRefSuffix != "" {
-		ctx.APIError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix")
-		return nil, nil
-	}
-
 	_, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
 	switch {
 	case errors.Is(err, util.ErrInvalidArgument):
@@ -1152,10 +1146,10 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git
 		return nil, nil
 	}
 
-	baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx)))
-	headRef := headGitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch))
+	baseRef := common.ResolveRefWithSuffix(ctx.Repo.GitRepo, util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx)), compareReq.BaseOriRefSuffix)
+	headRef := common.ResolveRefWithSuffix(headGitRepo, util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch), compareReq.HeadOriRefSuffix)
 
-	log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef)
+	log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef+compareReq.BaseOriRefSuffix, baseRef, compareReq.HeadOriRef+compareReq.HeadOriRefSuffix, headRef)
 
 	baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName())
 	headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName())

routers/common/compare.go

@@ -5,6 +5,7 @@ package common
 
 import (
 	"context"
+	"regexp"
 	"strings"
 
 	repo_model "gitea.dev/models/repo"
@@ -19,9 +20,10 @@ type CompareRouterReq struct {
 
 	CompareSeparator string
 
-	HeadOwner    string
-	HeadRepoName string
-	HeadOriRef   string
+	HeadOwner        string
+	HeadRepoName     string
+	HeadOriRef       string
+	HeadOriRefSuffix string
 }
 
 func (cr *CompareRouterReq) DirectComparison() bool {
@@ -80,8 +82,10 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
 		basePart, headPart, ok = strings.Cut(routerParam, sep)
 		if !ok {
 			headOwnerName, headRepoName, headRef := parseHead(routerParam)
+			headRef, headRefSuffix := git.ParseRefSuffix(headRef)
 			return &CompareRouterReq{
 				HeadOriRef:       headRef,
+				HeadOriRefSuffix: headRefSuffix,
 				HeadOwner:        headOwnerName,
 				HeadRepoName:     headRepoName,
 				CompareSeparator: "...",
@@ -92,9 +96,30 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
 	ci := &CompareRouterReq{CompareSeparator: sep}
 	ci.BaseOriRef, ci.BaseOriRefSuffix = git.ParseRefSuffix(basePart)
 	ci.HeadOwner, ci.HeadRepoName, ci.HeadOriRef = parseHead(headPart)
+	ci.HeadOriRef, ci.HeadOriRefSuffix = git.ParseRefSuffix(ci.HeadOriRef)
 	return ci
 }
 
+// validRefSuffix matches git ancestry navigation (^, ^N, ~, ~N and combinations). It rejects the
+// ^{...}, @{...} and :path forms, which would let a reader probe object types or commit messages.
+var validRefSuffix = regexp.MustCompile(`^(?:[~^][0-9]*)+$`)
+
+// ResolveRefWithSuffix resolves oriRef plus an optional revision suffix (^, ~N) to a RefName,
+// returning the peeled commit ID for a suffix or an empty RefName when it can not be resolved.
+func ResolveRefWithSuffix(gitRepo *git.Repository, oriRef, refSuffix string) git.RefName {
+	if refSuffix == "" {
+		return gitRepo.UnstableGuessRefByShortName(oriRef)
+	}
+	if !validRefSuffix.MatchString(refSuffix) {
+		return ""
+	}
+	commit, err := gitRepo.GetCommit(oriRef + refSuffix)
+	if err != nil {
+		return ""
+	}
+	return git.RefNameFromCommit(commit.ID.String())
+}
+
 // maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository.
 const maxForkTraverseLevel = 10
 

routers/common/compare_test.go

@@ -97,6 +97,43 @@ func TestCompareRouterReq(t *testing.T) {
 				HeadOriRef:       "develop",
 			},
 		},
+		{
+			input: "main...develop^",
+			CompareRouterReq: &CompareRouterReq{
+				BaseOriRef:       "main",
+				CompareSeparator: "...",
+				HeadOriRef:       "develop",
+				HeadOriRefSuffix: "^",
+			},
+		},
+		{
+			input: "main~2...develop",
+			CompareRouterReq: &CompareRouterReq{
+				BaseOriRef:       "main",
+				BaseOriRefSuffix: "~2",
+				CompareSeparator: "...",
+				HeadOriRef:       "develop",
+			},
+		},
+		{
+			input: "main...lunny/forked_repo:develop~3",
+			CompareRouterReq: &CompareRouterReq{
+				BaseOriRef:       "main",
+				CompareSeparator: "...",
+				HeadOwner:        "lunny",
+				HeadRepoName:     "forked_repo",
+				HeadOriRef:       "develop",
+				HeadOriRefSuffix: "~3",
+			},
+		},
+		{
+			input: "develop^",
+			CompareRouterReq: &CompareRouterReq{
+				CompareSeparator: "...",
+				HeadOriRef:       "develop",
+				HeadOriRefSuffix: "^",
+			},
+		},
 	}
 
 	for _, c := range cases {

routers/web/repo/compare.go

@@ -208,8 +208,8 @@ func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context, comparePa
 	// 1 Parse compare router param
 	compareReq := common.ParseCompareRouterParam(compareParam)
 
-	// remove the check when we support compare with carets
-	if compareReq.BaseOriRefSuffix != "" {
+	// remove the check when the web compare page supports carets
+	if compareReq.BaseOriRefSuffix != "" || compareReq.HeadOriRefSuffix != "" {
 		return util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
 	}
 

templates/swagger/v1_json.tmpl

@@ -8135,7 +8135,7 @@
           },
           {
             "type": "string",
-            "description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs, including branch names that contain slashes.",
+            "description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs (including branch names that contain slashes), optionally with a `^` or `~N` revision suffix.",
             "name": "basehead",
             "in": "path",
             "required": true

templates/swagger/v1_openapi3_json.tmpl

@@ -19490,7 +19490,7 @@
             }
           },
           {
-            "description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs, including branch names that contain slashes.",
+            "description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs (including branch names that contain slashes), optionally with a `^` or `~N` revision suffix.",
             "in": "path",
             "name": "basehead",
             "required": true,

tests/integration/api_repo_compare_test.go

@@ -45,6 +45,38 @@ func TestAPICompareBranches(t *testing.T) {
 			assert.Len(t, apiResp.Commits, 1)
 		})
 
+		t.Run("CompareWithRefSuffix", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// remove-files-b^ is the parent of the tip, so the range drops the tip and ends at that parent
+			const parentSHA = "b67e43a07d48243a5f670ace063acd5e13f719df"
+			req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b^").AddTokenAuth(token2)
+			resp := MakeRequest(t, req, http.StatusOK)
+			apiResp := DecodeJSON(t, resp, &api.Compare{})
+			assert.Equal(t, 1, apiResp.TotalCommits)
+			assert.Len(t, apiResp.Commits, 1)
+			assert.Equal(t, parentSHA, apiResp.Commits[0].SHA)
+
+			// the same suffix on the direct ".." comparison resolves to the same commit
+			req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv..remove-files-b^").AddTokenAuth(token2)
+			resp = MakeRequest(t, req, http.StatusOK)
+			apiResp = DecodeJSON(t, resp, &api.Compare{})
+			assert.Equal(t, 1, apiResp.TotalCommits)
+			assert.Equal(t, parentSHA, apiResp.Commits[0].SHA)
+
+			req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv~1...add-csv").AddTokenAuth(token2)
+			resp = MakeRequest(t, req, http.StatusOK)
+			apiResp = DecodeJSON(t, resp, &api.Compare{})
+			assert.Equal(t, 1, apiResp.TotalCommits)
+			assert.Len(t, apiResp.Commits, 1)
+
+			// an unresolvable suffix is not found, and only ^/~ navigation is accepted (^{...} is rejected)
+			req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b~50").AddTokenAuth(token2)
+			MakeRequest(t, req, http.StatusNotFound)
+			req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b^{/Add}").AddTokenAuth(token2)
+			MakeRequest(t, req, http.StatusNotFound)
+		})
+
 		t.Run("CompareForkOnlyCommit", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()