From 8a1924b51abdfd0ca0d990393e761ae28fc7dabd Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 17 Jul 2024 05:13:59 +0000 Subject: [PATCH] [PORT] Use FullName in Emails to address the recipient if possible (gitea#31527) (#4516) Before we had just the plain mail address as recipient. But now we provide additional Information for the Mail clients. --- Porting information: - Two behavior changes are noted with this patch, the display name is now always quoted although in some scenarios unnecessary it's a safety precaution of Go. B encoding is used when certain characters are present as they aren't 'legal' to be used as a display name and Q encoding would still show them and B encoding needs to be used, this is now done by Go's `address.String()`. - Update and add new unit tests. Co-authored-by: 6543 <6543@obermui.de> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4516 Reviewed-by: Earl Warren Co-authored-by: Gusted Co-committed-by: Gusted --- models/user/user.go | 28 ++++++++++++++++++++++++++++ models/user/user_test.go | 26 ++++++++++++++++++++++++++ services/mailer/mail.go | 6 +++--- services/mailer/mail_release.go | 8 ++++---- services/mailer/mail_repo.go | 12 ++++++------ 5 files changed, 67 insertions(+), 13 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 6d8c5fa2b5..16b0f16243 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -9,6 +9,7 @@ import ( "context" "encoding/hex" "fmt" + "net/mail" "net/url" "path/filepath" "regexp" @@ -439,6 +440,33 @@ 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 + } + + address, err := mail.ParseAddress(fmt.Sprintf("%s <%s>", sanitizedDisplayName, u.Email)) + if err != nil { + return u.Email + } + + return address.String() +} + // 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 7457256017..bf3fc801ce 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -601,6 +601,32 @@ 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" `}, + {"name@example.com", "name@example.com", "name@example.com"}, + {"Hi Its ", "ee@mail.box", `"Hi Its Mee" `}, + {"Sinéad.O'Connor", "sinead.oconnor@gmail.com", "=?utf-8?b?U2luw6lhZC5PJ0Nvbm5vcg==?= "}, + {"Æsir", "aesir@gmx.de", "=?utf-8?q?=C3=86sir?= "}, + {"new😀user", "new.user@alo.com", "=?utf-8?q?new=F0=9F=98=80user?= "}, + {`"quoted"`, "quoted@test.com", `"quoted" `}, + {`gusted`, "gusted@test.com", `"gusted" `}, + {`Joe Q. Public`, "john.q.public@example.com", `"Joe Q. Public" `}, + {`Who?`, "one@y.test", `"Who?" `}, + } + + 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 5eb8efe128..ca8fb34ef1 100644 --- 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", setting.AppName), content.String()) + msg := NewMessage(u.EmailTo(), locale.TrString("mail.register_notify", setting.AppName), 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 2b0e7cfdc0..445e58724c 100644 --- 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 @@ -88,7 +88,7 @@ func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_mo publisherName := rel.Publisher.DisplayName() msgID := createMessageIDForRelease(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 e0d55bb120..28b9cef8a7 100644 --- 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)