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.

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

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

Merged
merged 14 commits into from
Jul 8, 2024
31 changes: 31 additions & 0 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"context"
"encoding/hex"
"fmt"
"net/mail"
"net/url"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"
"unicode"
Expand Down Expand Up @@ -413,6 +415,35 @@ func (u *User) DisplayName() string {
return u.Name
}

// EmailTo returns full name and email.
6543 marked this conversation as resolved.
Show resolved Hide resolved
func (u *User) EmailTo() string {
// we don't deal with utf8 to ascii conversion
if u.DisplayName() != strings.Trim(strconv.QuoteToASCII(u.DisplayName()), `"`) {
return u.Email
}
6543 marked this conversation as resolved.
Show resolved Hide resolved
// ok just be sure we don't let the user break things somehow
sanitizedDisplayName := strings.ReplaceAll(u.DisplayName(), "\n", "")
6543 marked this conversation as resolved.
Show resolved Hide resolved
sanitizedDisplayName = strings.ReplaceAll(sanitizedDisplayName, "\r", "")
sanitizedDisplayName = strings.ReplaceAll(sanitizedDisplayName, "<", "")
sanitizedDisplayName = strings.ReplaceAll(sanitizedDisplayName, ">", "")
sanitizedDisplayName = strings.ReplaceAll(sanitizedDisplayName, ",", "")
sanitizedDisplayName = strings.ReplaceAll(sanitizedDisplayName, ":", "")
sanitizedDisplayName = strings.ReplaceAll(sanitizedDisplayName, ";", "")
6543 marked this conversation as resolved.
Show resolved Hide resolved

// 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>", 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 {
Expand Down
23 changes: 23 additions & 0 deletions models/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
6543 marked this conversation as resolved.
Show resolved Hide resolved
{"Sinéad.O'Connor", "sinead.oconnor@gmail.com", "sinead.oconnor@gmail.com"},
{"Æsir", "aesir@gmx.de", "aesir@gmx.de"},
{"new😀user", "new.user@alo.com", "new.user@alo.com"},
{`"quoted"`, "quoted@test.com", "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())

Expand Down
6 changes: 3 additions & 3 deletions services/mailer/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions services/mailer/mail_release.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions services/mailer/mail_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func SendRepoTransferNotifyMail(ctx context.Context, doer, newOwner *user_model.
// 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.EmailTo())
}

for lang, tos := range langMap {
Expand All @@ -46,7 +46,7 @@ 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, []string{newOwner.EmailTo()}, repo)
}

// sendRepoTransferNotifyMail triggers a notification e-mail when a pending repository transfer was created for each language
Expand Down
Loading