Fix stange behavior of DownloadPullDiffOrPatch in incorect index (#17223) (#17227)

Fix GetPullRequestByIndex by validate index > 1

Signed-off-by: Danila Kryukov <pricly_yellow@dismail.de>
Co-authored-by: a1012112796 <1012112796@qq.com>

diff --git a/models/pull.go b/models/pull.go
index 3717878f4201f15636330526f87ad0f45e3ec6a7..c737688410d7cbeca1d06f2cae20db43ca7c627a 100644 (file)
--- a/models/pull.go
+++ b/models/pull.go
@@ -502,6 +502,9 @@ func GetLatestPullRequestByHeadInfo(repoID int64, branch string) (*PullRequest,
 
 // GetPullRequestByIndex returns a pull request by the given index
 func GetPullRequestByIndex(repoID, index int64) (*PullRequest, error) {
+       if index < 1 {
+               return nil, ErrPullRequestNotExist{}
+       }
        pr := &PullRequest{
                BaseRepoID: repoID,
                Index:      index,

diff --git a/models/pull_test.go b/models/pull_test.go
index 5eaeb60e67281a9704c15adaba30b822dfbe7e15..055d5a55381283787cd67c40c117279e4c523193 100644 (file)
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -133,6 +133,10 @@ func TestGetPullRequestByIndex(t *testing.T) {
        _, err = GetPullRequestByIndex(9223372036854775807, 9223372036854775807)
        assert.Error(t, err)
        assert.True(t, IsErrPullRequestNotExist(err))
+
+       _, err = GetPullRequestByIndex(1, 0)
+       assert.Error(t, err)
+       assert.True(t, IsErrPullRequestNotExist(err))
 }
 
 func TestGetPullRequestByID(t *testing.T) {

diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 5600252286f381659e4da55708db8113a61f6d52..42c5818e33bd4767939a37f8c9060d4e2f04aa95 100644 (file)
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -1307,30 +1307,16 @@ func DownloadPullPatch(ctx *context.Context) {
 
 // DownloadPullDiffOrPatch render a pull's raw diff or patch
 func DownloadPullDiffOrPatch(ctx *context.Context, patch bool) {
-       issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
+       pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
        if err != nil {
-               if models.IsErrIssueNotExist(err) {
-                       ctx.NotFound("GetIssueByIndex", err)
+               if models.IsErrPullRequestNotExist(err) {
+                       ctx.NotFound("GetPullRequestByIndex", err)
                } else {
-                       ctx.ServerError("GetIssueByIndex", err)
+                       ctx.ServerError("GetPullRequestByIndex", err)
                }
                return
        }
 
-       // Return not found if it's not a pull request
-       if !issue.IsPull {
-               ctx.NotFound("DownloadPullDiff",
-                       fmt.Errorf("Issue is not a pull request"))
-               return
-       }
-
-       if err = issue.LoadPullRequest(); err != nil {
-               ctx.ServerError("LoadPullRequest", err)
-               return
-       }
-
-       pr := issue.PullRequest
-
        if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch); err != nil {
                ctx.ServerError("DownloadDiffOrPatch", err)
                return