From 0b5db0dcc608e9a9e79ead094a20a7775c4f9559 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 13 Jan 2024 11:55:33 +0100 Subject: [PATCH] [GITEA] Fix panic in `canSoftDeleteContentHistory` - It's possible that `canSoftDeleteContentHistory` is called without `ctx.Doer` being set, such as an anonymous user requesting the `/content-history/detail` endpoint. - Add a simple condition to always set to `canSoftDelete` to false if an anonymous user is requesting this, this avoids a panic in the code that assumes `ctx.Doer` is set. - Added integration testing. --- routers/web/repo/issue_content_history.go | 2 + .../issue_content_history.yml | 17 ++++++ tests/integration/issue_test.go | 52 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 tests/integration/fixtures/TestGetContentHistory/issue_content_history.yml diff --git a/routers/web/repo/issue_content_history.go b/routers/web/repo/issue_content_history.go index 0f376db145..31e6ac608c 100644 --- a/routers/web/repo/issue_content_history.go +++ b/routers/web/repo/issue_content_history.go @@ -94,6 +94,8 @@ func canSoftDeleteContentHistory(ctx *context.Context, issue *issues_model.Issue // CanWrite means the doer can manage the issue/PR list if ctx.Repo.IsOwner() || ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { canSoftDelete = true + } else if ctx.Doer == nil { + canSoftDelete = false } else { // for read-only users, they could still post issues or comments, // they should be able to delete the history related to their own issue/comment, a case is: diff --git a/tests/integration/fixtures/TestGetContentHistory/issue_content_history.yml b/tests/integration/fixtures/TestGetContentHistory/issue_content_history.yml new file mode 100644 index 0000000000..6633b50374 --- /dev/null +++ b/tests/integration/fixtures/TestGetContentHistory/issue_content_history.yml @@ -0,0 +1,17 @@ +- + id: 1 + issue_id: 1 + comment_id: 3 + edited_unix: 1687612839 + content_text: Original Text + is_first_created: true + is_deleted: false + +- + id: 2 + issue_id: 1 + comment_id: 3 + edited_unix: 1687612840 + content_text: "meh..." # This has to be consistent with comment.yml + is_first_created: false + is_deleted: false diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index de4113a37b..4da8aeae0c 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -718,3 +718,55 @@ func TestIssueReferenceURL(t *testing.T) { ref, _ = htmlDoc.Find(`.timeline-item.comment:not(.first) .reference-issue`).Attr("data-reference") assert.EqualValues(t, "/user2/repo1/issues/1#issuecomment-2", ref) } + +func TestGetContentHistory(t *testing.T) { + defer tests.AddFixtures("tests/integration/fixtures/TestGetContentHistory/")() + defer tests.PrepareTestEnv(t)() + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID}) + issueURL := fmt.Sprintf("%s/issues/%d", repo.FullName(), issue.Index) + contentHistory := unittest.AssertExistsAndLoadBean(t, &issues_model.ContentHistory{ID: 2, IssueID: issue.ID}) + contentHistoryURL := fmt.Sprintf("%s/issues/%d/content-history/detail?comment_id=%d&history_id=%d", repo.FullName(), issue.Index, contentHistory.CommentID, contentHistory.ID) + + type contentHistoryResp struct { + CanSoftDelete bool `json:"canSoftDelete"` + HistoryID int `json:"historyId"` + PrevHistoryID int `json:"prevHistoryId"` + } + + testCase := func(t *testing.T, session *TestSession, canDelete bool) { + t.Helper() + contentHistoryURL := contentHistoryURL + "&_csrf=" + GetCSRF(t, session, issueURL) + + req := NewRequest(t, "GET", contentHistoryURL) + resp := session.MakeRequest(t, req, http.StatusOK) + + var respJSON contentHistoryResp + DecodeJSON(t, resp, &respJSON) + + assert.EqualValues(t, canDelete, respJSON.CanSoftDelete) + assert.EqualValues(t, contentHistory.ID, respJSON.HistoryID) + assert.EqualValues(t, contentHistory.ID-1, respJSON.PrevHistoryID) + } + + t.Run("Anonymous", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + testCase(t, emptyTestSession(t), false) + }) + + t.Run("Another user", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + testCase(t, loginUser(t, "user8"), false) + }) + + t.Run("Repo owner", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + testCase(t, loginUser(t, "user2"), true) + }) + + t.Run("Poster", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + testCase(t, loginUser(t, "user5"), true) + }) +}