mail: handle multipart without closing boundary

Emails with multipart that was not properly closed may cause the parser
to go into an infinite loop. In that case, io.Copy in findTextParts may
return a de-facto EOF that is not *the* io.EOF. (Same with NextPart,
this is not handled in this commit as I don't have a test case).

Introduce an arbitrary limit of parts to prevent this from happening in
the future. Also, add another error handler for this special unexpected
EOF.

Signed-off-by: Franciszek Stachura <fbstachura@gmail.com>
Acked-by: Robin Jarry <robin@jarry.cc>

Author: fstachura

pkg/mail/content.go

@@ -26,7 +26,8 @@ func findTextParts(m *mail.Reader) []textPart {
 	var results []textPart
 	var buf strings.Builder
 
-	for {
+	// Arbitrary limit of parts to prevent an infinite loop
+	for range 1024 {
 		part, err := m.NextPart()
 		if errors.Is(err, io.EOF) {
 			break
@@ -57,6 +58,10 @@ func findTextParts(m *mail.Reader) []textPart {
 
 		buf.Reset()
 		if n, err := io.Copy(&buf, part.Body); err != nil {
+			if strings.Contains(err.Error(), "EOF") {
+				break
+			}
+
 			log.Warnf("failed to read part body: %s", err)
 			continue
 		} else if n == 0 {

pkg/mail/content_test.go

@@ -149,3 +149,10 @@ func TestInvalidCharset(t *testing.T) {
 	assert.NotEmpty(t, diff, "expected diff despite invalid charset")
 	assert.NotEmpty(t, comment, "expected comment despite invalid charset")
 }
+
+func TestMultipartWithoutClosingBoundary(t *testing.T) {
+	m := openTestMail(t, "mail/0028-multipart-without-closing-boundary.mbox")
+	diff, comment := FindPatchContent(m)
+	assert.Empty(t, diff, "expected no diff")
+	assert.NotEmpty(t, comment, "expected a comment despite multipart without closing boundary")
+}

pkg/mail/testdata/mail/0028-multipart-without-closing-boundary.mbox

@@ -0,0 +1,24 @@
+From stephenfinucane@hotmail.com Wed Jun 20 13:35:48 2018
+From: Stephen Finucane <stephenfinucane@hotmail.com>
+To: "stephen@that.guru" <stephen@that.guru>
+Subject: Re: [PATCH] parsemail: ignore html part of multi-part comments
+Date: Wed, 20 Jun 2018 13:35:37 +0000
+Message-ID: <DB5PR03MB18774049A0E62D211988EC8CA3770@DB5PR03MB1877.eurprd03.prod.outlook.com>
+References: <20180620122142.9917-1-yuri.volchkov@gmail.com>
+In-Reply-To: <20180620122142.9917-1-yuri.volchkov@gmail.com>
+Content-Type: multipart/alternative;
+	boundary="_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_"
+MIME-Version: 1.0
+
+
+--_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_
+Content-Type: text/plain; charset="iso-8859-1"
+Content-Transfer-Encoding: 8bit
+
+Yup, this looks sensible to me. Replying from Outlook's awful HTML editor to get
+a sample comment to test with.
+
+Stephen
+
+
+--_000_DB5PR03MB18774049A0E62D211988EC8CA3770DB5PR03MB1877eurp_