Skip to content

Commit 20eaca6

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

File tree

3 files changed

+11
-17
lines changed

3 files changed

+11
-17
lines changed

models/pull.go

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

523523
// GetPullRequestByIndex returns a pull request by the given index
524524
func GetPullRequestByIndex(repoID, index int64) (*PullRequest, error) {
525+
if index < 1 {
526+
return nil, ErrPullRequestNotExist{}
527+
}
525528
pr := &PullRequest{
526529
BaseRepoID: repoID,
527530
Index: index,

models/pull_test.go

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

139143
func TestGetPullRequestByID(t *testing.T) {

routers/web/repo/pull.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,29 +1322,16 @@ func DownloadPullPatch(ctx *context.Context) {
13221322

13231323
// DownloadPullDiffOrPatch render a pull's raw diff or patch
13241324
func DownloadPullDiffOrPatch(ctx *context.Context, patch bool) {
1325-
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
1325+
pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
13261326
if err != nil {
1327-
if models.IsErrIssueNotExist(err) {
1328-
ctx.NotFound("GetIssueByIndex", err)
1327+
if models.IsErrPullRequestNotExist(err) {
1328+
ctx.NotFound("GetPullRequestByIndex", err)
13291329
} else {
1330-
ctx.ServerError("GetIssueByIndex", err)
1330+
ctx.ServerError("GetPullRequestByIndex", err)
13311331
}
13321332
return
13331333
}
13341334

1335-
// Return not found if it's not a pull request
1336-
if !issue.IsPull {
1337-
ctx.NotFound("DownloadPullDiff",
1338-
fmt.Errorf("Issue is not a pull request"))
1339-
return
1340-
}
1341-
1342-
if err = issue.LoadPullRequest(); err != nil {
1343-
ctx.ServerError("LoadPullRequest", err)
1344-
return
1345-
}
1346-
1347-
pr := issue.PullRequest
13481335
binary := ctx.FormBool("binary")
13491336

13501337
if err := pull_service.DownloadDiffOrPatch(pr, ctx, patch, binary); err != nil {

0 commit comments

Comments
 (0)