From 24fbf4e0592713633144c3fc9a04f64d790f99b0 Mon Sep 17 00:00:00 2001 From: CaiCandong <50507092+CaiCandong@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:43:18 +0800 Subject: [PATCH] Fix nil pointer dereference error when open link with invalid pull index (#26353) fix #26331 Before: ![image](https://github.com/go-gitea/gitea/assets/50507092/028e6944-84d1-4404-80b6-4a4accdc7d0a) After: ![image](https://github.com/go-gitea/gitea/assets/50507092/b78978f9-e77f-459f-96e1-3a1f36ec8abe) --- routers/web/repo/pull.go | 55 +++++++++++++++++---------------- routers/web/repo/pull_review.go | 4 +-- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0be8bede74..be4e9711e7 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) (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) { @@ -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,14 +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) + 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 } @@ -702,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{} @@ -735,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 @@ -779,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 @@ -1016,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 { @@ -1101,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 } @@ -1308,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 } @@ -1447,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 c2271750c4..3e433dcf4d 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