From 8b33d56b592c77643a532b8723d38cf6fbaa2b06 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Tue, 14 Mar 2023 10:34:03 +0100 Subject: [PATCH] fix(GODT-2479): Ensure messages always have a text body part Proton Backend requires that all messages have at least one text/plain or text/html body part, even if it is empty. --- internal/bridge/send_test.go | 179 +++++++++++++++++++++++++++++++++++ internal/bridge/sync_test.go | 7 +- internal/user/smtp.go | 4 + pkg/message/parser/parser.go | 34 +++++++ 4 files changed, 222 insertions(+), 2 deletions(-) diff --git a/internal/bridge/send_test.go b/internal/bridge/send_test.go index b9e6e0ca..c3b5e1ca 100644 --- a/internal/bridge/send_test.go +++ b/internal/bridge/send_test.go @@ -330,3 +330,182 @@ func TestBridge_SendInvite(t *testing.T) { }) }) } + +func TestBridge_SendAddTextBodyPartIfNotExists(t *testing.T) { + const messageMultipartWithoutText = `Content-Type: multipart/mixed; + boundary="Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84" +Subject: A new message +Date: Mon, 13 Mar 2023 16:06:16 +0100 + + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84 +Content-Disposition: inline; + filename=Cat_August_2010-4.jpeg +Content-Type: image/jpeg; + name="Cat_August_2010-4.jpeg" +Content-Transfer-Encoding: base64 + +SGVsbG8gd29ybGQ= + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84-- + ` + + const messageMultipartWithText = `Content-Type: multipart/mixed; + boundary="Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84" +Subject: A new message Part2 +Date: Mon, 13 Mar 2023 16:06:16 +0100 + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84 +Content-Disposition: inline; + filename=Cat_August_2010-4.jpeg +Content-Type: image/jpeg; + name="Cat_August_2010-4.jpeg" +Content-Transfer-Encoding: base64 + +SGVsbG8gd29ybGQ= + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84 +Content-Type: text/html;charset=utf8 +Content-Transfer-Encoding: quoted-printable + +Hello world + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84-- +` + + const messageWithTextOnly = `Content-Type: text/plain;charset=utf8 +Content-Transfer-Encoding: quoted-printable +Subject: A new message Part3 +Date: Mon, 13 Mar 2023 16:06:16 +0100 + +Hello world + +` + + const messageMultipartWithoutTextWithTextAttachment = `Content-Type: multipart/mixed; + boundary="Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84" +Subject: A new message Part4 +Date: Mon, 13 Mar 2023 16:06:16 +0100 + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84 +Content-Type: text/plain; charset=UTF-8; name="text.txt" +Content-Disposition: attachment; filename="text.txt" +Content-Transfer-Encoding: base64 + +SGVsbG8gd29ybGQK + +--Apple-Mail=_E7AC06C7-4EB2-4453-8CBB-80F4412A7C84-- +` + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + _, _, err := s.CreateUser("recipient", password) + require.NoError(t, err) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { + senderUserID, err := bridge.LoginFull(ctx, username, password, nil, nil) + require.NoError(t, err) + + recipientUserID, err := bridge.LoginFull(ctx, "recipient", password, nil, nil) + require.NoError(t, err) + + senderInfo, err := bridge.GetUserInfo(senderUserID) + require.NoError(t, err) + + recipientInfo, err := bridge.GetUserInfo(recipientUserID) + require.NoError(t, err) + + messages := []string{ + messageMultipartWithoutText, + messageMultipartWithText, + messageWithTextOnly, + messageMultipartWithoutTextWithTextAttachment, + } + + for _, m := range messages { + // Dial the server. + client, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) + require.NoError(t, err) + defer client.Close() //nolint:errcheck + + // Upgrade to TLS. + require.NoError(t, client.StartTLS(&tls.Config{InsecureSkipVerify: true})) + + // Authorize with SASL LOGIN. + require.NoError(t, client.Auth(sasl.NewLoginClient( + senderInfo.Addresses[0], + string(senderInfo.BridgePass)), + )) + + // Send the message. + require.NoError(t, client.SendMail( + senderInfo.Addresses[0], + []string{recipientInfo.Addresses[0]}, + strings.NewReader(m), + )) + } + + // Connect the sender IMAP client. + senderIMAPClient, err := client.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetIMAPPort()))) + require.NoError(t, err) + require.NoError(t, senderIMAPClient.Login(senderInfo.Addresses[0], string(senderInfo.BridgePass))) + defer senderIMAPClient.Logout() //nolint:errcheck + + // Connect the recipient IMAP client. + recipientIMAPClient, err := client.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetIMAPPort()))) + require.NoError(t, err) + require.NoError(t, recipientIMAPClient.Login(recipientInfo.Addresses[0], string(recipientInfo.BridgePass))) + defer recipientIMAPClient.Logout() //nolint:errcheck + + require.Eventually(t, func() bool { + messages, err := clientFetch(senderIMAPClient, `Sent`, imap.FetchBodyStructure) + require.NoError(t, err) + require.Equal(t, 4, len(messages)) + + // messages may not be in order + for _, message := range messages { + switch { + case message.Envelope.Subject == "A new message": + // The message that was sent should now include an empty text/plain body part since there was none + // in the original message. + require.Equal(t, 2, len(message.BodyStructure.Parts)) + + require.Equal(t, "text", message.BodyStructure.Parts[0].MIMEType) + require.Equal(t, "plain", message.BodyStructure.Parts[0].MIMESubType) + require.Equal(t, uint32(0), message.BodyStructure.Parts[0].Size) + require.Equal(t, "image", message.BodyStructure.Parts[1].MIMEType) + require.Equal(t, "jpeg", message.BodyStructure.Parts[1].MIMESubType) + + case message.Envelope.Subject == "A new message Part2": + // This message already has a text body, should be unchanged + require.Equal(t, 2, len(message.BodyStructure.Parts)) + + require.Equal(t, "image", message.BodyStructure.Parts[1].MIMEType) + require.Equal(t, "jpeg", message.BodyStructure.Parts[1].MIMESubType) + require.Equal(t, "text", message.BodyStructure.Parts[0].MIMEType) + require.Equal(t, "html", message.BodyStructure.Parts[0].MIMESubType) + + case message.Envelope.Subject == "A new message Part3": + // This message already has a text body, should be unchanged + require.Equal(t, 0, len(message.BodyStructure.Parts)) + + require.Equal(t, "text", message.BodyStructure.MIMEType) + require.Equal(t, "plain", message.BodyStructure.MIMESubType) + + case message.Envelope.Subject == "A new message Part4": + // The message that was sent should now include an empty text/plain body part since even though + // there was only a text/plain attachment in the original message. + require.Equal(t, 2, len(message.BodyStructure.Parts)) + + require.Equal(t, "text", message.BodyStructure.Parts[0].MIMEType) + require.Equal(t, "plain", message.BodyStructure.Parts[0].MIMESubType) + require.Equal(t, uint32(0), message.BodyStructure.Parts[0].Size) + require.Equal(t, "text", message.BodyStructure.Parts[1].MIMEType) + require.Equal(t, "plain", message.BodyStructure.Parts[1].MIMESubType) + require.Equal(t, "attachment", message.BodyStructure.Parts[1].Disposition) + } + } + + return true + }, 10*time.Second, 100*time.Millisecond) + }) + }) +} diff --git a/internal/bridge/sync_test.go b/internal/bridge/sync_test.go index 329a42c5..182986c2 100644 --- a/internal/bridge/sync_test.go +++ b/internal/bridge/sync_test.go @@ -351,7 +351,7 @@ func withClient(ctx context.Context, t *testing.T, s *server.Server, username st fn(ctx, c) } -func clientFetch(client *client.Client, mailbox string) ([]*imap.Message, error) { +func clientFetch(client *client.Client, mailbox string, extraItems ...imap.FetchItem) ([]*imap.Message, error) { status, err := client.Select(mailbox, false) if err != nil { return nil, err @@ -363,10 +363,13 @@ func clientFetch(client *client.Client, mailbox string) ([]*imap.Message, error) resCh := make(chan *imap.Message) + fetchItems := []imap.FetchItem{imap.FetchFlags, imap.FetchEnvelope, imap.FetchUid, imap.FetchBodyStructure, "BODY.PEEK[]"} + fetchItems = append(fetchItems, extraItems...) + go func() { if err := client.Fetch( &imap.SeqSet{Set: []imap.Seq{{Start: 1, Stop: status.Messages}}}, - []imap.FetchItem{imap.FetchFlags, imap.FetchEnvelope, imap.FetchUid, "BODY.PEEK[]"}, + fetchItems, resCh, ); err != nil { panic(err) diff --git a/internal/user/smtp.go b/internal/user/smtp.go index 0fb6fafd..1a0dd706 100644 --- a/internal/user/smtp.go +++ b/internal/user/smtp.go @@ -117,6 +117,10 @@ func (user *User) sendMail(authID string, from string, to []string, r io.Reader) return fmt.Errorf("failed to get first key: %w", err) } + // Ensure that there is always a text/html or text/plain body part. This is required by the API. If none + // exists and empty text part will be added. + parser.AttachEmptyTextPartIfNoneExists() + // If we have to attach the public key, do it now. if settings.AttachPublicKey { key, err := addrKR.GetKey(0) diff --git a/pkg/message/parser/parser.go b/pkg/message/parser/parser.go index 8266f6fb..e911c76b 100644 --- a/pkg/message/parser/parser.go +++ b/pkg/message/parser/parser.go @@ -20,6 +20,7 @@ package parser import ( "fmt" "io" + "strings" "github.com/emersion/go-message" "github.com/sirupsen/logrus" @@ -81,6 +82,39 @@ func (p *Parser) AttachPublicKey(key, keyName string) { }) } +func (p *Parser) AttachEmptyTextPartIfNoneExists() { + root := p.Root() + if root.isMultipartMixed() { + for _, v := range root.children { + // Must be an attachment of sorts, skip. + if v.Header.Has("Content-Disposition") { + continue + } + contentType, _, err := v.Header.ContentType() + if err == nil && strings.HasPrefix(contentType, "text/") { + // Message already has text part + return + } + } + } else { + contentType, _, err := root.Header.ContentType() + if err == nil && strings.HasPrefix(contentType, "text/") { + // Message already has text part + return + } + } + + h := message.Header{} + + h.Set("Content-Type", "text/plain;charset=utf8") + h.Set("Content-Transfer-Encoding", "quoted-printable") + + p.Root().AddChild(&Part{ + Header: h, + Body: nil, + }) +} + // Section returns the message part referred to by the given section. A section // is zero or more integers. For example, section 1.2.3 will return the third // part of the second part of the first part of the message.