diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index a3cb88e76a63f..a16fbbdbef2a1 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -79,6 +79,10 @@ func NewComment(ctx *context.Context) { var comment *issues_model.Comment defer func() { + if ctx.Written() { + return + } + // Check if issue admin/poster changes the status of issue. if (ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) && (form.Status == "reopen" || form.Status == "close") && diff --git a/services/issue/comments.go b/services/issue/comments.go index 3ce2e2a5e13db..9c956fcc097ab 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -78,13 +78,14 @@ func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_m mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, comment.Content) if err != nil { - return nil, err + log.Error("FindAndUpdateIssueMentions: %v", err) } // reload issue to ensure it has the latest data, especially the number of comments - issue, err = issues_model.GetIssueByID(ctx, issue.ID) - if err != nil { - return nil, err + if reloadedIssue, err := issues_model.GetIssueByID(ctx, issue.ID); err != nil { + log.Error("GetIssueByID: %v", err) + } else { + issue = reloadedIssue } notify_service.CreateIssueComment(ctx, doer, repo, issue, comment, mentions) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index a9101d7b7efe2..fe80210ad7748 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/indexer/issues" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -248,6 +249,40 @@ func TestIssueCommentClose(t *testing.T) { assert.Equal(t, "Description", val) } +// Test that error responses from NewComment are clean JSON without +// appended redirect JSON from the deferred function. +func TestIssueCommentErrorResponse(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // user34 is blocked by user2 (repo1 owner), so commenting should fail + session := loginUser(t, "the_34-user.with.all.allowedChars") + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + + req := NewRequestWithValues(t, "POST", "/user2/repo1/issues/1/comments", map[string]string{ + "content": "blocked comment", + }) + resp := session.MakeRequest(t, req, http.StatusBadRequest) + + // Before the fix, the response body contained both a JSON error and a + // JSON redirect appended by the deferred function, producing an invalid + // response. Verify the body is valid JSON with only the error. + var result map[string]any + err := json.Unmarshal(resp.Body.Bytes(), &result) + assert.NoError(t, err, "response body should be valid JSON, got: %s", resp.Body.String()) + assert.Contains(t, result, "errorMessage") + assert.NotContains(t, result, "redirect") + + // Verify no comment was created + comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ + IssueID: issue.ID, + Type: issues_model.CommentTypeComment, + }) + assert.NoError(t, err) + for _, c := range comments { + assert.NotEqual(t, "blocked comment", c.Content) + } +} + func TestIssueCommentDelete(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2")