Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Always set a unique Message-ID header #17206

Merged
merged 1 commit into from
Oct 1, 2021
Merged

Always set a unique Message-ID header #17206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions models/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
27 changes: 20 additions & 7 deletions services/mailer/mail.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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

Expand Down
16 changes: 10 additions & 6 deletions services/mailer/mail_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down