From bb6edb2eed7a003bce6fb87e5e24bd7a511e72f2 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 25 Apr 2024 14:22:02 +0800 Subject: [PATCH] Fix a panic bug when head repository deleting (#30674) (#30676) Backport #30674 & #30679 by @lunny When visiting a pull request files which head repository has been deleted, it will panic because headrepo is nil. --------- Co-authored-by: Lunny Xiao --- routers/web/repo/pull.go | 26 ++++++++--------- tests/integration/pull_compare_test.go | 40 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a0a8e5410c..acd05b6af5 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -865,21 +865,21 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi if pull.HeadRepo != nil { ctx.Data["SourcePath"] = pull.HeadRepo.Link() + "/src/branch/" + util.PathEscapeSegments(pull.HeadBranch) - } - if !pull.HasMerged && ctx.Doer != nil { - perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) - if err != nil { - ctx.ServerError("GetUserRepoPermission", err) - return - } + if !pull.HasMerged && ctx.Doer != nil { + perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) + if err != nil { + ctx.ServerError("GetUserRepoPermission", err) + return + } - if perm.CanWrite(unit.TypeCode) || issues_model.CanMaintainerWriteToBranch(ctx, perm, pull.HeadBranch, ctx.Doer) { - ctx.Data["CanEditFile"] = true - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file") - ctx.Data["HeadRepoLink"] = pull.HeadRepo.Link() - ctx.Data["HeadBranchName"] = pull.HeadBranch - ctx.Data["BackToLink"] = setting.AppSubURL + ctx.Req.URL.RequestURI() + if perm.CanWrite(unit.TypeCode) || issues_model.CanMaintainerWriteToBranch(ctx, perm, pull.HeadBranch, ctx.Doer) { + ctx.Data["CanEditFile"] = true + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file") + ctx.Data["HeadRepoLink"] = pull.HeadRepo.Link() + ctx.Data["HeadBranchName"] = pull.HeadBranch + ctx.Data["BackToLink"] = setting.AppSubURL + ctx.Req.URL.RequestURI() + } } } } diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go index 5ce8ea3031..39d9103dfd 100644 --- a/tests/integration/pull_compare_test.go +++ b/tests/integration/pull_compare_test.go @@ -4,9 +4,17 @@ package integration import ( + "fmt" "net/http" + "net/url" "testing" + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -32,4 +40,36 @@ func TestPullCompare(t *testing.T) { doc := NewHTMLParser(t, resp.Body) editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length() assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none") + + onGiteaRun(t, func(t *testing.T, u *url.URL) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testCreateBranch(t, session, "user1", "repo1", "branch/master", "master1", http.StatusSeeOther) + testEditFile(t, session, "user1", "repo1", "master1", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", "repo1", false, "master", "master1", "This is a pull title") + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + issueIndex := unittest.AssertExistsAndLoadBean(t, &issues_model.IssueIndex{GroupID: repo1.ID}, unittest.OrderBy("group_id ASC")) + prFilesURL := fmt.Sprintf("/user2/repo1/pulls/%d/files", issueIndex.MaxIndex) + req = NewRequest(t, "GET", prFilesURL) + resp = session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length() + assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none") + + repoForked := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // delete the head repository and revisit the PR diff view + err := repo_service.DeleteRepositoryDirectly(db.DefaultContext, user2, repoForked.ID) + assert.NoError(t, err) + + req = NewRequest(t, "GET", prFilesURL) + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + editButtonCount = doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length() + assert.EqualValues(t, editButtonCount, 0, "Expected not to find a button to edit a file in the PR diff view because head repository has been deleted") + }) }