fix(web): avoid compare output route collisions (#40307)

Allow compare refs named diff or patch to use the normal compare page
while preserving explicit .diff and .patch downloads.

Compare refs ending in diff or patch were incorrectly matched as raw
.diff/.patch routes. This caused normal compare pages such as
main...diff and main...patch to return an empty or invalid response.

The route matching is now constrained so valid raw diff/patch downloads
continue working while branches named diff or patch use the normal
compare page.

Assisted-by: Agent:GPT-5.6 Luna

Fixes https://github.com/go-gitea/gitea/issues/40303

---------

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

routers/web/repo/compare.go

@@ -148,10 +148,11 @@ func setCsvCompareContext(ctx *context.Context) {
 }
 
 type comparePageInfoType struct {
-	compareInfo      *git_service.CompareInfo
-	nothingToCompare bool
-	allowCreatePull  bool
-	allowEmptyPull   bool
+	compareInfo           *git_service.CompareInfo
+	headRefDownloadSuffix string
+	nothingToCompare      bool
+	allowCreatePull       bool
+	allowEmptyPull        bool
 }
 
 func newComparePageInfo() *comparePageInfoType {
@@ -204,6 +205,18 @@ func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context, comparePa
 	}
 
 	headRef, err := common.ResolveRefWithSuffix(ctx, headGitRepo, headRefName, compareReq.HeadOriRefSuffix)
+	if errors.Is(err, util.ErrNotExist) && compareReq.HeadOriRefSuffix == "" {
+		// try to parse "base...head.patch" or "base...head.diff" download suffix
+		var hasDownloadSuffix bool
+		if headRefName, hasDownloadSuffix = strings.CutSuffix(headRefName, ".patch"); hasDownloadSuffix {
+			cpi.headRefDownloadSuffix = ".patch"
+		} else if headRefName, hasDownloadSuffix = strings.CutSuffix(headRefName, ".diff"); hasDownloadSuffix {
+			cpi.headRefDownloadSuffix = ".diff"
+		}
+		if hasDownloadSuffix {
+			headRef, err = common.ResolveRefWithSuffix(ctx, headGitRepo, headRefName, "")
+		}
+	}
 	if err != nil {
 		return err
 	}
@@ -522,6 +535,12 @@ func CompareDiff(ctx *context.Context) {
 		ctx.ServerErrorAuto(err)
 		return
 	}
+
+	if comparePageInfo.headRefDownloadSuffix != "" {
+		downloadCompareDiffOrPatch(ctx, comparePageInfo.compareInfo, comparePageInfo.headRefDownloadSuffix)
+		return
+	}
+
 	ci := comparePageInfo.compareInfo
 	ctx.Data["PageIsViewCode"] = true
 	ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
@@ -577,35 +596,12 @@ 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()
-		} else {
-			ctx.ServerErrorAuto(err)
-		}
-		return
-	}
-	ci := cpi.compareInfo
-
+func downloadCompareDiffOrPatch(ctx *context.Context, ci *git_service.CompareInfo, downloadSuffix string) {
 	ctx.Resp.Header().Set("Content-Type", "text/plain; charset=utf-8")
 	compareArg := ci.BaseCommitID + ci.CompareSeparator + ci.HeadCommitID
 
 	var err error
-	if patch {
+	if downloadSuffix == ".patch" {
 		err = ci.HeadGitRepo.GetPatch(ctx, compareArg, ctx.Resp)
 	} else {
 		err = ci.HeadGitRepo.GetDiff(ctx, compareArg, ctx.Resp)

routers/web/web.go

@@ -1280,12 +1280,8 @@ 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.PathGroup("/compare/*", func(g *web.RouterPathGroup) {
-			g.MatchPath("GET", "/<basehead:*>.diff", repo.MustBeNotEmpty, repo.DownloadCompareDiff)
-			g.MatchPath("GET", "/<basehead:*>.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("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff)
+		m.Post("/compare/*", 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

tests/integration/compare_test.go

@@ -236,6 +236,22 @@ func TestCompare(t *testing.T) {
 			assert.Contains(t, resp.Body.String(), "diff --git ")
 		})
 
+		t.Run("BranchNamesConflictingWithOutputSuffix", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 31})
+			for _, branch := range []string{"diff", "patch", "feature.diff", "feature.patch"} {
+				_, _, err := gitcmd.NewCommand("branch").AddDynamicArguments(branch, "add-csv").WithRepo(repo).RunStdString(t.Context())
+				require.NoError(t, err)
+			}
+
+			for _, branch := range []string{"diff", "patch", "feature.diff", "feature.patch"} {
+				req := NewRequest(t, "GET", "/user2/repo20/compare/master..."+branch)
+				resp := session.MakeRequest(t, req, http.StatusOK)
+				assert.True(t, test.IsNormalPageCompleted(resp.Body.String()), "branch %q should render the compare page", branch)
+			}
+		})
+
 		t.Run("InvalidBaseRef", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()