Use FullName in Emails to address the recipient if possible (#31527)

Before we had just the plain mail address as recipient. But now we
provide additional Information for the Mail clients.

---
*Sponsored by Kithara Software GmbH*

diff --git a/models/user/user.go b/models/user/user.go
index 23637f461628837e085eb74fc7143444ab4b4dcc..5ac24f09778c3f2b179434c9514bec5762e12f37 100644 (file)
--- a/models/user/user.go
+++ b/models/user/user.go
@@ -8,6 +8,8 @@ import (
        "context"
        "encoding/hex"
        "fmt"
+       "mime"
+       "net/mail"
        "net/url"
        "path/filepath"
        "regexp"
@@ -413,6 +415,34 @@ func (u *User) DisplayName() string {
        return u.Name
 }
 
+var emailToReplacer = strings.NewReplacer(
+       "\n", "",
+       "\r", "",
+       "<", "",
+       ">", "",
+       ",", "",
+       ":", "",
+       ";", "",
+)
+
+// EmailTo returns a string suitable to be put into a e-mail `To:` header.
+func (u *User) EmailTo() string {
+       sanitizedDisplayName := emailToReplacer.Replace(u.DisplayName())
+
+       // should be an edge case but nice to have
+       if sanitizedDisplayName == u.Email {
+               return u.Email
+       }
+
+       to := fmt.Sprintf("%s <%s>", sanitizedDisplayName, u.Email)
+       add, err := mail.ParseAddress(to)
+       if err != nil {
+               return u.Email
+       }
+
+       return fmt.Sprintf("%s <%s>", mime.QEncoding.Encode("utf-8", add.Name), add.Address)
+}
+
 // GetDisplayName returns full name if it's not empty and DEFAULT_SHOW_FULL_NAME is set,
 // returns username otherwise.
 func (u *User) GetDisplayName() string {

diff --git a/models/user/user_test.go b/models/user/user_test.go
index c4e278caab756d97751bcf783b7bd23f7df325eb..26cda3f89e3247c4dcd027f720b22f31018da2f2 100644 (file)
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -529,6 +529,29 @@ func Test_NormalizeUserFromEmail(t *testing.T) {
        }
 }
 
+func TestEmailTo(t *testing.T) {
+       testCases := []struct {
+               fullName string
+               mail     string
+               result   string
+       }{
+               {"Awareness Hub", "awareness@hub.net", "Awareness Hub <awareness@hub.net>"},
+               {"name@example.com", "name@example.com", "name@example.com"},
+               {"Hi Its <Mee>", "ee@mail.box", "Hi Its Mee <ee@mail.box>"},
+               {"Sinéad.O'Connor", "sinead.oconnor@gmail.com", "=?utf-8?q?Sin=C3=A9ad.O'Connor?= <sinead.oconnor@gmail.com>"},
+               {"Æsir", "aesir@gmx.de", "=?utf-8?q?=C3=86sir?= <aesir@gmx.de>"},
+               {"new😀user", "new.user@alo.com", "=?utf-8?q?new=F0=9F=98=80user?= <new.user@alo.com>"},
+               {`"quoted"`, "quoted@test.com", "quoted <quoted@test.com>"},
+       }
+
+       for _, testCase := range testCases {
+               t.Run(testCase.result, func(t *testing.T) {
+                       testUser := &user_model.User{FullName: testCase.fullName, Email: testCase.mail}
+                       assert.EqualValues(t, testCase.result, testUser.EmailTo())
+               })
+       }
+}
+
 func TestDisabledUserFeatures(t *testing.T) {
        assert.NoError(t, unittest.PrepareTestDatabase())
 

diff --git a/services/mailer/mail.go b/services/mailer/mail.go
index 000cc835c8b77b6d9b03d8ac165e8693d5385c90..11810a036d62d904608079cec046901cdcf317b9 100644 (file)
--- a/services/mailer/mail.go
+++ b/services/mailer/mail.go
@@ -82,7 +82,7 @@ func sendUserMail(language string, u *user_model.User, tpl base.TplName, code, s
                return
        }
 
-       msg := NewMessage(u.Email, subject, content.String())
+       msg := NewMessage(u.EmailTo(), subject, content.String())
        msg.Info = fmt.Sprintf("UID: %d, %s", u.ID, info)
 
        SendAsync(msg)
@@ -158,7 +158,7 @@ func SendRegisterNotifyMail(u *user_model.User) {
                return
        }
 
-       msg := NewMessage(u.Email, locale.TrString("mail.register_notify"), content.String())
+       msg := NewMessage(u.EmailTo(), locale.TrString("mail.register_notify"), content.String())
        msg.Info = fmt.Sprintf("UID: %d, registration notify", u.ID)
 
        SendAsync(msg)
@@ -189,7 +189,7 @@ func SendCollaboratorMail(u, doer *user_model.User, repo *repo_model.Repository)
                return
        }
 
-       msg := NewMessage(u.Email, subject, content.String())
+       msg := NewMessage(u.EmailTo(), subject, content.String())
        msg.Info = fmt.Sprintf("UID: %d, add collaborator", u.ID)
 
        SendAsync(msg)

diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go
index b7a4da0db92b31baf3a5ccb67d825cc10234256f..f1f2e558a776bd217b5ebaf8168eb4037065c45f 100644 (file)
--- a/services/mailer/mail_release.go
+++ b/services/mailer/mail_release.go
@@ -40,10 +40,10 @@ func MailNewRelease(ctx context.Context, rel *repo_model.Release) {
                return
        }
 
-       langMap := make(map[string][]string)
+       langMap := make(map[string][]*user_model.User)
        for _, user := range recipients {
                if user.ID != rel.PublisherID {
-                       langMap[user.Language] = append(langMap[user.Language], user.Email)
+                       langMap[user.Language] = append(langMap[user.Language], user)
                }
        }
 
@@ -52,7 +52,7 @@ func MailNewRelease(ctx context.Context, rel *repo_model.Release) {
        }
 }
 
-func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_model.Release) {
+func mailNewRelease(ctx context.Context, lang string, tos []*user_model.User, rel *repo_model.Release) {
        locale := translation.NewLocale(lang)
 
        var err error
@@ -89,7 +89,7 @@ func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_mo
        publisherName := rel.Publisher.DisplayName()
        msgID := generateMessageIDForRelease(rel)
        for _, to := range tos {
-               msg := NewMessageFrom(to, publisherName, setting.MailService.FromEmail, subject, mailBody.String())
+               msg := NewMessageFrom(to.EmailTo(), publisherName, setting.MailService.FromEmail, subject, mailBody.String())
                msg.Info = subject
                msg.SetHeader("Message-ID", msgID)
                msgs = append(msgs, msg)

diff --git a/services/mailer/mail_repo.go b/services/mailer/mail_repo.go
index e0d55bb12004305156975fb2fd8d0889e0aa1bc6..28b9cef8a75e197e903296040193b108db1dd635 100644 (file)
--- a/services/mailer/mail_repo.go
+++ b/services/mailer/mail_repo.go
@@ -28,13 +28,13 @@ func SendRepoTransferNotifyMail(ctx context.Context, doer, newOwner *user_model.
                        return err
                }
 
-               langMap := make(map[string][]string)
+               langMap := make(map[string][]*user_model.User)
                for _, user := range users {
                        if !user.IsActive {
                                // don't send emails to inactive users
                                continue
                        }
-                       langMap[user.Language] = append(langMap[user.Language], user.Email)
+                       langMap[user.Language] = append(langMap[user.Language], user)
                }
 
                for lang, tos := range langMap {
@@ -46,11 +46,11 @@ func SendRepoTransferNotifyMail(ctx context.Context, doer, newOwner *user_model.
                return nil
        }
 
-       return sendRepoTransferNotifyMailPerLang(newOwner.Language, newOwner, doer, []string{newOwner.Email}, repo)
+       return sendRepoTransferNotifyMailPerLang(newOwner.Language, newOwner, doer, []*user_model.User{newOwner}, repo)
 }
 
 // sendRepoTransferNotifyMail triggers a notification e-mail when a pending repository transfer was created for each language
-func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.User, emails []string, repo *repo_model.Repository) error {
+func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.User, emailTos []*user_model.User, repo *repo_model.Repository) error {
        var (
                locale  = translation.NewLocale(lang)
                content bytes.Buffer
@@ -78,8 +78,8 @@ func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.U
                return err
        }
 
-       for _, to := range emails {
-               msg := NewMessage(to, subject, content.String())
+       for _, to := range emailTos {
+               msg := NewMessage(to.EmailTo(), subject, content.String())
                msg.Info = fmt.Sprintf("UID: %d, repository pending transfer notification", newOwner.ID)
 
                SendAsync(msg)