From ca5bd599fd7cc006e77124b070414e58dd6deb15 Mon Sep 17 00:00:00 2001 From: CaiCandong <1290147055@qq.com> Date: Sun, 6 Aug 2023 10:51:46 +0800 Subject: [PATCH 1/3] fix nil pointer dereference --- routers/web/repo/pull.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0be8bede74f0..4e2f176c8925 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -362,6 +362,9 @@ func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) { // GetPullDiffStats get Pull Requests diff stats func GetPullDiffStats(ctx *context.Context) { issue := checkPullInfo(ctx) + if issue == nil { + return + } pull := issue.PullRequest mergeBaseCommitID := GetMergedBaseCommitID(ctx, issue) From fb209c55aa4fbe2cab352e8b59a39fcaa7973bf8 Mon Sep 17 00:00:00 2001 From: CaiCandong <1290147055@qq.com> Date: Sun, 6 Aug 2023 15:40:53 +0800 Subject: [PATCH 2/3] refactor `checkPullInfo` to `getPullInfo` --- routers/web/repo/pull.go | 54 ++++++++++++++++----------------- routers/web/repo/pull_review.go | 4 +-- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 4e2f176c8925..d198e4653c54 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -299,7 +299,7 @@ func ForkPost(ctx *context.Context) { ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) } -func checkPullInfo(ctx *context.Context) *issues_model.Issue { +func getPullInfo(ctx *context.Context) (*issues_model.Issue, bool) { issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if issues_model.IsErrIssueNotExist(err) { @@ -307,43 +307,43 @@ func checkPullInfo(ctx *context.Context) *issues_model.Issue { } else { ctx.ServerError("GetIssueByIndex", err) } - return nil + return nil, false } if err = issue.LoadPoster(ctx); err != nil { ctx.ServerError("LoadPoster", err) - return nil + return nil, false } if err := issue.LoadRepo(ctx); err != nil { ctx.ServerError("LoadRepo", err) - return nil + return nil, false } ctx.Data["Title"] = fmt.Sprintf("#%d - %s", issue.Index, issue.Title) ctx.Data["Issue"] = issue if !issue.IsPull { ctx.NotFound("ViewPullCommits", nil) - return nil + return nil, false } if err = issue.LoadPullRequest(ctx); err != nil { ctx.ServerError("LoadPullRequest", err) - return nil + return nil, false } if err = issue.PullRequest.LoadHeadRepo(ctx); err != nil { ctx.ServerError("LoadHeadRepo", err) - return nil + return nil, false } if ctx.IsSigned { // Update issue-user. if err = activities_model.SetIssueReadBy(ctx, issue.ID, ctx.Doer.ID); err != nil { ctx.ServerError("ReadBy", err) - return nil + return nil, false } } - return issue + return issue, true } func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) { @@ -361,17 +361,15 @@ func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) { // GetPullDiffStats get Pull Requests diff stats func GetPullDiffStats(ctx *context.Context) { - issue := checkPullInfo(ctx) - if issue == nil { + issue, ok := getPullInfo(ctx) + if !ok { return } pull := issue.PullRequest mergeBaseCommitID := GetMergedBaseCommitID(ctx, issue) - if ctx.Written() { - return - } else if mergeBaseCommitID == "" { + if mergeBaseCommitID == "" { ctx.NotFound("PullFiles", nil) return } @@ -705,8 +703,8 @@ type pullCommitList struct { // GetPullCommits get all commits for given pull request func GetPullCommits(ctx *context.Context) { - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } resp := &pullCommitList{} @@ -738,8 +736,8 @@ func ViewPullCommits(ctx *context.Context) { ctx.Data["PageIsPullList"] = true ctx.Data["PageIsPullCommits"] = true - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } pull := issue.PullRequest @@ -782,8 +780,8 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["PageIsPullList"] = true ctx.Data["PageIsPullFiles"] = true - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } pull := issue.PullRequest @@ -1019,8 +1017,8 @@ func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) { // UpdatePullRequest merge PR's baseBranch into headBranch func UpdatePullRequest(ctx *context.Context) { - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } if issue.IsClosed { @@ -1104,8 +1102,8 @@ func UpdatePullRequest(ctx *context.Context) { // MergePullRequest response for merging pull request func MergePullRequest(ctx *context.Context) { form := web.GetForm(ctx).(*forms.MergePullRequestForm) - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } @@ -1311,8 +1309,8 @@ func MergePullRequest(ctx *context.Context) { // CancelAutoMergePullRequest cancels a scheduled pr func CancelAutoMergePullRequest(ctx *context.Context) { - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } @@ -1450,8 +1448,8 @@ func CompareAndPullRequestPost(ctx *context.Context) { // CleanUpPullRequest responses for delete merged branch when PR has been merged func CleanUpPullRequest(ctx *context.Context) { - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index c2271750c4d1..3e433dcf4d7a 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -259,8 +259,8 @@ type viewedFilesUpdate struct { func UpdateViewedFiles(ctx *context.Context) { // Find corresponding PR - issue := checkPullInfo(ctx) - if ctx.Written() { + issue, ok := getPullInfo(ctx) + if !ok { return } pull := issue.PullRequest From b12fbe658d68df286faffbd9bda2988b13e6f302 Mon Sep 17 00:00:00 2001 From: CaiCandong <50507092+CaiCandong@users.noreply.github.com> Date: Sun, 6 Aug 2023 21:05:28 +0800 Subject: [PATCH 3/3] Update named return values Co-authored-by: delvh --- routers/web/repo/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index d198e4653c54..be4e9711e74d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -299,7 +299,7 @@ func ForkPost(ctx *context.Context) { ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) } -func getPullInfo(ctx *context.Context) (*issues_model.Issue, bool) { +func getPullInfo(ctx *context.Context) (issue *issues_model.Issue, ok bool) { issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if issues_model.IsErrIssueNotExist(err) {