Changelog

.gitlab-ci.yml

@@ -82,7 +82,9 @@ dependency-updates:
   script:
     - make build
   artifacts:
-    expire_in: 2 week
+    # Note: The latest artifacts for refs are locked against deletion, and kept regardless of the expiry time.
+    # Introduced in GitLab 13.0 behind a disabled feature flag, and made the default behavior in GitLab 13.4.
+    expire_in: 1 day
 
 build-linux:
   extends: .build-base

Changelog.md

@@ -2,15 +2,29 @@
 
 Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
-## Unreleased
+## [IE 1.1.2] Danube (beta 2020-09-xx)
 
-## [IE 1.1.x] Danube (v1.1.0 beta 2020-09-XX)
+### Fixed
+* GODT-770 Better handling of extraneous end-of-mail indicator.
+
+### Changed
+* Bump crypto version to v0.0.0-20200818122824-ed5d25e28db8
+
+
+## [IE 1.1.1] Danube (beta 2020-09-xx) [Bridge 1.4.1] Forth (beta 2020-09-xx)
+
+### Fixed
+* GODT-752 Parsing message with empty addresses.
+* GODT-752 Parsing non-utf8 multipart/alternative message.
+* GODT-752 Parsing message with duplicate charset parameter.
+
+## [IE 1.1.0] Danube
 
 ### Fixed
 * GODT-703 Import-Export showed always at least one total message.
 * GODT-738 Fix for mbox files with long lines.
 
-## [Bridge 1.4.x] Forth (v1.4.0 beta 2020-09-XX)
+## [Bridge 1.4.0] Forth
 
 ### Added
 * GODT-682 Persistent anonymous API cookies for Import-Export.

go.mod

@@ -78,5 +78,5 @@ replace (
 	github.com/emersion/go-mbox => github.com/ProtonMail/mbox v0.0.0-20200918064939-909a18c9af45
 	github.com/emersion/go-smtp => github.com/ProtonMail/go-smtp v0.0.0-20181206232543-8261df20d309
 	github.com/jameskeane/bcrypt => github.com/ProtonMail/bcrypt v0.0.0-20170924085257-7509ea014998
-	golang.org/x/crypto => github.com/ProtonMail/crypto v0.0.0-20200416114516-1fa7f403fb9c
+	golang.org/x/crypto => github.com/ProtonMail/crypto v0.0.0-20200818122824-ed5d25e28db8
 )

go.sum

@@ -1,13 +1,12 @@
 github.com/0xAX/notificator v0.0.0-20191016112426-3962a5ea8da1 h1:j9HaafapDbPbGRDku6e/HRs6KBMcKHiWcm1/9Sbxnl4=
 github.com/0xAX/notificator v0.0.0-20191016112426-3962a5ea8da1/go.mod h1:NtXa9WwQsukMHZpjNakTTz0LArxvGYdPA9CjIcUSZ6s=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
-github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
 github.com/Masterminds/semver/v3 v3.1.0 h1:Y2lUDsFKVRSYGojLJ1yLxSXdMmMYTYls0rCvoqmMUQk=
 github.com/Masterminds/semver/v3 v3.1.0/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
 github.com/ProtonMail/bcrypt v0.0.0-20170924085257-7509ea014998 h1:YT2uVwQiRQZxCaaahwfcgTq2j3j66w00n/27gb/zubs=
 github.com/ProtonMail/bcrypt v0.0.0-20170924085257-7509ea014998/go.mod h1:HecWFHognK8GfRDGnFQbW/LiV7A3MX3gZVs45vk5h8I=
-github.com/ProtonMail/crypto v0.0.0-20200416114516-1fa7f403fb9c h1:DAvlgde2Stu18slmjwikiMPs/CKPV35wSvmJS34z0FU=
+github.com/ProtonMail/crypto v0.0.0-20200818122824-ed5d25e28db8 h1:u1j0xLTrCHpNS40B6m4Sv3IVUz5m9jt+AnTIopT3IgM=
-github.com/ProtonMail/crypto v0.0.0-20200416114516-1fa7f403fb9c/go.mod h1:Pxr7w4gA2ikI4sWyYwEffm+oew1WAJHzG1SiDpQMkrI=
+github.com/ProtonMail/crypto v0.0.0-20200818122824-ed5d25e28db8/go.mod h1:Pxr7w4gA2ikI4sWyYwEffm+oew1WAJHzG1SiDpQMkrI=
 github.com/ProtonMail/docker-credential-helpers v1.1.0 h1:+kvUIpwWcbtP3WFv5sSvkFn/XLzSqPOB5AAthuk9xPk=
 github.com/ProtonMail/docker-credential-helpers v1.1.0/go.mod h1:mK0aBveCxhnQ756AmaTfXMZDeULvheYVhF/MWMErN5g=
 github.com/ProtonMail/go-appdir v1.1.0 h1:9hdNDlU9kTqRKVNzmoqah8qqrj5QZyLByQdwQNlFWig=
