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>

3 files changed, 11 insertions, 18 deletions

diff --git a/models/pull.go b/models/pull.go
index 3717878f42..c737688410 100644
--- 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 5eaeb60e67..055d5a5538 100644
--- 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 5600252286..42c5818e33 100644
--- 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