diff --git a/modules/git/ref.go b/modules/git/ref.go index 7d0bbcbae9..ba69cf87ff 100644 --- a/modules/git/ref.go +++ b/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:] } diff --git a/modules/git/ref_test.go b/modules/git/ref_test.go index 5397191561..5016eea9d5 100644 --- a/modules/git/ref_test.go +++ b/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) + } +} diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index ddd5a9bb21..ddf34215c0 100644 --- a/routers/api/v1/repo/compare.go +++ b/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 diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index d5c912d243..036bb074ab 100644 --- a/routers/api/v1/repo/pull.go +++ b/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()) diff --git a/routers/common/compare.go b/routers/common/compare.go index 667a32a965..5c84981c69 100644 --- a/routers/common/compare.go +++ b/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 diff --git a/routers/common/compare_test.go b/routers/common/compare_test.go index e4e24a03cf..645173247d 100644 --- a/routers/common/compare_test.go +++ b/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 { diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 5a9fa1c1d1..7e93b67247 100644 --- a/routers/web/repo/compare.go +++ b/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") } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 37a358a26b..7afb44d2e8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/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 diff --git a/templates/swagger/v1_openapi3_json.tmpl b/templates/swagger/v1_openapi3_json.tmpl index 7829544526..7c5f738e45 100644 --- a/templates/swagger/v1_openapi3_json.tmpl +++ b/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, diff --git a/tests/integration/api_repo_compare_test.go b/tests/integration/api_repo_compare_test.go index 5bb8cf6bde..3a4d495ffb 100644 --- a/tests/integration/api_repo_compare_test.go +++ b/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)()