feat(api): support ref suffixes in compare (#38148)

Compare API requests with a `^` or `~N` revision suffix (for example
`compare/main...feature^`) were rejected with `400 Unsupported
comparison syntax: ref with suffix`. The fix resolves the suffix to a
commit before comparing, so `base...head^` and `~N` work on either side,
the same as git.

Only `^`/`~N` navigation is resolved. Pull request creation still
requires plain branch refs, and the web compare page keeps rejecting
suffixes since its branch selectors need separate UI work.

Closes #33943
This commit is contained in:
Eyüp Can Akman
2026-06-24 08:38:02 +03:00
committed by GitHub
parent 59d4825a95
commit ef927f9fa3
11 changed files with 246 additions and 34 deletions
+37 -5
View File
@@ -5,7 +5,9 @@ package common
import (
"context"
"regexp"
"strings"
"sync"
repo_model "gitea.dev/models/repo"
user_model "gitea.dev/models/user"
@@ -19,9 +21,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 {
@@ -79,9 +82,11 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
sep = ".."
basePart, headPart, ok = strings.Cut(routerParam, sep)
if !ok {
headOwnerName, headRepoName, headRef := parseHead(routerParam)
headOwnerName, headRepoName, headOriRef := parseHead(routerParam)
headOriRef, headOriRefSuffix := git.ParseRefSuffix(headOriRef)
return &CompareRouterReq{
HeadOriRef: headRef,
HeadOriRef: headOriRef,
HeadOriRefSuffix: headOriRefSuffix,
HeadOwner: headOwnerName,
HeadRepoName: headRepoName,
CompareSeparator: "...",
@@ -92,9 +97,36 @@ 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 only ^/~ ancestry navigation. The ^{...}, @{...} and :path forms address
// other objects (trees, blobs) or reflog/upstream state that compare does not resolve, so they are rejected.
var validRefSuffix = sync.OnceValue(func() *regexp.Regexp {
return regexp.MustCompile(`^(?:[~^][0-9]*)+$`)
})
// ResolveRefWithSuffix resolves oriRef plus an optional revision suffix (^, ~N) to a RefName.
// A nil error guarantees a usable RefName: an unsupported suffix yields an invalid-argument error
// and an unresolvable ref yields a not-found error.
func ResolveRefWithSuffix(gitRepo *git.Repository, oriRef, refSuffix string) (git.RefName, error) {
if refSuffix == "" {
if refName := gitRepo.UnstableGuessRefByShortName(oriRef); refName != "" {
return refName, nil
}
return "", util.NewNotExistErrorf("ref %q does not exist", oriRef)
}
if !validRefSuffix().MatchString(refSuffix) {
return "", util.NewInvalidArgumentErrorf("unsupported ref suffix %q", refSuffix)
}
commit, err := gitRepo.GetCommit(oriRef + refSuffix)
if err != nil {
return "", util.NewNotExistErrorf("ref %q does not exist", oriRef+refSuffix)
}
return git.RefNameFromCommit(commit.ID.String()), nil
}
// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository.
const maxForkTraverseLevel = 10
+49
View File
@@ -6,6 +6,8 @@ package common
import (
"testing"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert"
)
@@ -97,9 +99,56 @@ 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 {
assert.Equal(t, c.CompareRouterReq, ParseCompareRouterParam(c.input), "input: %s", c.input)
}
}
func TestResolveRefWithSuffix(t *testing.T) {
// The ^{...}, @{...} and :path forms address non-commit objects or reflog state, so they are
// rejected before any repository access and a nil repo is fine here.
for _, refSuffix := range []string{"^{/Add}", "^{commit}", "@{upstream}", "~1:path"} {
ref, err := ResolveRefWithSuffix(nil, "branch", refSuffix)
assert.ErrorIs(t, err, util.ErrInvalidArgument, "suffix %q", refSuffix)
assert.Empty(t, ref, "suffix %q", refSuffix)
}
}