Repository: jhillyerd/enmime
Pull request #262 (2022-09-05 14:31:05)

feat: ability to recover partial content from broken encoding
### --> Problem

**What I did:**
I wrote a simple email which has two parts (text/plain and text/html) with base64 encoded data, but with one characteristic: both part contents have one extra base64 character (so they are slightly broken). Instead of having 4\*n characters, the contents are (4\*n)+1 characters long.

**What I expected:**
ReadParts() returns the root Part, with a FirstChild, and this one with a NextSibling. FirstChild and NextSibling have the partially decoded base64 data (Content != nil, parser recovers partial data from the parts with broken encoding).

**What I got:**
ReadParts() returns the root Part, with a FirstChild, and this one with a NextSibling. FirstChild and NextSibling have Content == nil (Parser does not recover partial data)
The reason is that for each part a `base64.CorruptInputError` is thrown, which then is catched by `base64CorruptInputCheck()`, leaving the part content empty and returning nil error (`skipMalformedParts` flag on true/false would not change the output).

### --> Solution

With this particular scenario, in which there are text/plain and text/html contents, I would like to ignore that specific error (base64.CorruptInputError) and to keep the buffer read at that moment.
But the thing is that there might be different preferences (for now on, "policies"). For example:
- keep the buffer read only when base64.CorruptInputError is raised
- keep the buffer read only when another error is raised
- keep the buffer read only on a base64.CorruptInputError error and when ContentType is text/plain
- keep the buffer read only on a base64.CorruptInputError error and when ContentType is text/plain or text/html
- always keep the buffer read, no matter the error
- and so on ...

To solve this, I added a callback function `readPartErrorPolicy` to the `Parser` type, from which it will be possible to decide when to keep the buffer or not when an error is thrown while reading a part content.

> When no policy is sent to the parser, default behavior is kept.

**Release or branch I am using:**
`master`

fcisnerosrojo: > Thanks, code looks good overall, but I have some suggestions.

Thanks for your feedback @jhillyerd ! All suggestions were included in the last commit .

Patch:
diff --git a/options.go b/options.go
index 4139d7b..573e397 100644
--- a/options.go
+++ b/options.go
@@ -27,3 +27,14 @@ type multipartWOBoundaryAsSinglePartOption bool
 func (o multipartWOBoundaryAsSinglePartOption) apply(p *Parser) {
 	p.multipartWOBoundaryAsSinglePart = bool(o)
 }
+
+// SetReadPartErrorPolicy sets the given callback function to readPartErrorPolicy.
+func SetReadPartErrorPolicy(f ReadPartErrorPolicy) Option {
+	return readPartErrorPolicyOption(f)
+}
+
+type readPartErrorPolicyOption ReadPartErrorPolicy
+
+func (o readPartErrorPolicyOption) apply(p *Parser) {
+	p.readPartErrorPolicy = ReadPartErrorPolicy(o)
+}
diff --git a/parser.go b/parser.go
index 5a32dbb..5013028 100644
--- a/parser.go
+++ b/parser.go
@@ -1,10 +1,24 @@
 package enmime
 
+// ReadPartErrorPolicy allows to recover the buffer (or not) on an error when reading a Part content.
+//
+// See AllowCorruptTextPartErrorPolicy for usage.
+type ReadPartErrorPolicy func(*Part, error) bool
+
+// AllowCorruptTextPartErrorPolicy recovers partial content from base64.CorruptInputError when content type is text/plain or text/html.
+func AllowCorruptTextPartErrorPolicy(p *Part, err error) bool {
+	if IsBase64CorruptInputError(err) && (p.ContentType == ctTextHTML || p.ContentType == ctTextPlain) {
+		return true
+	}
+	return false
+}
+
 // Parser parses MIME.
 // Default parser is a valid one.
 type Parser struct {
 	skipMalformedParts              bool
 	multipartWOBoundaryAsSinglePart bool
+	readPartErrorPolicy             ReadPartErrorPolicy
 }
 
 // defaultParser is a Parser with default configuration.
diff --git a/part.go b/part.go
index 52737f5..482842c 100644
--- a/part.go
+++ b/part.go
@@ -160,11 +160,23 @@ func (p *Part) setupContentHeaders(mediaParams map[string]string) {
 	}
 }
 