@@ -68,8 +67,6 @@ github.com/emersion/go-imap-specialuse v0.0.0-20200722111535-598ff00e4075 h1:z8T
 github.com/emersion/go-imap-specialuse v0.0.0-20200722111535-598ff00e4075/go.mod h1:/nybxhI8kXom8Tw6BrHMl42usALvka6meORflnnYwe4=
 github.com/emersion/go-imap-unselect v0.0.0-20171113212723-b985794e5f26 h1:FiSb8+XBQQSkcX3ubr+1tAtlRJBYaFmRZqOAweZ9Wy8=
 github.com/emersion/go-imap-unselect v0.0.0-20171113212723-b985794e5f26/go.mod h1:+gnnZx3Mg3MnCzZrv0eZdp5puxXQUgGT/6N6L7ShKfM=
-github.com/emersion/go-mbox v1.0.0 h1:HN6aKbyqmgIfK9fS/gen+NRr2wXLSxZXWfdAIAnzQPc=
-github.com/emersion/go-mbox v1.0.0/go.mod h1:Yp9IVuuOYLEuMv4yjgDHvhb5mHOcYH6x92Oas3QqEZI=
 github.com/emersion/go-message v0.11.1/go.mod h1:C4jnca5HOTo4bGN9YdqNQM9sITuT3Y0K6bSUw9RklvY=
 github.com/emersion/go-message v0.12.1-0.20200903165315-e1abe21f389a h1:3C6qIGgPr1qAT0ikRD5NbyKpME/iHCDeXhpv/JJsFsE=
 github.com/emersion/go-message v0.12.1-0.20200903165315-e1abe21f389a/go.mod h1:kYIioST9GDHte9/BRWgi93rpqbDuFftMjKSMaXS8ABo=
@@ -101,8 +98,6 @@ github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/U
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-multierror v1.1.0 h1:B9UzwGQJehnUY1yNrnwREHc3fGbC2xefo8g4TbElacI=
 github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA=
-github.com/jameshoulahan/go-imap v0.0.0-20200728140727-d57327f48843 h1:suxlO4AC4E4bjueAsL0m+qp8kmkxRWMGj+5bBU/KJ8g=
-github.com/jameshoulahan/go-imap v0.0.0-20200728140727-d57327f48843/go.mod h1:yKASt+C3ZiDAiCSssxg9caIckWF/JG7ZQTO7GAmvicU=
 github.com/jaytaylor/html2text v0.0.0-20200412013138-3577fbdbcff7 h1:g0fAGBisHaEQ0TRq1iBvemFRf+8AEWEmBESSiWB3Vsc=
 github.com/jaytaylor/html2text v0.0.0-20200412013138-3577fbdbcff7/go.mod h1:CVKlgaMiht+LXvHG173ujK6JUhZXKb2u/BQtjPDIvyk=
 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA=

pkg/message/header.go

