Skip to content

Commit

Permalink
Add email headers (#15939)
Browse files Browse the repository at this point in the history 
* Added additional email headers.

* Added tests.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
@KN4CK3R @lunny
KN4CK3R and lunny authored May 22, 2021
1 parent be745be commit 77fa714
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 61 deletions.
52 changes: 46 additions & 6 deletions services/mailer/mail.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"html/template"
"mime"
"regexp"
"strconv"
"strings"
texttmpl "text/template"

Expand Down Expand Up @@ -174,7 +175,7 @@ func SendCollaboratorMail(u, doer *models.User, repo *models.Repository) {
SendAsync(msg)
}

func composeIssueCommentMessages(ctx *mailCommentContext, lang string, tos []string, fromMention bool, info string) ([]*Message, error) {
func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipients []*models.User, fromMention bool, info string) ([]*Message, error) {
var (
subject string
link string
Expand Down Expand Up @@ -265,9 +266,9 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, tos []str
}

// Make sure to compose independent messages to avoid leaking user emails
msgs := make([]*Message, 0, len(tos))
for _, to := range tos {
msg := NewMessageFrom([]string{to}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String())
msgs := make([]*Message, 0, len(recipients))
for _, recipient := range recipients {
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
Expand All @@ -277,12 +278,51 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, tos []str
msg.SetHeader("In-Reply-To", "<"+ctx.Issue.ReplyReference()+">")
msg.SetHeader("References", "<"+ctx.Issue.ReplyReference()+">")
}

for key, value := range generateAdditionalHeaders(ctx, actType, recipient) {
msg.SetHeader(key, value)
}

msgs = append(msgs, msg)
}

return msgs, nil
}

func generateAdditionalHeaders(ctx *mailCommentContext, reason string, recipient *models.User) map[string]string {
repo := ctx.Issue.Repo

return map[string]string{
// https://datatracker.ietf.org/doc/html/rfc2919
"List-ID": fmt.Sprintf("%s <%s.%s.%s>", repo.FullName(), repo.Name, repo.OwnerName, setting.Domain),

// https://datatracker.ietf.org/doc/html/rfc2369
"List-Archive": fmt.Sprintf("<%s>", repo.HTMLURL()),
//"List-Post": https://github.com/go-gitea/gitea/pull/13585
//"List-Unsubscribe": https://github.com/go-gitea/gitea/issues/10808, https://github.com/go-gitea/gitea/issues/13283

"X-Gitea-Reason": reason,
"X-Gitea-Sender": ctx.Doer.DisplayName(),
"X-Gitea-Recipient": recipient.DisplayName(),
"X-Gitea-Recipient-Address": recipient.Email,
"X-Gitea-Repository": repo.Name,
"X-Gitea-Repository-Path": repo.FullName(),
"X-Gitea-Repository-Link": repo.HTMLURL(),
"X-Gitea-Issue-ID": strconv.FormatInt(ctx.Issue.Index, 10),
"X-Gitea-Issue-Link": ctx.Issue.HTMLURL(),

"X-GitHub-Reason": reason,
"X-GitHub-Sender": ctx.Doer.DisplayName(),
"X-GitHub-Recipient": recipient.DisplayName(),
"X-GitHub-Recipient-Address": recipient.Email,

"X-GitLab-NotificationReason": reason,
"X-GitLab-Project": repo.Name,
"X-GitLab-Project-Path": repo.FullName(),
"X-GitLab-Issue-IID": strconv.FormatInt(ctx.Issue.Index, 10),
}
}

func sanitizeSubject(subject string) string {
runes := []rune(strings.TrimSpace(subjectRemoveSpaces.ReplaceAllLiteralString(subject, " ")))
if len(runes) > mailMaxSubjectRunes {
Expand All @@ -294,9 +334,9 @@ func sanitizeSubject(subject string) string {

// SendIssueAssignedMail composes and sends issue assigned email
func SendIssueAssignedMail(issue *models.Issue, doer *models.User, content string, comment *models.Comment, recipients []*models.User) error {
langMap := make(map[string][]string)
langMap := make(map[string][]*models.User)
for _, user := range recipients {
langMap[user.Language] = append(langMap[user.Language], user.Email)
langMap[user.Language] = append(langMap[user.Language], user)
}

for lang, tos := range langMap {
Expand Down
4 changes: 2 additions & 2 deletions services/mailer/mail_issue.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*models.User, visite
checkUnit = models.UnitTypePullRequests
}

langMap := make(map[string][]string)
langMap := make(map[string][]*models.User)
for _, user := range users {
// At this point we exclude:
// user that don't have all mails enabled or users only get mail on mention and this is one ...
Expand All @@ -138,7 +138,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*models.User, visite
continue
}

langMap[user.Language] = append(langMap[user.Language], user.Email)
langMap[user.Language] = append(langMap[user.Language], user)
}

for lang, receivers := range langMap {
Expand Down
109 changes: 56 additions & 53 deletions services/mailer/mail_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const bodyTpl = `
</html>
`

func TestComposeIssueCommentMessage(t *testing.T) {
func prepareMailerTest(t *testing.T) (doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) {
assert.NoError(t, models.PrepareTestDatabase())
var mailService = setting.Mailer{
From: "test@gitea.com",
Expand All @@ -48,18 +48,24 @@ func TestComposeIssueCommentMessage(t *testing.T) {
setting.MailService = &mailService
setting.Domain = "localhost"

doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository)
issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue)
comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment)
doer = models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
repo = models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository)
issue = models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue)
assert.NoError(t, issue.LoadRepo())
comment = models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment)
return
}

func TestComposeIssueCommentMessage(t *testing.T) {
doer, _, issue, comment := prepareMailerTest(t)

stpl := texttmpl.Must(texttmpl.New("issue/comment").Parse(subjectTpl))
btpl := template.Must(template.New("issue/comment").Parse(bodyTpl))
InitMailRender(stpl, btpl)

tos := []string{"test@gitea.com", "test2@gitea.com"}
recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}, {Name: "Test2", Email: "test2@gitea.com"}}
msgs, err := composeIssueCommentMessages(&mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCommentIssue,
Content: "test body", Comment: comment}, "en-US", tos, false, "issue comment")
Content: "test body", Comment: comment}, "en-US", recipients, false, "issue comment")
assert.NoError(t, err)
assert.Len(t, msgs, 2)
gomailMsg := msgs[0].ToMessage()
Expand All @@ -76,25 +82,15 @@ func TestComposeIssueCommentMessage(t *testing.T) {
}

