From 93b14bc454fd7f350b990567280118cee84a6837 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Wed, 3 Jul 2024 11:25:31 +0200 Subject: [PATCH] fixed the notification service error when the user's display name contained special characters --- .../unreleased/fix-email-notification.md | 7 ++ .../notifications/pkg/channels/channels.go | 37 +++++++---- .../pkg/channels/channels_test.go | 64 +++++++++++++++++++ .../notifications/pkg/config/parser/parse.go | 10 +++ 4 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 changelog/unreleased/fix-email-notification.md create mode 100644 services/notifications/pkg/channels/channels_test.go diff --git a/changelog/unreleased/fix-email-notification.md b/changelog/unreleased/fix-email-notification.md new file mode 100644 index 00000000000..af07ca23ab7 --- /dev/null +++ b/changelog/unreleased/fix-email-notification.md @@ -0,0 +1,7 @@ +Bugfix: Fix the email notification service + +We fixed an error in the notification service that caused the email notification to fail when the user's display name contained special characters. + + +https://github.com/owncloud/ocis/pull/9514 +https://github.com/owncloud/ocis/issues/9402 diff --git a/services/notifications/pkg/channels/channels.go b/services/notifications/pkg/channels/channels.go index d8d368a63ed..fec426d923a 100644 --- a/services/notifications/pkg/channels/channels.go +++ b/services/notifications/pkg/channels/channels.go @@ -4,7 +4,7 @@ package channels import ( "context" "crypto/tls" - "fmt" + stdmail "net/mail" "strings" "github.com/owncloud/ocis/v2/ocis-pkg/log" @@ -31,16 +31,24 @@ type Message struct { // NewMailChannel instantiates a new mail communication channel. func NewMailChannel(cfg config.Config, logger log.Logger) (Channel, error) { + var err error + var a *stdmail.Address + a, err = stdmail.ParseAddress(cfg.Notifications.SMTP.Sender) + if err != nil { + logger.Err(err).Msg("parsing error, the 'smtp_sender' must be a valid single RFC 5322 address.") + } return Mail{ - conf: cfg, - logger: logger, + conf: cfg, + smtpAddress: a, + logger: logger, }, nil } // Mail is the communication channel for email. type Mail struct { - conf config.Config - logger log.Logger + conf config.Config + smtpAddress *stdmail.Address + logger log.Logger } func (m Mail) getMailClient() (*mail.SMTPClient, error) { @@ -102,8 +110,12 @@ func (m Mail) getMailClient() (*mail.SMTPClient, error) { // SendMessage sends a message to all given users. func (m Mail) SendMessage(ctx context.Context, message *Message) error { + if m.smtpAddress == nil { + m.logger.Error().Str("mail", "SendMessage").Msg("failed to send a message. SMTP address is not set") + return nil + } if m.conf.Notifications.SMTP.Host == "" { - m.logger.Info().Str("mail", "SendMessage").Msg("failed to send a message. SMTP host is not set") + m.logger.Info().Str("mail", "SendMessage").Msg("failed to send a message. SMTP host is not set") return nil } @@ -113,11 +125,7 @@ func (m Mail) SendMessage(ctx context.Context, message *Message) error { } email := mail.NewMSG() - if message.Sender != "" { - email.SetFrom(fmt.Sprintf("%s via %s", message.Sender, m.conf.Notifications.SMTP.Sender)).AddTo(message.Recipient...) - } else { - email.SetFrom(m.conf.Notifications.SMTP.Sender).AddTo(message.Recipient...) - } + email.SetFrom(appendSender(message.Sender, *m.smtpAddress)).AddTo(message.Recipient...) email.SetSubject(message.Subject) email.SetBody(mail.TextPlain, message.TextBody) if message.HTMLBody != "" { @@ -129,3 +137,10 @@ func (m Mail) SendMessage(ctx context.Context, message *Message) error { return email.Send(smtpClient) } + +func appendSender(sender string, a stdmail.Address) string { + if strings.TrimSpace(sender) != "" { + a.Name = strings.TrimSpace(sender + " via " + a.Name) + } + return a.String() +} diff --git a/services/notifications/pkg/channels/channels_test.go b/services/notifications/pkg/channels/channels_test.go new file mode 100644 index 00000000000..5140859d980 --- /dev/null +++ b/services/notifications/pkg/channels/channels_test.go @@ -0,0 +1,64 @@ +package channels + +import ( + "net/mail" + "testing" +) + +func Test_appendSender(t *testing.T) { + type args struct { + sender string + a mail.Address + } + + a1, err := mail.ParseAddress("ownCloud ") + if err != nil { + t.Error(err) + } + a2, err := mail.ParseAddress("noreply@example.com") + if err != nil { + t.Error(err) + } + + tests := []struct { + name string + sender string + want1 string + want2 string + }{ + { + name: "empty sender", + sender: "", + want1: `"ownCloud" `, + want2: ``, + }, + { + name: "not empty sender", + sender: `Joe Q. Public`, + want1: `"Joe Q. Public via ownCloud" `, + want2: `"Joe Q. Public via" `, + }, + { + name: "sender whit comma and semicolon", + sender: `Joe, Q; Public:`, + want1: `"Joe, Q; Public: via ownCloud" `, + want2: `"Joe, Q; Public: via" `, + }, + { + name: "sender with quotes", + sender: `Joe Q. "Public"`, + want1: `"Joe Q. \"Public\" via ownCloud" `, + want2: `"Joe Q. \"Public\" via" `, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := appendSender(tt.sender, *a1); got != tt.want1 { + t.Errorf("appendSender() = %v, want %v", got, tt.want1) + } + if got := appendSender(tt.sender, *a2); got != tt.want2 { + t.Errorf("appendSender() = %v, want %v", got, tt.want2) + } + }) + } +} diff --git a/services/notifications/pkg/config/parser/parse.go b/services/notifications/pkg/config/parser/parse.go index 127a26cc688..82100ffa422 100644 --- a/services/notifications/pkg/config/parser/parse.go +++ b/services/notifications/pkg/config/parser/parse.go @@ -3,6 +3,8 @@ package parser import ( "errors" "fmt" + "net/mail" + "strings" ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config" "github.com/owncloud/ocis/v2/ocis-pkg/shared" @@ -54,6 +56,14 @@ func Validate(cfg *config.Config) error { } } + if strings.TrimSpace(cfg.Notifications.SMTP.Sender) != "" { + if s, err := mail.ParseAddress(cfg.Notifications.SMTP.Sender); err == nil { + cfg.Notifications.SMTP.Sender = s.String() + } else { + return fmt.Errorf("the 'smtp_sender' must be a valid single RFC 5322 address. parsing error %w", err) + } + } + if cfg.ServiceAccount.ServiceAccountID == "" { return shared.MissingServiceAccountID(cfg.Service.Name) }