@@ -152,6 +152,10 @@ func parseAddressComment(raw string) string {
 }
 
 func parseAddressList(val string) (addrs []*mail.Address, err error) {
+	if val == "" || val == "<>" {
+		return
+	}
+
 	addrs, err = mail.ParseAddressList(parseAddressComment(val))
 	if err == nil {
 		if addrs == nil {

pkg/message/parser.go

@@ -103,7 +103,11 @@ func convertForeignEncodings(p *parser.Parser) error {
 			return p.ConvertToUTF8()
 		}).
 		RegisterDefaultHandler(func(p *parser.Part) error {
-			t, _, _ := p.Header.ContentType()
+			t, params, _ := p.ContentType()
+			// multipart/alternative, for example, can contain extra charset.
+			if params != nil && params["charset"] != "" {
+				return p.ConvertToUTF8()
+			}
 			logrus.WithField("type", t).Trace("Not converting part to utf-8")
 			return nil
 		}).
@@ -293,7 +297,7 @@ func allPartsHaveContentType(parts parser.Parts, contentType string) bool {
 	}
 
 	for _, part := range parts {
-		t, _, err := part.Header.ContentType()
+		t, _, err := part.ContentType()
 		if err != nil {
 			return false
 		}
@@ -329,7 +333,7 @@ func determineMIMEType(p *parser.Parser) (string, error) {
 // getPlainBody returns the body of the given part, converting html to
 // plaintext where possible.
 func getPlainBody(part *parser.Part) []byte {
-	contentType, _, err := part.Header.ContentType()
+	contentType, _, err := part.ContentType()
 	if err != nil {
 		return part.Body
 	}

pkg/message/parser/handler.go

@@ -17,7 +17,9 @@
 
 package parser
 
-import "regexp"
+import (
+	"regexp"
+)
 
 type HandlerFunc func(*Part) error
 
@@ -35,7 +37,7 @@ func (h *handler) matchType(p *Part) bool {
 		return false
 	}
 
-	t, _, err := p.Header.ContentType()
+	t, _, err := p.ContentType()
 	if err != nil {
 		t = ""
 	}

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
 	}

pkg/message/parser/part.go

@@ -40,6 +40,18 @@ type Part struct {
 	children Parts
 }
 
+func (p *Part) ContentType() (string, map[string]string, error) {
+	t, params, err := p.Header.ContentType()
+	if err != nil {
+		// go-message's implementation of ContentType() doesn't handle duplicate parameters
+		// e.g. Content-Type: text/plain; charset=utf-8; charset=UTF-8
+		// so if it fails, we try again with pmmime's implementation, which does.
+		t, params, err = pmmime.ParseMediaType(p.Header.Get("Content-Type"))
+	}
+
+	return t, params, err
+}
+
 func (p *Part) Child(n int) (part *Part, err error) {
 	if len(p.children) < n {
 		return nil, errors.New("no such part")
@@ -72,7 +84,7 @@ func (p *Part) AddChild(child *Part) {
 func (p *Part) ConvertToUTF8() error {
 	logrus.Trace("Converting part to utf-8")
 
-	t, params, err := p.Header.ContentType()
+	t, params, err := p.ContentType()
 	if err != nil {
 		return err
 	}
@@ -102,7 +114,7 @@ func (p *Part) ConvertMetaCharset() error {
 
 	goquery.NewDocumentFromNode(doc).Find("meta").Each(func(n int, sel *goquery.Selection) {
 		if val, ok := sel.Attr("content"); ok {
-			t, params, err := mime.ParseMediaType(val)
+			t, params, err := pmmime.ParseMediaType(val)
 			if err != nil {
 				return
 			}
@@ -163,7 +175,7 @@ func (p *Part) is7BitClean() bool {
 }
 
 func (p *Part) isMultipartMixed() bool {
-	t, _, err := p.Header.ContentType()
+	t, _, err := p.ContentType()
 	if err != nil {
 		return false
 	}

pkg/message/parser/part_test.go

@@ -49,7 +49,7 @@ func TestPart(t *testing.T) {
 		part, err := p.Section(getSectionNumber(partNumber))
 		require.NoError(t, err)
 
-		contType, _, err := part.Header.ContentType()
+		contType, _, err := part.ContentType()
 		require.NoError(t, err)
 		assert.Equal(t, wantContType, contType)
 	}

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 <https://www.gnu.org/licenses/>.
+
+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)
+}

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 <https://www.gnu.org/licenses/>.
+
+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
+}

pkg/message/parser/visitor.go

@@ -58,7 +58,7 @@ func (v *Visitor) Visit() (interface{}, error) {
 }
 
 func (v *Visitor) visit(p *Part) (interface{}, error) {
-	t, _, err := p.Header.ContentType()
+	t, _, err := p.ContentType()
 	if err != nil {
 		return nil, err
 	}

pkg/message/parser_test.go

@@ -224,6 +224,21 @@ func TestParseTextPlainWithPlainAttachment(t *testing.T) {
 	assert.Equal(t, readerToString(attReaders[0]), "attachment")
 }
 
+func TestParseTextPlainEmptyAddresses(t *testing.T) {
+	f := getFileReader("text_plain_empty_addresses.eml")
+
+	m, _, plainBody, attReaders, err := Parse(f, "", "")
+	require.NoError(t, err)
+
+	assert.Equal(t, `"Sender" <sender@pm.me>`, m.Sender.String())
+	assert.Equal(t, `"Receiver" <receiver@pm.me>`, m.ToList[0].String())
+
+	assert.Equal(t, "body", m.Body)
+	assert.Equal(t, "body", plainBody)
+
+	assert.Len(t, attReaders, 0)
+}
+
 func TestParseTextPlainWithImageInline(t *testing.T) {
 	f := getFileReader("text_plain_image_inline.eml")
 
@@ -244,6 +259,21 @@ func TestParseTextPlainWithImageInline(t *testing.T) {
 	assert.Equal(t, 8, img.Height)
 }
 
+func TestParseTextPlainWithDuplicateCharset(t *testing.T) {
+	f := getFileReader("text_plain_duplicate_charset.eml")
+
+	m, _, plainBody, attReaders, err := Parse(f, "", "")
+	require.NoError(t, err)
+
+	assert.Equal(t, `"Sender" <sender@pm.me>`, m.Sender.String())
+	assert.Equal(t, `"Receiver" <receiver@pm.me>`, m.ToList[0].String())
+
+	assert.Equal(t, "body", m.Body)
+	assert.Equal(t, "body", plainBody)
+
+	assert.Len(t, attReaders, 0)
+}
+
 func TestParseWithMultipleTextParts(t *testing.T) {
 	f := getFileReader("multiple_text_parts.eml")
 
@@ -416,6 +446,40 @@ func TestParseMultipartAlternativeNested(t *testing.T) {
 	assert.Equal(t, "*multipart 2.1*\n\n", plainBody)
 }
 
+func TestParseMultipartAlternativeLatin1(t *testing.T) {
+	f := getFileReader("multipart_alternative_latin1.eml")
+
+	m, _, plainBody, _, err := Parse(f, "", "")
+	require.NoError(t, err)
+
+	assert.Equal(t, `"schizofrenic" <schizofrenic@pm.me>`, m.Sender.String())
+	assert.Equal(t, `<pmbridgeietest@outlook.com>`, m.ToList[0].String())
+
+	assert.Equal(t, `<html><head>
+    <meta http-equiv="content-type" content="text/html; charset=UTF-8"/>
+  </head>
+  <body>
+    <b>aoeuaoeu</b>
+  
+
+</body></html>`, m.Body)
+
+	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" <sender@sender.com>`, m.Sender.String())
+	assert.Equal(t, `"Receiver" <receiver@receiver.com>`, m.ToList[0].String())
+
+	assert.Equal(t, "<!DOCTYPE html><html><head></head><body>boo!</body></html>", m.Body)
+	assert.Equal(t, "boo!", plainBody)
+}
+
 func getFileReader(filename string) io.Reader {
 	f, err := os.Open(filepath.Join("testdata", filename))
 	if err != nil {

pkg/message/testdata/multipart_alternative_latin1.eml

@@ -0,0 +1,30 @@
+To: pmbridgeietest@outlook.com
+From: schizofrenic <schizofrenic@pm.me>
+Subject: aoeuaoeu
+Date: Thu, 30 Jul 2020 13:35:24 +0200
+MIME-Version: 1.0
+Content-Type: multipart/alternative; boundary="------------22BC647264E52252E386881A"; charset="iso-8859-1"
+Content-Language: en-US
+
+This is a multi-part message in MIME format.
+--------------22BC647264E52252E386881A
+Content-Type: text/plain
+Content-Transfer-Encoding: 7bit
+
+*aoeuaoeu*
+
+
+--------------22BC647264E52252E386881A
+Content-Type: text/html
+Content-Transfer-Encoding: 7bit
+
+<html>
+  <head>
+    <meta http-equiv="content-type" content="text/html; charset=iso-8859-1">
+  </head>
+  <body>
+    <b>aoeuaoeu</b>
+  </body>
+</html>
+
+--------------22BC647264E52252E386881A--

pkg/message/testdata/text_html_trailing_end_of_mail.eml

@@ -0,0 +1,8 @@
+From: "Sender" <sender@sender.com>
+To: "Receiver" <receiver@receiver.com>
+Content-Type: text/html; charset="utf-8"
+Content-Transfer-Encoding: base64
+MIME-Version: 1.0
+
+PCFET0NUWVBFIEhUTUw+CjxodG1sPjxib2R5PmJvbyE8L2JvZHk+PC9odG1sPg==
+.

pkg/message/testdata/text_plain_duplicate_charset.eml

@@ -0,0 +1,5 @@
+From: Sender <sender@pm.me>
+To: Receiver <receiver@pm.me>
+Content-Type: text/plain; charset=utf-8; charset=UTF-8
+
+body

pkg/message/testdata/text_plain_empty_addresses.eml

@@ -0,0 +1,6 @@
+From: Sender <sender@pm.me>
+To: Receiver <receiver@pm.me>
+CC: 
+Reply-To: <>
+
+body

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

test/features/bridge/imap/message/import.feature

@@ -4,7 +4,6 @@ Feature: IMAP import messages
     And there is IMAP client logged in as "user"
     And there is IMAP client selected in "INBOX"
 
-  @ignore
   Scenario: Import message with double charset in content type
     When IMAP client imports message to "INBOX"
       """