func TestComposeIssueMessage(t *testing.T) {
assert.NoError(t, models.PrepareTestDatabase())
var mailService = setting.Mailer{
From: "test@gitea.com",
}

setting.MailService = &mailService
setting.Domain = "localhost"

doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository)
issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue)
doer, _, issue, _ := prepareMailerTest(t)

stpl := texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl))
btpl := template.Must(template.New("issue/new").Parse(bodyTpl))
InitMailRender(stpl, btpl)

tos := []string{"test@gitea.com", "test2@gitea.com"}
recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}, {Name: "Test2", Email: "test2@gitea.com"}}
msgs, err := composeIssueCommentMessages(&mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCreateIssue,
Content: "test body"}, "en-US", tos, false, "issue create")
Content: "test body"}, "en-US", recipients, false, "issue create")
assert.NoError(t, err)
assert.Len(t, msgs, 2)

Expand All @@ -111,18 +107,8 @@ func TestComposeIssueMessage(t *testing.T) {
}

func TestTemplateSelection(t *testing.T) {
assert.NoError(t, models.PrepareTestDatabase())
var mailService = setting.Mailer{
From: "test@gitea.com",
}

setting.MailService = &mailService
setting.Domain = "localhost"

doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository)
issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue)
tos := []string{"test@gitea.com"}
doer, repo, issue, comment := prepareMailerTest(t)
recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}}

stpl := texttmpl.Must(texttmpl.New("issue/default").Parse("issue/default/subject"))
texttmpl.Must(stpl.New("issue/new").Parse("issue/new/subject"))
Expand All @@ -146,38 +132,26 @@ func TestTemplateSelection(t *testing.T) {
}

