Skip to content

Commit 6995be6

Browse files
Fix stange behavior of DownloadPullDiffOrPatch in incorect index (#17223) (#17227)
Fix GetPullRequestByIndex by validate index > 1 Signed-off-by: Danila Kryukov <[email protected]> Co-authored-by: a1012112796 <[email protected]>
1 parent 28971c7 commit 6995be6

File tree

3 files changed

+11
-18
lines changed

3 files changed

+11
-18
lines changed

models/pull.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,9 @@ func GetLatestPullRequestByHeadInfo(repoID int64, branch string) (*PullRequest,
502502

503503
// GetPullRequestByIndex returns a pull request by the given index
504504
func GetPullRequestByIndex(repoID, index int64) (*PullRequest, error) {
505+
if index < 1 {
506+
return nil, ErrPullRequestNotExist{}
507+
}
505508
pr := &PullRequest{
506509
BaseRepoID: repoID,
507510
Index: index,

models/pull_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ func TestGetPullRequestByIndex(t *testing.T) {
133133
_, err = GetPullRequestByIndex(9223372036854775807, 9223372036854775807)
134134
assert.Error(t, err)
135135
assert.True(t, IsErrPullRequestNotExist(err))
136+
137+
_, err = GetPullRequestByIndex(1, 0)
138+
assert.Error(t, err)
139+
assert.True(t, IsErrPullRequestNotExist(err))
136140
}
137141

138142
func TestGetPullRequestByID(t *testing.T) {

routers/web/repo/pull.go

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,30 +1307,16 @@ func DownloadPullPatch(ctx *context.Context) {
13071307

13081308
// DownloadPullDiffOrPatch render a pull's raw diff or patch
13091309
func DownloadPullDiffOrPatch(ctx *context.Context, patch bool) {
1310-
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
1310+
pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
13111311
if err != nil {
1312-
if models.IsErrIssueNotExist(err) {
1313-
ctx.NotFound("GetIssueByIndex", err)
1312+
if models.IsErrPullRequestNotExist(err) {
1313+
ctx.NotFound("GetPullRequestByIndex", err)
13141314
} else {
1315-
ctx.ServerError("GetIssueByIndex", err)
1315+
ctx.ServerError("GetPullRequestByIndex", err)
13161316
}
13171317
return
13181318
}
13191319

1320-
// Return not found if it's not a pull request
1321-
if !issue.IsPull {
1322-
ctx.NotFound("DownloadPullDiff",
1323-
fmt.Errorf("Issue is not a pull request"))
1324-
return
1325-
}
1326-
1327-
if err = issue.LoadPullRequest(); err != nil {
1328-
ctx.ServerError("LoadPullRequest", err)
1329-
return
1330-
}
1331-
1332-
pr := issue.PullRequest
1333-
13341320
if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch); err != nil {
13351321
ctx.ServerError("DownloadDiffOrPatch", err)
13361322
return

0 commit comments

Comments
 (0)