From 02ca6428b5920c2662436f397a7269f3d93ecc84 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Fri, 17 Mar 2023 13:11:07 +0100 Subject: [PATCH] fix(GODT-2407): Replace invalid email addresses with emtpy for new Drafts --- internal/user/imap.go | 2 +- internal/user/smtp.go | 2 +- pkg/message/parser.go | 51 ++++++++++++++++++++++++++++++-------- pkg/message/parser_test.go | 29 +++++++++++++++++++++- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/internal/user/imap.go b/internal/user/imap.go index fb5cdaa3..8a62ba3a 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -626,7 +626,7 @@ func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addr return proton.Message{}, fmt.Errorf("failed to create parser: %w", err) } - message, err := message.ParseWithParser(parser) + message, err := message.ParseWithParser(parser, true) if err != nil { return proton.Message{}, fmt.Errorf("failed to parse message: %w", err) } diff --git a/internal/user/smtp.go b/internal/user/smtp.go index 1a0dd706..27eee7e9 100644 --- a/internal/user/smtp.go +++ b/internal/user/smtp.go @@ -137,7 +137,7 @@ func (user *User) sendMail(authID string, from string, to []string, r io.Reader) } // Parse the message we want to send (after we have attached the public key). - message, err := message.ParseWithParser(parser) + message, err := message.ParseWithParser(parser, false) if err != nil { return fmt.Errorf("failed to parse message: %w", err) } diff --git a/pkg/message/parser.go b/pkg/message/parser.go index cba83aee..ff6472c2 100644 --- a/pkg/message/parser.go +++ b/pkg/message/parser.go @@ -73,6 +73,15 @@ type Attachment struct { // Parse parses an RFC822 message. func Parse(r io.Reader) (m Message, err error) { + return parseIOReaderImpl(r, false) +} + +// ParseAndAllowInvalidAddressLists parses an RFC822 message and allows email address lists to be invalid. +func ParseAndAllowInvalidAddressLists(r io.Reader) (m Message, err error) { + return parseIOReaderImpl(r, true) +} + +func parseIOReaderImpl(r io.Reader, allowInvalidAddressLists bool) (m Message, err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic while parsing message: %v", r) @@ -84,21 +93,21 @@ func Parse(r io.Reader) (m Message, err error) { return Message{}, errors.Wrap(err, "failed to create new parser") } - return parse(p) + return parse(p, allowInvalidAddressLists) } // ParseWithParser parses an RFC822 message using an existing parser. -func ParseWithParser(p *parser.Parser) (m Message, err error) { +func ParseWithParser(p *parser.Parser, allowInvalidAddressLists bool) (m Message, err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic while parsing message: %v", r) } }() - return parse(p) + return parse(p, allowInvalidAddressLists) } -func parse(p *parser.Parser) (Message, error) { +func parse(p *parser.Parser, allowInvalidAddressLists bool) (Message, error) { if err := convertEncodedTransferEncoding(p); err != nil { return Message{}, errors.Wrap(err, "failed to convert encoded transfer encoding") } @@ -107,7 +116,7 @@ func parse(p *parser.Parser) (Message, error) { return Message{}, errors.Wrap(err, "failed to convert foreign encodings") } - m, err := parseMessageHeader(p.Root().Header) + m, err := parseMessageHeader(p.Root().Header, allowInvalidAddressLists) if err != nil { return Message{}, errors.Wrap(err, "failed to parse message header") } @@ -433,7 +442,7 @@ func getPlainBody(part *parser.Part) []byte { } } -func parseMessageHeader(h message.Header) (Message, error) { +func parseMessageHeader(h message.Header, allowInvalidAddressLists bool) (Message, error) { var m Message for fields := h.Fields(); fields.Next(); { @@ -451,7 +460,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "from": sender, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse from") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse from") + } + + logrus.WithError(err).Warn("failed to parse from") } if len(sender) > 0 { @@ -461,7 +474,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "to": toList, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse to") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse to") + } + + logrus.WithError(err).Warn("failed to parse to") } m.ToList = toList @@ -469,7 +486,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "reply-to": replyTos, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse reply-to") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse reply-to") + } + + logrus.WithError(err).Warn("failed to parse reply-to") } m.ReplyTos = replyTos @@ -477,7 +498,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "cc": ccList, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse cc") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse cc") + } + + logrus.WithError(err).Warn("failed to parse cc") } m.CCList = ccList @@ -485,7 +510,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "bcc": bccList, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse bcc") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse bcc") + } + + logrus.WithError(err).Warn("failed to parse bcc") } m.BCCList = bccList diff --git a/pkg/message/parser_test.go b/pkg/message/parser_test.go index 6942deee..d536333b 100644 --- a/pkg/message/parser_test.go +++ b/pkg/message/parser_test.go @@ -23,6 +23,7 @@ import ( "io" "os" "path/filepath" + "strings" "testing" "github.com/ProtonMail/go-proton-api" @@ -444,7 +445,7 @@ func TestParseWithAttachedPublicKey(t *testing.T) { p, err := parser.New(f) require.NoError(t, err) - m, err := ParseWithParser(p) + m, err := ParseWithParser(p, false) require.NoError(t, err) p.AttachPublicKey("publickey", "publickeyname") @@ -636,6 +637,32 @@ func TestParseIcsAttachment(t *testing.T) { assert.Equal(t, string(m.Attachments[0].Data), "This is an ics calendar invite") } +func TestParseAllowInvalidAddress(t *testing.T) { + const literal = `To: foo + From: bar + BCC: fff + CC: FFF + Reply-To: AAA + Subject: Test +` + + // This will fail as the addresses are not valid. + { + _, err := Parse(strings.NewReader(literal)) + require.Error(t, err) + } + + // This will work as invalid addresses will be ignored. + m, err := ParseAndAllowInvalidAddressLists(strings.NewReader(literal)) + require.NoError(t, err) + + assert.Empty(t, m.ToList) + assert.Empty(t, m.Sender) + assert.Empty(t, m.CCList) + assert.Empty(t, m.BCCList) + assert.Empty(t, m.ReplyTos) +} + func TestParsePanic(t *testing.T) { var err error