Shiloh/githaven
Shiloh/githaven
1
0
Fork 1
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix issue & comment history bugs (#29525)

Browse Source 
* Follow #17746: `HasIssueContentHistory` should use expr builder to
make sure zero value (0) be respected.
* Add "doer" check to make sure `canSoftDeleteContentHistory` only be
called by sign-in users.
This commit is contained in:
wxiaoguang 2024-03-02 00:46:02 +08:00 committed by GitHub
parent 3b99066aa8
commit 4b8293aa09
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 26 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
models/issues/content_history.go
View File

@ -172,13 +172,9 @@ func FetchIssueContentHistoryList(dbCtx context.Context, issueID, commentID int6
// HasIssueContentHistory check if a ContentHistory entry exists // HasIssueContentHistory check if a ContentHistory entry exists
func HasIssueContentHistory(dbCtx context.Context, issueID, commentID int64) (bool, error) { func HasIssueContentHistory(dbCtx context.Context, issueID, commentID int64) (bool, error) {
exists, err := db.GetEngine(dbCtx).Cols("id").Exist(&ContentHistory{ exists, err := db.GetEngine(dbCtx).Where(builder.Eq{"issue_id": issueID, "comment_id": commentID}).Exist(&ContentHistory{})
IssueID: issueID,
CommentID: commentID,
})
if err != nil { if err != nil {
log.Error("can not fetch issue content history. err=%v", err) return false, fmt.Errorf("can not check issue content history. err: %w", err)
return false, err
} }
return exists, err return exists, err
} }

19
models/issues/content_history_test.go
View File

@ -78,3 +78,22 @@ func TestContentHistory(t *testing.T) {
assert.EqualValues(t, 7, list2[1].HistoryID) assert.EqualValues(t, 7, list2[1].HistoryID)
assert.EqualValues(t, 4, list2[2].HistoryID) assert.EqualValues(t, 4, list2[2].HistoryID)
} }
func TestHasIssueContentHistoryForCommentOnly(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
_ = db.TruncateBeans(db.DefaultContext, &issues_model.ContentHistory{})
hasHistory1, _ := issues_model.HasIssueContentHistory(db.DefaultContext, 10, 0)
assert.False(t, hasHistory1)
hasHistory2, _ := issues_model.HasIssueContentHistory(db.DefaultContext, 10, 100)
assert.False(t, hasHistory2)
_ = issues_model.SaveIssueContentHistory(db.DefaultContext, 1, 10, 100, timeutil.TimeStampNow(), "c-a", true)
_ = issues_model.SaveIssueContentHistory(db.DefaultContext, 1, 10, 100, timeutil.TimeStampNow().Add(5), "c-b", false)
hasHistory1, _ = issues_model.HasIssueContentHistory(db.DefaultContext, 10, 0)
assert.False(t, hasHistory1)
hasHistory2, _ = issues_model.HasIssueContentHistory(db.DefaultContext, 10, 100)
assert.True(t, hasHistory2)
}

6
routers/web/repo/issue_content_history.go
View File

@ -94,7 +94,7 @@ func canSoftDeleteContentHistory(ctx *context.Context, issue *issues_model.Issue
// CanWrite means the doer can manage the issue/PR list // CanWrite means the doer can manage the issue/PR list
if ctx.Repo.IsOwner() || ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { if ctx.Repo.IsOwner() || ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
canSoftDelete = true canSoftDelete = true
} else { } else if ctx.Doer != nil {
// for read-only users, they could still post issues or comments, // 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: // they should be able to delete the history related to their own issue/comment, a case is:
// 1. the user posts some sensitive data // 1. the user posts some sensitive data
@ -186,6 +186,10 @@ func SoftDeleteContentHistory(ctx *context.Context) {
if ctx.Written() { if ctx.Written() {
return return
} }
if ctx.Doer == nil {
ctx.NotFound("Require SignIn", nil)
return
}
commentID := ctx.FormInt64("comment_id") commentID := ctx.FormInt64("comment_id")
historyID := ctx.FormInt64("history_id") historyID := ctx.FormInt64("history_id")