Always set a unique Message-ID header. (#17206)

2 files changed, 30 insertions, 13 deletions

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], "<user2/repo1/issues/1@localhost>", "In-Reply-To header doesn't match")
-	assert.Equal(t, references[0], "<user2/repo1/issues/1@localhost>", "References header doesn't match")
+	assert.Equal(t, "<user2/repo1/issues/1@localhost>", inReplyTo[0], "In-Reply-To header doesn't match")
+	assert.Equal(t, "<user2/repo1/issues/1@localhost>", references[0], "References header doesn't match")
+	assert.Equal(t, "<user2/repo1/issues/1/comment/2@localhost>", 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], "<user2/repo1/issues/1@localhost>", "Message-ID header doesn't match")
+	assert.Equal(t, "<user2/repo1/issues/1@localhost>", inReplyTo[0], "In-Reply-To header doesn't match")
+	assert.Equal(t, "<user2/repo1/issues/1@localhost>", references[0], "References header doesn't match")
+	assert.Equal(t, "<user2/repo1/issues/1@localhost>", messageID[0], "Message-ID header doesn't match")
 }
 
 func TestTemplateSelection(t *testing.T) {