From 78741830520ef1301e2d1c11ad9be16f7154a2af Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Thu, 1 Oct 2020 14:40:01 +0200 Subject: [PATCH] fix(GODT-770): handle extraneous end-of-mail --- Changelog.md | 3 + pkg/message/parser/parser.go | 2 +- pkg/message/parser/trimmer.go | 56 +++++++++++++++++++ pkg/message/parser/trimmer_test.go | 55 ++++++++++++++++++ pkg/message/parser_test.go | 13 +++++ .../text_html_trailing_end_of_mail.eml | 8 +++ pkg/pmapi/mocks/mocks.go | 5 +- 7 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 pkg/message/parser/trimmer.go create mode 100644 pkg/message/parser/trimmer_test.go create mode 100644 pkg/message/testdata/text_html_trailing_end_of_mail.eml diff --git a/Changelog.md b/Changelog.md index f95395e2..196fe17d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,9 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) ## Unreleased +### Fixed +* GODT-770 Better handling of extraneous end-of-mail indicator. + ### Changed * Bump crypto version to v0.0.0-20200818122824-ed5d25e28db8 diff --git a/pkg/message/parser/parser.go b/pkg/message/parser/parser.go index 7d4d4f7b..6639006e 100644 --- a/pkg/message/parser/parser.go +++ b/pkg/message/parser/parser.go @@ -32,7 +32,7 @@ type Parser struct { func New(r io.Reader) (*Parser, error) { p := new(Parser) - entity, err := message.Read(r) + entity, err := message.Read(newEndOfMailTrimmer(r)) if err != nil && !message.IsUnknownCharset(err) { return nil, err } diff --git a/pkg/message/parser/trimmer.go b/pkg/message/parser/trimmer.go new file mode 100644 index 00000000..fe90a202 --- /dev/null +++ b/pkg/message/parser/trimmer.go @@ -0,0 +1,56 @@ +// Copyright (c) 2020 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ProtonMail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with ProtonMail Bridge. If not, see . + +package parser + +import ( + "bytes" + "io" +) + +const endOfMail = "\r\n.\r\n" + +// endOfMailTrimmer wraps a reader to trim the End-Of-Mail indicator at the end +// of the input, if present. +// +// During SMTP sending of a message, the DATA command indicates that you are +// about to send the text (or body) of the message. The message text must end +// with "\r\n.\r\n." I'm 99% sure that these 5 bytes should not be considered +// part of the message body. However, some mail servers keep them as part of +// the message, which our parser sometimes doesn't like. Therefore, we strip +// them if we find them. +type endOfMailTrimmer struct { + r io.Reader + buf bytes.Buffer +} + +func newEndOfMailTrimmer(r io.Reader) *endOfMailTrimmer { + return &endOfMailTrimmer{r: r} +} + +func (r *endOfMailTrimmer) Read(p []byte) (int, error) { + _, err := io.CopyN(&r.buf, r.r, int64(len(p)+len(endOfMail)-r.buf.Len())) + if err != nil && err != io.EOF { + return 0, err + } + + if err == io.EOF && bytes.HasSuffix(r.buf.Bytes(), []byte(endOfMail)) { + r.buf.Truncate(r.buf.Len() - len(endOfMail)) + } + + return r.buf.Read(p) +} diff --git a/pkg/message/parser/trimmer_test.go b/pkg/message/parser/trimmer_test.go new file mode 100644 index 00000000..a8d7d66b --- /dev/null +++ b/pkg/message/parser/trimmer_test.go @@ -0,0 +1,55 @@ +// Copyright (c) 2020 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ProtonMail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with ProtonMail Bridge. If not, see . + +package parser + +import ( + "io" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEndOfMailTrimmer(t *testing.T) { + var tests = []struct { + in string + out string + }{ + {"string without eom", "string without eom"}, + {"string with eom\r\n.\r\n", "string with eom"}, + {"string with eom\r\n.\r\nin the middle", "string with eom\r\n.\r\nin the middle"}, + } + + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + res := dumbRead(newEndOfMailTrimmer(strings.NewReader(tt.in))) + assert.Equal(t, tt.out, string(res)) + }) + } +} + +func dumbRead(r io.Reader) []byte { + out := []byte{} + + b := make([]byte, 1) + for _, err := r.Read(b); err == nil; _, err = r.Read(b) { + out = append(out, b...) + } + + return out +} diff --git a/pkg/message/parser_test.go b/pkg/message/parser_test.go index 5c2fabb0..9c5c7075 100644 --- a/pkg/message/parser_test.go +++ b/pkg/message/parser_test.go @@ -467,6 +467,19 @@ func TestParseMultipartAlternativeLatin1(t *testing.T) { assert.Equal(t, "*aoeuaoeu*\n\n", plainBody) } +func TestParseWithTrailingEndOfMailIndicator(t *testing.T) { + f := getFileReader("text_html_trailing_end_of_mail.eml") + + m, _, plainBody, _, err := Parse(f, "", "") + require.NoError(t, err) + + assert.Equal(t, `"Sender" `, m.Sender.String()) + assert.Equal(t, `"Receiver" `, m.ToList[0].String()) + + assert.Equal(t, "boo!", m.Body) + assert.Equal(t, "boo!", plainBody) +} + func getFileReader(filename string) io.Reader { f, err := os.Open(filepath.Join("testdata", filename)) if err != nil { diff --git a/pkg/message/testdata/text_html_trailing_end_of_mail.eml b/pkg/message/testdata/text_html_trailing_end_of_mail.eml new file mode 100644 index 00000000..ea8190b5 --- /dev/null +++ b/pkg/message/testdata/text_html_trailing_end_of_mail.eml @@ -0,0 +1,8 @@ +From: "Sender" +To: "Receiver" +Content-Type: text/html; charset="utf-8" +Content-Transfer-Encoding: base64 +MIME-Version: 1.0 + +PCFET0NUWVBFIEhUTUw+CjxodG1sPjxib2R5PmJvbyE8L2JvZHk+PC9odG1sPg== +. diff --git a/pkg/pmapi/mocks/mocks.go b/pkg/pmapi/mocks/mocks.go index 1e37cc2a..ecf7103c 100644 --- a/pkg/pmapi/mocks/mocks.go +++ b/pkg/pmapi/mocks/mocks.go @@ -5,11 +5,12 @@ package mocks import ( + io "io" + reflect "reflect" + crypto "github.com/ProtonMail/gopenpgp/v2/crypto" pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi" gomock "github.com/golang/mock/gomock" - io "io" - reflect "reflect" ) // MockClient is a mock of Client interface