+func (p *Part) readPartContent(r io.Reader, readPartErrorPolicy ReadPartErrorPolicy) ([]byte, error) {
+	buf, err := ioutil.ReadAll(r)
+	if err != nil {
+		if readPartErrorPolicy != nil && readPartErrorPolicy(p, err) {
+			p.addWarning(ErrorMalformedChildPart, "partial content: %s", err.Error())
+			return buf, nil
+		}
+		return nil, err
+	}
+	return buf, nil
+}
+
 // convertFromDetectedCharset attempts to detect the character set for the given part, and returns
 // an io.Reader that will convert from that charset to UTF-8. If the charset cannot be detected,
 // this method adds a warning to the part and automatically falls back to using
 // `convertFromStatedCharset` and returns the reader from that method.
-func (p *Part) convertFromDetectedCharset(r io.Reader) (io.Reader, error) {
+func (p *Part) convertFromDetectedCharset(r io.Reader, readPartErrorPolicy ReadPartErrorPolicy) (io.Reader, error) {
 	// Attempt to detect character set from part content.
 	var cd *chardet.Detector
 	switch p.ContentType {
@@ -174,7 +186,7 @@ func (p *Part) convertFromDetectedCharset(r io.Reader) (io.Reader, error) {
 		cd = chardet.NewTextDetector()
 	}
 
-	buf, err := ioutil.ReadAll(r)
+	buf, err := p.readPartContent(r, readPartErrorPolicy)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
@@ -249,7 +261,7 @@ func (p *Part) convertFromStatedCharset(r io.Reader) io.Reader {
 // decodeContent performs transport decoding (base64, quoted-printable) and charset decoding,
 // placing the result into Part.Content.  IO errors will be returned immediately; other errors
 // and warnings will be added to Part.Errors.
-func (p *Part) decodeContent(r io.Reader) error {
+func (p *Part) decodeContent(r io.Reader, readPartErrorPolicy ReadPartErrorPolicy) error {
 	// contentReader will point to the end of the content decoding pipeline.
 	contentReader := r
 	// b64cleaner aggregates errors, must maintain a reference to it to get them later.
@@ -277,7 +289,7 @@ func (p *Part) decodeContent(r io.Reader) error {
 	// Build charset decoding reader.
 	if validEncoding && strings.HasPrefix(p.ContentType, "text/") {
 		var err error
-		contentReader, err = p.convertFromDetectedCharset(contentReader)
+		contentReader, err = p.convertFromDetectedCharset(contentReader, readPartErrorPolicy)
 		if err != nil {
 			return p.base64CorruptInputCheck(err)
 		}
@@ -302,18 +314,28 @@ func (p *Part) decodeContent(r io.Reader) error {
 	return nil
 }
 
+// IsBase64CorruptInputError returns true when err is of type base64.CorruptInputError.
+//
+// It can be used to create ReadPartErrorPolicy functions.
+func IsBase64CorruptInputError(err error) bool {
+	switch errors.Cause(err).(type) {
+	case base64.CorruptInputError:
+		return true
+	default:
+		return false
+	}
+}
+
 // base64CorruptInputCheck will avoid fatal failure on corrupt base64 input
 //
 // This is a switch on errors.Cause(err).(type) for base64.CorruptInputError
 func (p *Part) base64CorruptInputCheck(err error) error {
-	switch errors.Cause(err).(type) {
-	case base64.CorruptInputError:
+	if IsBase64CorruptInputError(err) {
 		p.Content = nil
 		p.addError(ErrorMalformedBase64, err.Error())
 		return nil
-	default:
-		return err
 	}
+	return err
 }
 
 // Clone returns a clone of the current Part.
@@ -359,13 +381,13 @@ func (p Parser) ReadParts(r io.Reader) (*Part, error) {
 
 	if detectMultipartMessage(root, p.multipartWOBoundaryAsSinglePart) {
 		// Content is multipart, parse it.
-		err = parseParts(root, br, p.skipMalformedParts)
+		err = parseParts(root, br, p.skipMalformedParts, p.readPartErrorPolicy)
 		if err != nil {
 			return nil, err
 		}
 	} else {
 		// Content is text or data, decode it.
-		if err := root.decodeContent(br); err != nil {
+		if err := root.decodeContent(br, p.readPartErrorPolicy); err != nil {
 			return nil, err
 		}
 	}
@@ -373,7 +395,7 @@ func (p Parser) ReadParts(r io.Reader) (*Part, error) {
 }
 
 // parseParts recursively parses a MIME multipart document and sets each Parts PartID.
-func parseParts(parent *Part, reader *bufio.Reader, skipMalformedParts bool) error {
+func parseParts(parent *Part, reader *bufio.Reader, skipMalformedParts bool, readPartErrorPolicy ReadPartErrorPolicy) error {
 	firstRecursion := parent.Parent == nil
 	// Loop over MIME boundaries.
 	br := newBoundaryReader(reader, parent.Boundary)
@@ -409,7 +431,7 @@ func parseParts(parent *Part, reader *bufio.Reader, skipMalformedParts bool) err
 		// Insert this Part into the MIME tree.
 		if p.Boundary == "" {
 			// Content is text or data, decode it.
-			if err = p.decodeContent(bbr); err != nil {
+			if err = p.decodeContent(bbr, readPartErrorPolicy); err != nil {
 				if skipMalformedParts {
 					parent.addError(ErrorMalformedChildPart, "decode content: %s", err.Error())
 					continue
@@ -422,7 +444,7 @@ func parseParts(parent *Part, reader *bufio.Reader, skipMalformedParts bool) err
 
 		parent.AddChild(p)
 		// Content is another multipart.
-		if err = parseParts(p, bbr, skipMalformedParts); err != nil {
+		if err = parseParts(p, bbr, skipMalformedParts, readPartErrorPolicy); err != nil {
 			if skipMalformedParts {
 				parent.addError(ErrorMalformedChildPart, "parse parts: %s", err.Error())
 				continue


Test patch:
diff --git a/part_test.go b/part_test.go
index f962a9e..8df05c7 100644
--- a/part_test.go
+++ b/part_test.go
@@ -427,6 +427,163 @@ func TestMultiSkipMalformedPart(t *testing.T) {
 	test.ContentContainsString(t, p.Content, want)
 }
 
+func TestReadPartErrorPolicy(t *testing.T) {
+	// example policy 1
+	examplePolicy1 := enmime.AllowCorruptTextPartErrorPolicy
+
+	// example policy 2: recover partial content from base64.CorruptInputError when content type is text/plain
+	examplePolicy2 := enmime.ReadPartErrorPolicy(func(p *enmime.Part, err error) bool {
+		if enmime.IsBase64CorruptInputError(err) && p.ContentType == "text/plain" {
+			return true
+		}
+		return false
+	})
+
+	// example policy 3: always recover the partial content read, no matter the error
+	examplePolicy3 := enmime.ReadPartErrorPolicy(func(p *enmime.Part, err error) bool {
+		return true
+	})
+
+	plainContent := "Hello World!"
+	htmlContent := "<p>Hello World</p>"
+
+	illegalBase64Error := enmime.Error{
+		Name:   enmime.ErrorMalformedBase64,
+		Detail: "illegal base64 data at input byte 0",
+		Severe: true,
+	}
+
+	keepingBufferWarning := enmime.Error{
+		Name:   enmime.ErrorMalformedChildPart,
+		Detail: "partial content: illegal base64 data at input byte 0",
+		Severe: false,
+	}
+
+	tests := map[string]struct {
+		policy                       enmime.ReadPartErrorPolicy
+		expectedPlainTextPartContent string
+		expectedHTMLPartContent      string
+		expectedErrorPlainTextPart   enmime.Error
+		expectedErrorHTMLPart        enmime.Error
+	}{
+		"no policy (default behavior)": {
+			policy:                       nil,
+			expectedPlainTextPartContent: "",
+			expectedErrorPlainTextPart:   illegalBase64Error,
+			expectedHTMLPartContent:      "",
+			expectedErrorHTMLPart:        illegalBase64Error,
+		},
+		"keep buffer according to policy 1": {
+			policy:                       examplePolicy1,
+			expectedPlainTextPartContent: plainContent,
+			expectedErrorPlainTextPart:   keepingBufferWarning,
+			expectedHTMLPartContent:      htmlContent,
+			expectedErrorHTMLPart:        keepingBufferWarning,
+		},
+		"keep buffer according to policy 2": {
+			policy:                       examplePolicy2,
+			expectedPlainTextPartContent: plainContent,
+			expectedErrorPlainTextPart:   keepingBufferWarning,
+			expectedHTMLPartContent:      "",
+			expectedErrorHTMLPart:        illegalBase64Error,
+		},
+		"keep buffer according to policy 3": {
+			policy:                       examplePolicy3,
+			expectedPlainTextPartContent: plainContent,
+			expectedErrorPlainTextPart:   keepingBufferWarning,
+			expectedHTMLPartContent:      htmlContent,
+			expectedErrorHTMLPart:        keepingBufferWarning,
+		},
+	}
+
+	for testName, testData := range tests {
+		t.Run(testName, func(t *testing.T) {
+			r := test.OpenTestData("parts", "extra-base64-character.raw")
+			parser := enmime.NewParser(enmime.SetReadPartErrorPolicy(testData.policy))
+			p, err := parser.ReadParts(r)
+
+			if err != nil {
+				t.Fatalf("Unexpected parse error: %+v", err)
+			}
+
+			var expectedPart *enmime.Part
+
+			// Examine root
+			expectedPart = &enmime.Part{
+				FirstChild:  test.PartExists,
+				ContentType: "multipart/alternative",
+				PartID:      "0",
+			}
+			expectedSubject := "Each part contains 1 extra base64 character (4*n + 1)"
+			gotSubject := p.Header.Get("Subject")
+
+			test.ComparePart(t, p, expectedPart)
+			test.ContentEqualsString(t, p.Content, "")
+			if gotSubject != expectedSubject {
+				t.Errorf("Subject got: %q, expected: %q", gotSubject, expectedSubject)
+			}
+
+			// Examine parts
+			expectedCharset := "utf-8"
+			expectedContentTransferEncoding := "base64"
+			var foundExpectedErr bool
+
+			// Examine first child
+			p = p.FirstChild
+			expectedPart = &enmime.Part{
+				Parent:      test.PartExists,
+				NextSibling: test.PartExists,
+				ContentType: "text/plain",
+				Charset:     expectedCharset,
+				PartID:      "1",
+			}
+			gotContentTransferEncoding := p.Header.Get("Content-Transfer-Encoding")
+
+			test.ComparePart(t, p, expectedPart)
+			test.ContentEqualsString(t, p.Content, testData.expectedPlainTextPartContent)
+			if gotContentTransferEncoding != expectedContentTransferEncoding {
+				t.Errorf("Content-Transfer-Encoding got: %q, expected: %q", gotContentTransferEncoding, expectedContentTransferEncoding)
+			}
+			foundExpectedErr = false
+			for _, v := range p.Errors {
+				if *v == testData.expectedErrorPlainTextPart {
+					foundExpectedErr = true
+					break
+				}
+			}
+			if !foundExpectedErr {
+				t.Errorf("Expected to find error: %v", testData.expectedErrorPlainTextPart)
+			}
+
+			// Examine second child
+			p = p.NextSibling
+			expectedPart = &enmime.Part{
+				Parent:      test.PartExists,
+				ContentType: "text/html",
+				Charset:     expectedCharset,
+				PartID:      "2",
+			}
+			gotContentTransferEncoding = p.Header.Get("Content-Transfer-Encoding")
+
+			test.ComparePart(t, p, expectedPart)
+			test.ContentEqualsString(t, p.Content, testData.expectedHTMLPartContent)
+			if gotContentTransferEncoding != expectedContentTransferEncoding {
+				t.Errorf("Content-Transfer-Encoding got: %q, expected: %q", gotContentTransferEncoding, expectedContentTransferEncoding)
+			}
+			foundExpectedErr = false
+			for _, v := range p.Errors {
+				if *v == testData.expectedErrorHTMLPart {
+					foundExpectedErr = true
+					break
+				}
+			}
+			if !foundExpectedErr {
+				t.Errorf("Expected to find error: %v", testData.expectedErrorHTMLPart)
+			}
+		})
+	}
+}
+
 func TestMultiNoSkipMalformedPartFails(t *testing.T) {
 	r := test.OpenTestData("parts", "multi-malformed.raw")
 	parser := enmime.NewParser(enmime.SkipMalformedParts(false))
diff --git a/testdata/parts/extra-base64-character.raw b/testdata/parts/extra-base64-character.raw
new file mode 100644
index 0000000..5f8d393
--- /dev/null
+++ b/testdata/parts/extra-base64-character.raw
@@ -0,0 +1,16 @@
+Subject: Each part contains 1 extra base64 character (4*n + 1)
+Content-Type: multipart/alternative; boundary="Enmime-Test-Base64-One-Extra-Character"
+
+--Enmime-Test-Base64-One-Extra-Character
+Content-Type: text/plain; charset="utf-8"
+Content-Transfer-Encoding: base64
+
+SGVsbG8gV29ybGQhA
+
+--Enmime-Test-Base64-One-Extra-Character
+Content-Type: text/html; charset="utf-8"
+Content-Transfer-Encoding: base64
+
+PHA+SGVsbG8gV29ybGQ8L3A+A
+
+--Enmime-Test-Base64-One-Extra-Character--
\ No newline at end of file