From 6995be66e736831e09568676c06a581ab2978cf8 Mon Sep 17 00:00:00 2001 From: pricly-yellow <79628427+pricly-yellow@users.noreply.github.com> Date: Wed, 6 Oct 2021 01:16:22 +0700 Subject: [PATCH] Fix stange behavior of DownloadPullDiffOrPatch in incorect index (#17223) (#17227) Fix GetPullRequestByIndex by validate index > 1 Signed-off-by: Danila Kryukov Co-authored-by: a1012112796 <1012112796@qq.com> --- models/pull.go | 3 +++ models/pull_test.go | 4 ++++ routers/web/repo/pull.go | 22 ++++------------------ 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/models/pull.go b/models/pull.go index 3717878f4..c73768841 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 5eaeb60e6..055d5a553 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 560025228..42c5818e3 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