From 3b06675811cb3cce71447c47434661757e1ea38a Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 1 Oct 2021 17:24:43 +0200 Subject: [PATCH] Always set a unique Message-ID header. (#17206) --- models/issue.go | 13 ------------- services/mailer/mail.go | 27 ++++++++++++++++++++------- services/mailer/mail_test.go | 16 ++++++++++------ 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/models/issue.go b/models/issue.go index b8c7053b2d..9b02a83900 100644 --- a/models/issue.go +++ b/models/issue.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/references" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -415,18 +414,6 @@ func (issue *Issue) HasLabel(labelID int64) bool { return issue.hasLabel(db.GetEngine(db.DefaultContext), labelID) } -// ReplyReference returns tokenized address to use for email reply headers -func (issue *Issue) ReplyReference() string { - var path string - if issue.IsPull { - path = "pulls" - } else { - path = "issues" - } - - return fmt.Sprintf("%s/%s/%d@%s", issue.Repo.FullName(), path, issue.Index, setting.Domain) -} - func (issue *Issue) addLabel(e db.Engine, label *Label, doer *User) error { return newIssueLabel(e, issue, label, doer) } diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 721f6eb49b..bd8f059c5e 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -305,13 +305,10 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient msg := NewMessageFrom([]string{recipient.Email}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String()) msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) - // Set Message-ID on first message so replies know what to reference - if actName == "new" { - msg.SetHeader("Message-ID", "<"+ctx.Issue.ReplyReference()+">") - } else { - msg.SetHeader("In-Reply-To", "<"+ctx.Issue.ReplyReference()+">") - msg.SetHeader("References", "<"+ctx.Issue.ReplyReference()+">") - } + msg.SetHeader("Message-ID", "<"+createReference(ctx.Issue, ctx.Comment)+">") + reference := createReference(ctx.Issue, nil) + msg.SetHeader("In-Reply-To", "<"+reference+">") + msg.SetHeader("References", "<"+reference+">") for key, value := range generateAdditionalHeaders(ctx, actType, recipient) { msg.SetHeader(key, value) @@ -323,6 +320,22 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient return msgs, nil } +func createReference(issue *models.Issue, comment *models.Comment) string { + var path string + if issue.IsPull { + path = "pulls" + } else { + path = "issues" + } + + var extra string + if comment != nil { + extra = fmt.Sprintf("/comment/%d", comment.ID) + } + + return fmt.Sprintf("%s/%s/%d%s@%s", issue.Repo.FullName(), path, issue.Index, extra, setting.Domain) +} + func generateAdditionalHeaders(ctx *mailCommentContext, reason string, recipient *models.User) map[string]string { repo := ctx.Issue.Repo diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index cbac7912b6..cd730a13a4 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -72,14 +72,16 @@ func TestComposeIssueCommentMessage(t *testing.T) { gomailMsg := msgs[0].ToMessage() mailto := gomailMsg.GetHeader("To") subject := gomailMsg.GetHeader("Subject") - inreplyTo := gomailMsg.GetHeader("In-Reply-To") + messageID := gomailMsg.GetHeader("Message-ID") + inReplyTo := gomailMsg.GetHeader("In-Reply-To") references := gomailMsg.GetHeader("References") assert.Len(t, mailto, 1, "exactly one recipient is expected in the To field") assert.Equal(t, "Re: ", subject[0][:4], "Comment reply subject should contain Re:") assert.Equal(t, "Re: [user2/repo1] @user2 #1 - issue1", subject[0]) - assert.Equal(t, inreplyTo[0], "", "In-Reply-To header doesn't match") - assert.Equal(t, references[0], "", "References header doesn't match") + assert.Equal(t, "", inReplyTo[0], "In-Reply-To header doesn't match") + assert.Equal(t, "", references[0], "References header doesn't match") + assert.Equal(t, "", messageID[0], "Message-ID header doesn't match") } func TestComposeIssueMessage(t *testing.T) { @@ -99,12 +101,14 @@ func TestComposeIssueMessage(t *testing.T) { mailto := gomailMsg.GetHeader("To") subject := gomailMsg.GetHeader("Subject") messageID := gomailMsg.GetHeader("Message-ID") + inReplyTo := gomailMsg.GetHeader("In-Reply-To") + references := gomailMsg.GetHeader("References") assert.Len(t, mailto, 1, "exactly one recipient is expected in the To field") assert.Equal(t, "[user2/repo1] @user2 #1 - issue1", subject[0]) - assert.Nil(t, gomailMsg.GetHeader("In-Reply-To")) - assert.Nil(t, gomailMsg.GetHeader("References")) - assert.Equal(t, messageID[0], "", "Message-ID header doesn't match") + assert.Equal(t, "", inReplyTo[0], "In-Reply-To header doesn't match") + assert.Equal(t, "", references[0], "References header doesn't match") + assert.Equal(t, "", messageID[0], "Message-ID header doesn't match") } func TestTemplateSelection(t *testing.T) {