msg := testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCreateIssue,
Content: "test body"}, tos, false, "TestTemplateSelection")
Content: "test body"}, recipients, false, "TestTemplateSelection")
expect(t, msg, "issue/new/subject", "issue/new/body")

comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment)
msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCommentIssue,
Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection")
Content: "test body", Comment: comment}, recipients, false, "TestTemplateSelection")
expect(t, msg, "issue/default/subject", "issue/default/body")

pull := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 2, Repo: repo, Poster: doer}).(*models.Issue)
comment = models.AssertExistsAndLoadBean(t, &models.Comment{ID: 4, Issue: pull}).(*models.Comment)
msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: pull, Doer: doer, ActionType: models.ActionCommentPull,
Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection")
Content: "test body", Comment: comment}, recipients, false, "TestTemplateSelection")
expect(t, msg, "pull/comment/subject", "pull/comment/body")

msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCloseIssue,
Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection")
Content: "test body", Comment: comment}, recipients, false, "TestTemplateSelection")
expect(t, msg, "Re: [user2/repo1] issue1 (#1)", "issue/close/body")
}

func TestTemplateServices(t *testing.T) {
assert.NoError(t, models.PrepareTestDatabase())
var mailService = setting.Mailer{
From: "test@gitea.com",
}

setting.MailService = &mailService
setting.Domain = "localhost"

doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository)
issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue)
comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment)
doer, _, issue, comment := prepareMailerTest(t)
assert.NoError(t, issue.LoadRepo())

expect := func(t *testing.T, issue *models.Issue, comment *models.Comment, doer *models.User,
Expand All @@ -187,9 +161,9 @@ func TestTemplateServices(t *testing.T) {
btpl := template.Must(template.New("issue/default").Parse(tplBody))
InitMailRender(stpl, btpl)

tos := []string{"test@gitea.com"}
recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}}
msg := testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: actionType,
Content: "test body", Comment: comment}, tos, fromMention, "TestTemplateServices")
Content: "test body", Comment: comment}, recipients, fromMention, "TestTemplateServices")

subject := msg.ToMessage().GetHeader("Subject")
msgbuf := new(bytes.Buffer)
Expand Down Expand Up @@ -219,9 +193,38 @@ func TestTemplateServices(t *testing.T) {
"//Re: //")
}

func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, tos []string, fromMention bool, info string) *Message {
msgs, err := composeIssueCommentMessages(ctx, "en-US", tos, fromMention, info)
func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, recipients []*models.User, fromMention bool, info string) *Message {
msgs, err := composeIssueCommentMessages(ctx, "en-US", recipients, fromMention, info)
assert.NoError(t, err)
assert.Len(t, msgs, 1)
return msgs[0]
}

func TestGenerateAdditionalHeaders(t *testing.T) {
doer, _, issue, _ := prepareMailerTest(t)

ctx := &mailCommentContext{Issue: issue, Doer: doer}
recipient := &models.User{Name: "Test", Email: "test@gitea.com"}

headers := generateAdditionalHeaders(ctx, "dummy-reason", recipient)

expected := map[string]string{
"List-ID": "user2/repo1 <repo1.user2.localhost>",
"List-Archive": "<https://try.gitea.io/user2/repo1>",
"X-Gitea-Reason": "dummy-reason",
"X-Gitea-Sender": "< U<se>r Tw<o > ><",
"X-Gitea-Recipient": "Test",
"X-Gitea-Recipient-Address": "test@gitea.com",
"X-Gitea-Repository": "repo1",
"X-Gitea-Repository-Path": "user2/repo1",
"X-Gitea-Repository-Link": "https://try.gitea.io/user2/repo1",
"X-Gitea-Issue-ID": "1",
"X-Gitea-Issue-Link": "https://try.gitea.io/user2/repo1/issues/1",
}

for key, value := range expected {
if assert.Contains(t, headers, key) {
assert.Equal(t, value, headers[key])
}
}
}

0 comments on commit 77fa714

Please sign in to comment.