Merge branch 'release/0.6.0'

.travis.yml

@@ -14,3 +14,4 @@ after_success:
 
 go:
   - "1.11.x"
+  - "1.12.x"

CHANGELOG.md

@@ -4,6 +4,18 @@ Change Log
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [0.6.0] - 2019-08-10
+
+### Added
+- Make ParseMediaType public.
+
+### Fixed
+- Improve quoted display name handling (#112, thanks to requaos.)
+- Refactor MIME part boundary detection (thanks to requaos.)
+- Several improvements to MIME attribute decoding (thanks to requaos.)
+- Detect text/plain attachments properly (thanks to davrux.)
+
+
 ## [0.5.0] - 2018-12-15
 
 ### Added
@@ -84,6 +96,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Initial implementation of MIME encoding, using `enmime.MailBuilder`
 
 [Unreleased]: https://github.com/jhillyerd/enmime/compare/master...develop
+[0.6.0]:      https://github.com/jhillyerd/enmime/compare/v0.5.0...v0.6.0
 [0.5.0]:      https://github.com/jhillyerd/enmime/compare/v0.4.0...v0.5.0
 [0.4.0]:      https://github.com/jhillyerd/enmime/compare/v0.3.0...v0.4.0
 [0.3.0]:      https://github.com/jhillyerd/enmime/compare/v0.2.1...v0.3.0

boundary.go

@@ -3,8 +3,10 @@ package enmime
 import (
 	"bufio"
 	"bytes"
+	stderrors "errors"
 	"io"
 	"io/ioutil"
+	"unicode"
 
 	"github.com/pkg/errors"
 )
@@ -14,14 +16,18 @@ import (
 // from it.
 const peekBufferSize = 4096
 
+var errNoBoundaryTerminator = stderrors.New("expected boundary not present")
+
 type boundaryReader struct {
-	finished  bool          // No parts remain when finished
-	partsRead int           // Number of parts read thus far
-	r         *bufio.Reader // Source reader
-	nlPrefix  []byte        // NL + MIME boundary prefix
-	prefix    []byte        // MIME boundary prefix
-	final     []byte        // Final boundary prefix
-	buffer    *bytes.Buffer // Content waiting to be read
+	finished         bool          // No parts remain when finished
+	partsRead        int           // Number of parts read thus far
+	r                *bufio.Reader // Source reader
+	nlPrefix         []byte        // NL + MIME boundary prefix
+	prefix           []byte        // MIME boundary prefix
+	final            []byte        // Final boundary prefix
+	buffer           *bytes.Buffer // Content waiting to be read
+	crBoundaryPrefix bool          // Flag for CR in CRLF + MIME boundary
+	unbounded        bool          // Flag to throw errNoBoundaryTerminator
 }
 
 // newBoundaryReader returns an initialized boundaryReader
@@ -37,48 +43,104 @@ func newBoundaryReader(reader *bufio.Reader, boundary string) *boundaryReader {
 }
 
 // Read returns a buffer containing the content up until boundary
+//
+//   Excerpt from io package on io.Reader implementations:
+//
+//     type Reader interface {
+//        Read(p []byte) (n int, err error)
+//     }
+//
+//     Read reads up to len(p) bytes into p. It returns the number of
+//     bytes read (0 <= n <= len(p)) and any error encountered. Even
+//     if Read returns n < len(p), it may use all of p as scratch space
+//     during the call. If some data is available but not len(p) bytes,
+//     Read conventionally returns what is available instead of waiting
+//     for more.
+//
+//     When Read encounters an error or end-of-file condition after
+//     successfully reading n > 0 bytes, it returns the number of bytes
+//     read. It may return the (non-nil) error from the same call or
+//     return the error (and n == 0) from a subsequent call. An instance
+//     of this general case is that a Reader returning a non-zero number
+//     of bytes at the end of the input stream may return either err == EOF
+//     or err == nil. The next Read should return 0, EOF.
+//
+//     Callers should always process the n > 0 bytes returned before
+//     considering the error err. Doing so correctly handles I/O errors
+//     that happen after reading some bytes and also both of the allowed
+//     EOF behaviors.
 func (b *boundaryReader) Read(dest []byte) (n int, err error) {
 	if b.buffer.Len() >= len(dest) {
-		// This read request can be satisfied entirely by the buffer
+		// This read request can be satisfied entirely by the buffer.
 		return b.buffer.Read(dest)
 	}
 
-	peek, err := b.r.Peek(peekBufferSize)
-	peekEOF := (err == io.EOF)
-	if err != nil && !peekEOF && err != bufio.ErrBufferFull {
-		// Unexpected error
-		return 0, errors.WithStack(err)
-	}
-	var nCopy int
-	idx, complete := locateBoundary(peek, b.nlPrefix)
-	if idx != -1 {
-		// Peeked boundary prefix, read until that point
-		nCopy = idx
-		if !complete && nCopy == 0 {
-			// Incomplete boundary, move past it
-			nCopy = 1
+	for i := 0; i < cap(dest); i++ {
+		cs, err := b.r.Peek(1)
+		if err != nil && err != io.EOF {
+			return 0, errors.WithStack(err)
 		}
-	} else {
-		// No boundary found, move forward a safe distance
-		if nCopy = len(peek) - len(b.nlPrefix) - 1; nCopy <= 0 {
-			nCopy = 0
-			if peekEOF {
-				// No more peek space remaining and no boundary found
-				return 0, errors.WithStack(io.ErrUnexpectedEOF)
+		// Ensure that we can switch on the first byte of 'cs' without panic.
+		if len(cs) > 0 {
+			padding := 1
+			check := false
+
+			switch cs[0] {
+			// Check for carriage return as potential CRLF boundary prefix.
+			case '\r':
+				padding = 2
+				check = true
+			// Check for line feed as potential LF boundary prefix.
+			case '\n':
+				check = true
+			}
+
+			if check {
+				peek, err := b.r.Peek(len(b.nlPrefix) + padding + 1)
+				switch err {
+				case nil:
+					// Check the whitespace at the head of the peek to avoid checking for a boundary early.
+					if bytes.HasPrefix(peek, []byte("\n\n")) ||
+						bytes.HasPrefix(peek, []byte("\n\r")) ||
+						bytes.HasPrefix(peek, []byte("\r\n\r")) ||
+						bytes.HasPrefix(peek, []byte("\r\n\n")) {
+						break
+					}
+					// Check the peek buffer for a boundary delimiter or terminator.
+					if b.isDelimiter(peek[padding:]) || b.isTerminator(peek[padding:]) {
+						// We have found our boundary terminator, lets write out the final bytes
+						// and return io.EOF to indicate that this section read is complete.
+						n, err = b.buffer.Read(dest)
+						switch err {
+						case nil, io.EOF:
+							return n, io.EOF
+						default:
+							return 0, errors.WithStack(err)
+						}
+					}
+				case io.EOF:
+					// We have reached the end without finding a boundary,
+					// so we flag the boundary reader to add an error to
+					// the errors slice and write what we have to the buffer.
+					b.unbounded = true
+				default:
+					continue
+				}
 			}
 		}
-	}
-	if nCopy > 0 {
-		if _, err = io.CopyN(b.buffer, b.r, int64(nCopy)); err != nil {
-			return 0, errors.WithStack(err)
+
+		_, err = io.CopyN(b.buffer, b.r, 1)
+		if err != nil {
+			// EOF is not fatal, it just means that we have drained the reader.
+			if errors.Cause(err) == io.EOF {
+				break
+			}
+			return 0, err
 		}
 	}
 
+	// Read the contents of the buffer into the destination slice.
 	n, err = b.buffer.Read(dest)
-	if err == io.EOF && !complete {
-		// Only the buffer is empty, not the boundaryReader
-		return n, nil
-	}
 	return n, err
 }
 
@@ -88,7 +150,7 @@ func (b *boundaryReader) Next() (bool, error) {
 		return false, nil
 	}
 	if b.partsRead > 0 {
-		// Exhaust the current part to prevent errors when moving to the next part
+		// Exhaust the current part to prevent errors when moving to the next part.
 		_, _ = io.Copy(ioutil.Discard, b)
 	}
 	for {
@@ -105,21 +167,21 @@ func (b *boundaryReader) Next() (bool, error) {
 			return false, nil
 		}
 		if err != io.EOF && b.isDelimiter(line) {
-			// Start of a new part
+			// Start of a new part.
 			b.partsRead++
 			return true, nil
 		}
 		if err == io.EOF {
-			// Intentionally not wrapping with stack
+			// Intentionally not wrapping with stack.
 			return false, io.EOF
 		}
 		if b.partsRead == 0 {
 			// The first part didn't find the starting delimiter, burn off any preamble in front of
-			// the boundary
+			// the boundary.
 			continue
 		}
 		b.finished = true
-		return false, errors.Errorf("expecting boundary %q, got %q", string(b.prefix), string(line))
+		return false, errors.WithMessagef(errNoBoundaryTerminator, "expecting boundary %q, got %q", string(b.prefix), string(line))
 	}
 }
 
@@ -130,11 +192,10 @@ func (b *boundaryReader) isDelimiter(buf []byte) bool {
 		return false
 	}
 
-	// Fast forward to the end of the boundary prefix
+	// Fast forward to the end of the boundary prefix.
 	buf = buf[idx+len(b.prefix):]
-	buf = bytes.TrimLeft(buf, " \t")
 	if len(buf) > 0 {
-		if buf[0] == '\r' || buf[0] == '\n' {
+		if unicode.IsSpace(rune(buf[0])) {
 			return true
 		}
 	}
@@ -147,42 +208,3 @@ func (b *boundaryReader) isTerminator(buf []byte) bool {
 	idx := bytes.Index(buf, b.final)
 	return idx != -1
 }
-
-// Locate boundaryPrefix in buf, returning its starting idx. If complete is true, the boundary
-// is terminated properly in buf, otherwise it could be false due to running out of buffer, or
-// because it is not the actual boundary.
-//
-// Complete boundaries end in "--" or a newline
-func locateBoundary(buf, boundaryPrefix []byte) (idx int, complete bool) {
-	bpLen := len(boundaryPrefix)
-	idx = bytes.Index(buf, boundaryPrefix)
-	if idx == -1 {
-		return
-	}
-
-	// Handle CR if present
-	if idx > 0 && buf[idx-1] == '\r' {
-		idx--
-		bpLen++
-	}
-
-	// Fast forward to the end of the boundary prefix
-	buf = buf[idx+bpLen:]
-	if len(buf) == 0 {
-		// Need more bytes to verify completeness
-		return
-	}
-	if len(buf) > 1 {
-		if buf[0] == '-' && buf[1] == '-' {
-			return idx, true
-		}
-	}
-	buf = bytes.TrimLeft(buf, " \t")
-	if len(buf) > 0 {
-		if buf[0] == '\r' || buf[0] == '\n' {
-			return idx, true
-		}
-	}
-
-	return
-}

boundary_test.go

@@ -18,6 +18,11 @@ func TestBoundaryReader(t *testing.T) {
 			boundary: "STOPHERE",
 			want:     "good",
 		},
+		{
+			input:    "good\r\n--STOPHERE\t\r\nafter",
+			boundary: "STOPHERE",
+			want:     "good",
+		},
 		{
 			input:    "good\r\n--STOPHERE--\r\nafter",
 			boundary: "STOPHERE",
@@ -290,8 +295,8 @@ func TestBoundaryReaderNoTerminator(t *testing.T) {
 		t.Fatal("Next() = false, want: true")
 	}
 
-	// Second part should error
-	want := "expecting boundary"
+	// There is no second part should, error should be EOF.
+	want := "EOF"
 	next, err = br.Next()
 	if err == nil {
 		t.Fatal("Error was nil, wanted:", want)

cmd/example_test.go

@@ -24,6 +24,34 @@ Text section.
 Content-Type: text/html
 
 <em>HTML</em> section.
+--Enmime-Test-100
+Content-Transfer-Encoding: base64
+Content-Disposition: inline;
+	filename=favicon.png
+Content-Type: image/png;
+	x-unix-mode=0644;
+	name="favicon.png"
+Content-Id: <8B8481A2-25CA-4886-9B5A-8EB9115DD064@skynet>
+
+iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJ
+bWFnZVJlYWR5ccllPAAAAlFJREFUeNqUU8tOFEEUPVVdNV3dPe8xYRBnjGhmBgKjKzCIiQvBoIaN
+bly5Z+PSv3Aj7DSiP2B0rwkLGVdGgxITSCRIJGSMEQWZR3eVt5sEFBgTb/dN1yvnnHtPNTPG4Pqd
+HgCMXnPRSZrpSuH8vUJu4DE4rYHDGAZDX62BZttHqTiIayM3gGiXQsgYLEvATaqxU+dy1U13YXap
+XptpNHY8iwn8KyIAzm1KBdtRZWErpI5lEWTXp5Z/vHpZ3/wyKKwYGGOdAYwR0EZwoezTYApBEIOb
+yELl/aE1/83cp40Pt5mxqCKrE4Ck+mVWKKcI5tA8BLEhRBKJLjez6a7MLq7XZtp+yyOawwCBtkiB
+VZDKzRk4NN7NQBMYPHiZDFhXY+p9ff7F961vVcnl4R5I2ykJ5XFN7Ab7Gc61VoipNBKF+PDyztu5
+lfrSLT/wIwCxq0CAGtXHZTzqR2jtwQiXONma6hHpj9sLT7YaPxfTXuZdBGA02Wi7FS48YiTfj+i2
+NhqtdhP5RC8mh2/Op7y0v6eAcWVLFT8D7kWX5S9mepp+C450MV6aWL1cGnvkxbwHtLW2B9AOkLeU
+d9KEDuh9fl/7CEj7YH5g+3r/lWfF9In7tPz6T4IIwBJOr1SJyIGQMZQbsh5P9uBq5VJtqHh2mo49
+pdw5WFoEwKWqWHacaWOjQXWGcifKo6vj5RGS6zykI587XeUIQDqJSmAp+lE4qt19W5P9o8+Lma5D
+cjsC8JiT607lMVkdqQ0Vyh3lHhmh52tfNy78ajXv0rgYzv8nfwswANuk+7sD/Q0aAAAAAElFTkSu
+QmCC
+--Enmime-Test-100
+Content-Transfer-Encoding: base64
+Content-Type: text/html; name="test.html"
+Content-Disposition: attachment; filename=test.html
+
+PGh0bWw+Cg==
 --Enmime-Test-100--
 `
 	// Convert MIME text to Envelope
@@ -66,14 +94,19 @@ Content-Type: text/html
 	// <em>HTML</em> section.
 	//
 	// ## Attachment List
+	// - test.html (text/html)
 	//
 	// ## Inline List
+	// - favicon.png (image/png)
+	//   Content-ID: 8B8481A2-25CA-4886-9B5A-8EB9115DD064@skynet
 	//
 	// ## Other Part List
 	//
 	// ## MIME Part Tree
 	//     multipart/mixed
 	//     |-- text/plain
-	//     `-- text/html
+	//     |-- text/html
+	//     |-- image/png, disposition: inline, filename: "favicon.png"
+	//     `-- text/html, disposition: attachment, filename: "test.html"
 	//
 }