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

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 004af62f035ce44408ab66bdfc4f7ed08e6cbd5c..38f2c1b8ce414806dbc7f87a359d01881195061d 100644 (file)
--- a/models/pull.go
+++ b/models/pull.go
@@ -522,6 +522,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 2b7ef2f664afc3f38b412e07e89b5b9aa1789a85..173977aafeebf0a4c553f416314e798e0fed18d6 100644 (file)
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -134,6 +134,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 9c9cf0f21daed23e54532ff2d7d475b55192062b..dde556135177d2b35f5f1d071a3407610a33a474 100644 (file)
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -1322,29 +1322,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
        binary := ctx.FormBool("binary")
 
        if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch, binary); err != nil {