From 30d168bcc867387f3c94582a4668cce62f77c171 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
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.
(cherry picked from commit 0b5db0dcc608e9a9e79ead094a20a7775c4f9559)
---
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)
+ })
+}