From 052feee34a807fcf6964ce93e2792379cbf34c71 Mon Sep 17 00:00:00 2001 From: Rafail Giavrimis <47496212+grafail@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:37:15 +0100 Subject: [PATCH] feat: add raw diff/patch endpoint for repository comparisons (#37632) ## Summary Adds `GET /repos/{owner}/{repo}/compare/{basehead}.{diffType:diff|patch}`, mirroring the existing `/git/commits/{sha}.{diffType}` endpoint but for comparisons between two arbitrary refs. The new endpoint streams a raw unified diff or `git format-patch` output between any two refs: GET /repos/{owner}/{repo}/compare/main...feature.diff GET /repos/{owner}/{repo}/compare/v1.0..v1.1.patch GET /repos/{owner}/{repo}/compare/abc1234...def5678.diff Resolves #5561, #13416 and #17165. AI was used while creating this PR. Automated tests were added as per the contribution policy. --------- Co-authored-by: bircni Co-authored-by: Claude Opus 4.8 --- routers/api/v1/repo/compare.go | 41 +++++++- routers/web/repo/compare.go | 45 ++++++++- routers/web/repo/pull.go | 2 +- routers/web/web.go | 9 +- templates/swagger/v1_json.tmpl | 16 ++- templates/swagger/v1_openapi3_json.tmpl | 15 ++- tests/integration/api_repo_compare_test.go | 111 +++++++++++++++++++++ tests/integration/compare_test.go | 47 +++++++++ 8 files changed, 275 insertions(+), 11 deletions(-) diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 081ea5e91b4..ddd5a9bb21a 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -11,6 +11,7 @@ import ( api "gitea.dev/modules/structs" "gitea.dev/services/context" "gitea.dev/services/convert" + git_service "gitea.dev/services/git" ) // CompareDiff compare two branches or commits @@ -18,8 +19,12 @@ func CompareDiff(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/compare/{basehead} repository repoCompareDiff // --- // summary: Get commit comparison information + // description: | + // By default returns JSON commit comparison information. The raw diff or patch can be + // requested with the `output` query parameter set to `diff` or `patch` respectively. // produces: // - application/json + // - text/plain // parameters: // - name: owner // in: path @@ -33,9 +38,16 @@ func CompareDiff(ctx *context.APIContext) { // required: true // - name: basehead // in: path - // description: compare two branches or commits + // 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. // type: string // required: true + // - name: output + // in: query + // description: return the raw comparison as `diff` or `patch` instead of JSON + // type: string + // enum: + // - diff + // - patch // responses: // "200": // "$ref": "#/responses/Compare" @@ -57,6 +69,16 @@ func CompareDiff(ctx *context.APIContext) { } defer closer() + // ?output=diff|patch returns the raw output, otherwise the JSON comparison is returned. + switch ctx.FormString("output") { + case "diff": + downloadCompareDiffOrPatch(ctx, compareInfo, false) + return + case "patch": + downloadCompareDiffOrPatch(ctx, compareInfo, true) + return + } + verification := ctx.FormString("verification") == "" || ctx.FormBool("verification") files := ctx.FormString("files") == "" || ctx.FormBool("files") @@ -88,3 +110,20 @@ func CompareDiff(ctx *context.APIContext) { Commits: apiCommits, }) } + +// downloadCompareDiffOrPatch writes a comparison's raw diff or patch to the response. +func downloadCompareDiffOrPatch(ctx *context.APIContext, compareInfo *git_service.CompareInfo, patch bool) { + ctx.Resp.Header().Set("Content-Type", "text/plain; charset=utf-8") + compareArg := compareInfo.BaseCommitID + compareInfo.CompareSeparator + compareInfo.HeadCommitID + + var err error + if patch { + err = compareInfo.HeadGitRepo.GetPatch(compareArg, ctx.Resp) + } else { + err = compareInfo.HeadGitRepo.GetDiff(compareArg, ctx.Resp) + } + if err != nil { + ctx.APIErrorInternal(err) + return + } +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 45735fc8fe3..5a9fa1c1d13 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -201,12 +201,12 @@ func newComparePageInfo() *comparePageInfoType { } // parseCompareInfo parse compare info between two commit for preparing comparing references -func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context) error { +func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context, compareParam string) error { baseRepo := ctx.Repo.Repository fileOnly := ctx.FormBool("file-only") // 1 Parse compare router param - compareReq := common.ParseCompareRouterParam(ctx.PathParam("*")) + compareReq := common.ParseCompareRouterParam(compareParam) // remove the check when we support compare with carets if compareReq.BaseOriRefSuffix != "" { @@ -545,7 +545,7 @@ func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repositor // CompareDiff show different from one commit to another commit func CompareDiff(ctx *context.Context) { comparePageInfo := newComparePageInfo() - err := comparePageInfo.parseCompareInfo(ctx) + err := comparePageInfo.parseCompareInfo(ctx, ctx.PathParam("*")) if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) { ctx.NotFound(nil) return @@ -605,6 +605,45 @@ func CompareDiff(ctx *context.Context) { ctx.HTML(http.StatusOK, tplCompare) } +// DownloadCompareDiff render a comparison's raw unified diff +func DownloadCompareDiff(ctx *context.Context) { + downloadCompareDiffOrPatch(ctx, false) +} + +// DownloadComparePatch render a comparison as a git format-patch +func DownloadComparePatch(ctx *context.Context) { + downloadCompareDiffOrPatch(ctx, true) +} + +func downloadCompareDiffOrPatch(ctx *context.Context, patch bool) { + // The route captures `basehead` separately so the `.diff`/`.patch` suffix is + // stripped from the catch-all `*` param parseCompareInfo would otherwise read. + cpi := newComparePageInfo() + if err := cpi.parseCompareInfo(ctx, ctx.PathParam("basehead")); err != nil { + if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) { + ctx.NotFound(nil) + } else { + ctx.ServerError("ParseCompareInfo", err) + } + return + } + ci := cpi.compareInfo + + ctx.Resp.Header().Set("Content-Type", "text/plain; charset=utf-8") + compareArg := ci.BaseCommitID + ci.CompareSeparator + ci.HeadCommitID + + var err error + if patch { + err = ci.HeadGitRepo.GetPatch(compareArg, ctx.Resp) + } else { + err = ci.HeadGitRepo.GetDiff(compareArg, ctx.Resp) + } + if err != nil { + ctx.ServerError("DownloadCompareDiffOrPatch", err) + return + } +} + func (cpi *comparePageInfoType) prepareCreatePullRequestPage(ctx *context.Context) { ci := cpi.compareInfo if cpi.allowCreatePull { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 650f94110ff..81283a25383 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1310,7 +1310,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CreateIssueForm) repo := ctx.Repo.Repository comparePageInfo := newComparePageInfo() - err := comparePageInfo.parseCompareInfo(ctx) + err := comparePageInfo.parseCompareInfo(ctx, ctx.PathParam("*")) if errors.Is(err, util.ErrNotExist) { ctx.JSONErrorNotFound() return diff --git a/routers/web/web.go b/routers/web/web.go index ee30b614c85..bde886177c3 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1269,9 +1269,12 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.TreeViewNodes) }) m.Get("/compare", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff) - m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists). - Get(repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff). - Post(reqSignIn, context.RepoMustNotBeArchived(), reqUnitPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost) + m.PathGroup("/compare/*", func(g *web.RouterPathGroup) { + g.MatchPath("GET", "/.diff", repo.MustBeNotEmpty, repo.DownloadCompareDiff) + g.MatchPath("GET", "/.patch", repo.MustBeNotEmpty, repo.DownloadComparePatch) + g.MatchPath("GET", "/<*:*>", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff) + g.MatchPath("POST", "/<*:*>", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, reqSignIn, context.RepoMustNotBeArchived(), reqUnitPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost) + }) m.Get("/pulls/new/*", repo.PullsNewRedirect) }, optSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}": repo code: find, compare, list diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fa41917aabe..37a358a26b6 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -8108,8 +8108,10 @@ }, "/repos/{owner}/{repo}/compare/{basehead}": { "get": { + "description": "By default returns JSON commit comparison information. The raw diff or patch can be\nrequested with the `output` query parameter set to `diff` or `patch` respectively.\n", "produces": [ - "application/json" + "application/json", + "text/plain" ], "tags": [ "repository" @@ -8133,10 +8135,20 @@ }, { "type": "string", - "description": "compare two branches or commits", + "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.", "name": "basehead", "in": "path", "required": true + }, + { + "enum": [ + "diff", + "patch" + ], + "type": "string", + "description": "return the raw comparison as `diff` or `patch` instead of JSON", + "name": "output", + "in": "query" } ], "responses": { diff --git a/templates/swagger/v1_openapi3_json.tmpl b/templates/swagger/v1_openapi3_json.tmpl index 68a495d5466..78295445268 100644 --- a/templates/swagger/v1_openapi3_json.tmpl +++ b/templates/swagger/v1_openapi3_json.tmpl @@ -19468,6 +19468,7 @@ }, "/repos/{owner}/{repo}/compare/{basehead}": { "get": { + "description": "By default returns JSON commit comparison information. The raw diff or patch can be\nrequested with the `output` query parameter set to `diff` or `patch` respectively.\n", "operationId": "repoCompareDiff", "parameters": [ { @@ -19489,13 +19490,25 @@ } }, { - "description": "compare two branches or commits", + "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.", "in": "path", "name": "basehead", "required": true, "schema": { "type": "string" } + }, + { + "description": "return the raw comparison as `diff` or `patch` instead of JSON", + "in": "query", + "name": "output", + "schema": { + "enum": [ + "diff", + "patch" + ], + "type": "string" + } } ], "responses": { diff --git a/tests/integration/api_repo_compare_test.go b/tests/integration/api_repo_compare_test.go index 0d8cec77767..5bb8cf6bde4 100644 --- a/tests/integration/api_repo_compare_test.go +++ b/tests/integration/api_repo_compare_test.go @@ -6,6 +6,7 @@ package integration import ( "net/http" "net/url" + "strings" "testing" auth_model "gitea.dev/models/auth" @@ -62,3 +63,113 @@ func TestAPICompareBranches(t *testing.T) { }) }) } + +func TestAPIDownloadCompareDiffOrPatch(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("BranchToBranchDiff", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b?output=diff").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + body := resp.Body.String() + assert.Contains(t, body, "diff --git ") + }) + + t.Run("BranchToBranchPatch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b?output=patch").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + body := resp.Body.String() + assert.True(t, strings.HasPrefix(body, "From "), "patch output should start with a format-patch header, got: %q", body[:min(40, len(body))]) + }) + + t.Run("CommitToCommitDiff", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/808038d2f71b0ab02099...c8e31bc7688741a5287f?output=diff").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + + t.Run("BranchToCommitDiff", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // 8babce96... is the head of remove-files-b; pairing it with add-csv guarantees a non-empty diff. + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...8babce967f21b9dfa6987f943b91093dac58a4f0?output=diff").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + + t.Run("TwoDotSeparator", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv..remove-files-b?output=diff").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + + t.Run("SlashedBranchName", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // user2/repo1's `feature/1` branch contains a slash; the route must match it + // without URL-encoding. master and feature/1 happen to share a SHA in the fixture, + // so we only assert the route resolves (200 OK) rather than checking diff content. + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/compare/master...feature/1?output=diff").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + }) + + t.Run("UnknownOutputReturnsJSON", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Only "diff"/"patch" switch to raw output; any other value falls through to JSON. + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b?output=foo").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + apiResp := DecodeJSON(t, resp, &api.Compare{}) + assert.Equal(t, 2, apiResp.TotalCommits) + }) + + t.Run("SingleRefImplicitBase", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // No `...`/`..` separator: parseCompareInfo defaults the base to the + // repo's PR target branch (master for repo20) and compares it against + // the given head. + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv?output=diff").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + + t.Run("PrivateRepoAnonymous", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // repo16 is private; an unauthenticated request must not leak its existence. + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo16/compare/master...good-sign?output=diff") + MakeRequest(t, req, http.StatusNotFound) + }) + + t.Run("CrossRepoFork", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user13 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 13}) + repo11 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11}) + user13Sess := loginUser(t, "user13") + user13Token := getTokenForLoggedInUser(t, user13Sess, auth_model.AccessTokenScopeWriteRepository) + + _, err := createFileInBranch(user13, repo11, createFileInBranchOptions{OldBranch: "master", NewBranch: "cross-repo-diff"}, map[string]string{"hello.txt": "hi\n"}) + require.NoError(t, err) + + req := NewRequest(t, "GET", "/api/v1/repos/user12/repo10/compare/master...user13:cross-repo-diff?output=diff").AddTokenAuth(user13Token) + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + }) +} diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index ac2e014d92f..d5f5bc2cc27 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -167,6 +167,53 @@ Hello from 2 assert.Equal(t, 0, htmlDoc.doc.Find(".pullrequest-form").Length()) } +func TestCompareDownloadDiffOrPatch(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user2") + + t.Run("BranchToBranchDiff", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user2/repo20/compare/add-csv...remove-files-b.diff") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + + t.Run("BranchToBranchPatch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user2/repo20/compare/add-csv...remove-files-b.patch") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + assert.True(t, strings.HasPrefix(resp.Body.String(), "From "), "patch output should start with a format-patch header") + }) + + t.Run("SingleRefImplicitBase", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user2/repo20/compare/add-csv.diff") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "diff --git ") + }) + + t.Run("InvalidBaseRef", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user2/repo20/compare/does-not-exist...remove-files-b.diff") + session.MakeRequest(t, req, http.StatusNotFound) + }) + + t.Run("PrivateRepoAnonymous", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // repo16 is private; an unauthenticated request must not leak its existence. + req := NewRequest(t, "GET", "/user2/repo16/compare/master...good-sign.diff") + MakeRequest(t, req, http.StatusNotFound) + }) +} + func TestCompareCodeExpand(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})