From 2ae78db660603b48d74d369778cba7d66f320739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Mon, 31 Jan 2022 15:51:19 +0100 Subject: [PATCH] satellite/email: add delimiter to close the last part of Multipart emails The existing implementation doesn't send proper rfc1341 compatible mails according to https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html as the closing `--boundary_id--` is missing from our mails. (see 7.2.1 from the standard). Mailservers which are not very flexible, deny the mails. Example mail log: (see the missing boundary delimiter at the end). ``` Subject: Activate your email From: "Storj DCS - EU1" To: "..." <...> ... --26d7220f6f1c9f6fb47b535319eb15dce513bb6e1d941a0efddf25e96712 Content-Type: text/html; charset=UTF-8 .... . smtp: DATA error {"msg_id":"6d82c9e3","reason":"unexpected EOF"} smtp: 554 5.0.0 Internal server error (msg ID = 6d82c9e3) ``` This patch moves all the Multipart writing to a function to make sure that the `wr.Close()` (which writes out the last part) is executed BEFORE `body.Bytes()` (defer added it AFTER `body.Bytes()` was calculated) Change-Id: I8f18fc81b1857b646470eab32e73d6cbdc50d2ad --- private/post/message.go | 84 ++++++++++++++++++++---------------- private/post/message_test.go | 39 +++++++++++++++++ 2 files changed, 85 insertions(+), 38 deletions(-) create mode 100644 private/post/message_test.go diff --git a/private/post/message.go b/private/post/message.go index a2a5c3954..7feca45bc 100644 --- a/private/post/message.go +++ b/private/post/message.go @@ -66,44 +66,9 @@ func (msg *Message) Bytes() (data []byte, err error) { switch { // multipart upload case len(msg.Parts) > 0: - wr := multipart.NewWriter(&body) - defer func() { err = errs.Combine(err, wr.Close()) }() - - fmt.Fprintf(&body, "Content-Type: multipart/alternative;") - fmt.Fprintf(&body, "\tboundary=\"%v\"\r\n", wr.Boundary()) - fmt.Fprintf(&body, "\r\n") - - var sub io.Writer - - if len(msg.PlainText) > 0 { - sub, err := wr.CreatePart(textproto.MIMEHeader{ - "Content-Type": []string{"text/plain; charset=UTF-8; format=flowed"}, - "Content-Transfer-Encoding": []string{"quoted-printable"}, - }) - if err != nil { - return nil, Error.Wrap(err) - } - - enc := quotedprintable.NewWriter(sub) - defer func() { err = errs.Combine(err, enc.Close()) }() - - _, err = enc.Write([]byte(msg.PlainText)) - if err != nil { - return nil, Error.Wrap(err) - } - } - - for _, part := range msg.Parts { - header := textproto.MIMEHeader{"Content-Type": []string{mime.QEncoding.Encode("utf-8", part.Type)}} - if part.Encoding != "" { - header["Content-Transfer-Encoding"] = []string{mime.QEncoding.Encode("utf-8", part.Encoding)} - } - if part.Disposition != "" { - header["Content-Disposition"] = []string{mime.QEncoding.Encode("utf-8", part.Disposition)} - } - - sub, _ = wr.CreatePart(header) - fmt.Fprint(sub, part.Content) + err = msg.writeMultipart(&body) + if err != nil { + return nil, err } // fallback if there are no parts, write PlainText with appropriate Content-Type @@ -122,6 +87,49 @@ func (msg *Message) Bytes() (data []byte, err error) { return tocrlf(body.Bytes()), nil } +func (msg *Message) writeMultipart(body *bytes.Buffer) (err error) { + wr := multipart.NewWriter(body) + defer func() { err = errs.Combine(err, wr.Close()) }() + + fmt.Fprintf(body, "Content-Type: multipart/alternative;") + fmt.Fprintf(body, "\tboundary=\"%v\"\r\n", wr.Boundary()) + fmt.Fprintf(body, "\r\n") + + var sub io.Writer + + if len(msg.PlainText) > 0 { + sub, err := wr.CreatePart(textproto.MIMEHeader{ + "Content-Type": []string{"text/plain; charset=UTF-8; format=flowed"}, + "Content-Transfer-Encoding": []string{"quoted-printable"}, + }) + if err != nil { + return Error.Wrap(err) + } + + enc := quotedprintable.NewWriter(sub) + defer func() { err = errs.Combine(err, enc.Close()) }() + + _, err = enc.Write([]byte(msg.PlainText)) + if err != nil { + return Error.Wrap(err) + } + } + + for _, part := range msg.Parts { + header := textproto.MIMEHeader{"Content-Type": []string{mime.QEncoding.Encode("utf-8", part.Type)}} + if part.Encoding != "" { + header["Content-Transfer-Encoding"] = []string{mime.QEncoding.Encode("utf-8", part.Encoding)} + } + if part.Disposition != "" { + header["Content-Disposition"] = []string{mime.QEncoding.Encode("utf-8", part.Disposition)} + } + + sub, _ = wr.CreatePart(header) + fmt.Fprint(sub, part.Content) + } + return nil +} + func tocrlf(data []byte) []byte { lf := bytes.ReplaceAll(data, []byte("\r\n"), []byte("\n")) crlf := bytes.ReplaceAll(lf, []byte("\n"), []byte("\r\n")) diff --git a/private/post/message_test.go b/private/post/message_test.go new file mode 100644 index 000000000..fbe1a95f1 --- /dev/null +++ b/private/post/message_test.go @@ -0,0 +1,39 @@ +// Copyright (C) 2022 Storj Labs, Inc. +// See LICENSE for copying information + +package post + +import ( + "net/mail" + "regexp" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMessage_ClosingLastPart(t *testing.T) { + from := mail.Address{Name: "No reply", Address: "noreply@eu1.storj.io"} + + m := &Message{ + From: from, + To: []mail.Address{{Name: "Foo Bar", Address: "foo@storj.io"}}, + Subject: "This is a proper test mail", + PlainText: "", + Parts: []Part{ + { + Type: "text/html; charset=UTF-8", + Content: string("

ahoj

"), + }, + }, + } + + data, err := m.Bytes() + require.NoError(t, err) + lines := strings.Split(string(data), "\n") + + // last part should be closed. see 7.2.1 of https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html + final := regexp.MustCompile("--.+--") + lastNonEmptyLine := lines[len(lines)-2] + require.True(t, final.MatchString(lastNonEmptyLine), "Last line '%s' doesn't include RFC1341 distinguished delimiter", lastNonEmptyLine) +}