Repository: jhillyerd/enmime (go)

Issue:
ReadEnvelope fails for large string in a single line.
What I did:
Run TestParseMimeTree with the file
https://www.mediafire.com/file/27w6hosijzd85pi/Get_started_with_OneDrive.eml/file
.

What I expected:
no error and HTML, text, attachments of email.

What I got:
envelope_test.go:87: Failed to parse MIME: Failed to ReadParts: multiple Read calls return no data or error

Release or branch I am using:
master branch. commit no: 6b038639b93ca41ddebf246745595190bd22d491

(Please attach a sample message if you feel it will help reproduce the issue)
Check parseParts method. The buffer is getting full. If we shorten or delete line#152 of Get_started_with_OneDrive.eml. Everything works.

Thanks

Pull request:
Resolve QPCleaner issues
- QPCleaner: support tiny destination buffers
- QPCleaner: break long lines

Fixes #185


Patch:
diff --git a/internal/coding/quotedprint.go b/internal/coding/quotedprint.go
index 5bbcb67..da37357 100644
--- a/internal/coding/quotedprint.go
+++ b/internal/coding/quotedprint.go
@@ -9,33 +9,88 @@ import (
 // QPCleaner scans quoted printable content for invalid characters and encodes them so that
 // Go's quoted-printable decoder does not abort with an error.
 type QPCleaner struct {
-	in *bufio.Reader
+	in       *bufio.Reader
+	overflow []byte
+	lineLen  int
 }
 
-// Assert QPCleaner implements io.Reader.
-var _ io.Reader = &QPCleaner{}
+// MaxQPLineLen is the maximum line length we allow before inserting `=\r\n`.  Prevents buffer
+// overflows in mime/quotedprintable.Reader.
+const MaxQPLineLen = 1024
+
+var (
+	_ io.Reader = &QPCleaner{} // Assert QPCleaner implements io.Reader.
+
+	escapedEquals = []byte("=3D") // QP encoded value of an equals sign.
+	lineBreak     = []byte("=\r\n")
+)
 
 // NewQPCleaner returns a QPCleaner for the specified reader.
 func NewQPCleaner(r io.Reader) *QPCleaner {
 	return &QPCleaner{
-		in: bufio.NewReader(r),
+		in:       bufio.NewReader(r),
+		overflow: nil,
+		lineLen:  0,
 	}
 }
 
 // Read method for io.Reader interface.
 func (qp *QPCleaner) Read(dest []byte) (n int, err error) {
-	// Ensure room to write a byte or =XX string.
-	destLen := len(dest) - 3
-	// Loop over bytes in qp.in ByteReader.
+	destLen := len(dest)
+
+	if len(qp.overflow) > 0 {
+		// Copy bytes that didn't fit into dest buffer during previous read.
+		n = copy(dest, qp.overflow)
+		qp.overflow = qp.overflow[n:]
+	}
+
+	// writeByte outputs a single byte, space for which will have already been ensured by the loop
+	// condition. Updates counters.
+	writeByte := func(in byte) {
+		dest[n] = in
+		n++
+		qp.lineLen++
+	}
+
+	// writeBytes outputs multiple bytes, storing overflow for next read. Updates counters.
+	writeBytes := func(in []byte) {
+		nc := copy(dest[n:], in)
+		if nc < len(in) {
+			// Stash unwritten bytes into overflow.
+			qp.overflow = append(qp.overflow, []byte(in[nc:])...)
+		}
+		n += nc
+		qp.lineLen += len(in)
+	}
+
+	// ensureLineLen ensures there is room to write `requested` bytes, preventing a line break being
+	// inserted in the middle of the escaped string.  The requested count is in addition to the
+	// byte that was already reserved for this loop iteration.
+	ensureLineLen := func(requested int) {
+		if qp.lineLen+requested >= MaxQPLineLen {
+			writeBytes(lineBreak)
+			qp.lineLen = 0
+		}
+	}
+
+	// Loop over bytes in qp.in ByteReader while there is space in dest.
 	for n < destLen {
 		var b byte
 		b, err = qp.in.ReadByte()
 		if err != nil {
 			return n, err
 		}
+
+		if qp.lineLen >= MaxQPLineLen {
+			writeBytes(lineBreak)
+			qp.lineLen = 0
+		}
+
 		switch {
+		// Pass valid hex bytes through, otherwise escapes the equals symbol.
 		case b == '=':
-			// Pass valid hex bytes through.
+			ensureLineLen(2)
+
 			var hexBytes []byte
 			hexBytes, err = qp.in.Peek(2)
 			if err != nil && err != io.EOF {
@@ -45,23 +100,29 @@ func (qp *QPCleaner) Read(dest []byte) (n int, err error) {
 				dest[n] = b
 				n++
 			} else {
-				s := fmt.Sprintf("=%02X", b)
-				n += copy(dest[n:], s)
+				writeBytes(escapedEquals)
 			}
-		case b == '\t' || b == '\r' || b == '\n':
-			// Valid special character.
-			dest[n] = b
-			n++
+
+		// Valid special character.
+		case b == '\t':
+			writeByte(b)
+
+		// Valid special characters that reset line length.
+		case b == '\r' || b == '\n':
+			writeByte(b)
+			qp.lineLen = 0
+
+		// Invalid characters, render as quoted-printable.
 		case b < ' ' || '~' < b:
-			// Invalid character, render quoted-printable into buffer.
-			s := fmt.Sprintf("=%02X", b)
-			n += copy(dest[n:], s)
+			ensureLineLen(2)
+			writeBytes([]byte(fmt.Sprintf("=%02X", b)))
+
+		// Acceptable characters.
 		default:
-			// Acceptable character.
-			dest[n] = b
-			n++
+			writeByte(b)
 		}
 	}
+
 	return n, err
 }
 
@@ -69,6 +130,8 @@ func validHexByte(b byte) bool {
 	return '0' <= b && b <= '9' || 'A' <= b && b <= 'F' || 'a' <= b && b <= 'f'
 }
 
+// validHexBytes returns true if this byte sequence represents a valid quoted-printable escape
+// sequence or line break, minus the initial equals sign.
 func validHexBytes(v []byte) bool {
 	if len(v) > 0 && v[0] == '\n' {
 		// Soft line break.


Test patch:
diff --git a/internal/coding/quotedprint_test.go b/internal/coding/quotedprint_test.go
index f465873..ed2f091 100644
--- a/internal/coding/quotedprint_test.go
+++ b/internal/coding/quotedprint_test.go
@@ -3,7 +3,9 @@ package coding_test
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
+	"io/ioutil"
 	"strings"
 	"testing"
 
@@ -42,7 +44,7 @@ func TestQPCleaner(t *testing.T) {
 	}
 }
 
-// TestQPCleanerOverflow attempts to confuse the cleaner by issuing a smaller subsequent read
+// TestQPCleanerOverflow attempts to confuse the cleaner by issuing smaller subsequent reads.
 func TestQPCleanerOverflow(t *testing.T) {
 	input := bytes.Repeat([]byte("pédagogues =\r\n"), 1000)
 	want := bytes.Repeat([]byte("p=C3=A9dagogues =\r\n"), 1000)
@@ -57,7 +59,7 @@ func TestQPCleanerOverflow(t *testing.T) {
 			t.Fatal(err)
 		}
 		if n < 1 {
-			t.Fatalf("Read(p) = %v, wanted >0", n)
+			t.Fatalf("Read(p) = %v, wanted >0, at want[%v]", n, offset)
 		}
 		for i := 0; i < n; i++ {
 			if p[i] != want[offset] {
@@ -68,6 +70,73 @@ func TestQPCleanerOverflow(t *testing.T) {
 	}
 }
 
+// TestQPCleanerSmallDest repeatedly calls Read with a small destination buffer.
+func TestQPCleanerSmallDest(t *testing.T) {
+	input := bytes.Repeat([]byte("pédagogues =z =\r\n"), 100)
+	want := bytes.Repeat([]byte("p=C3=A9dagogues =3Dz =\r\n"), 100)
+
+	for bufSize := 5; bufSize > 0; bufSize-- {
+		t.Run(fmt.Sprintf("%v byte buffer", bufSize), func(t *testing.T) {
+			inbuf := bytes.NewBuffer(input)
+			qp := coding.NewQPCleaner(inbuf)
+
+			offset := 0
+			p := make([]byte, bufSize)
+			for {
+				n, err := qp.Read(p)
+				if err != nil && err != io.EOF {
+					t.Fatal(err)
+				}
+				if n < 1 && offset < len(want) {
+					t.Fatalf("Read(p) = %v, wanted >0, at want[%v]", n, offset)
+				}
+				for i := 0; i < n; i++ {
+					if p[i] != want[offset] {
+						t.Errorf("p[%v] = %q, want: %q (want[%v])", i, p[i], want[offset], offset)
+					}
+					offset++
+				}
+				if err == io.EOF {
+					break
+				}
+			}
+		})
+	}
+}
+
+// TestQPCleanerLineBreak verifies QPCleaner breaks long lines correctly.
+func TestQPCleanerLineBreak(t *testing.T) {
+	input := bytes.Repeat([]byte("pédagogues =z "), 10000)
+	inbuf := bytes.NewBuffer(input)
+	qp := coding.NewQPCleaner(inbuf)
+
+	output, err := ioutil.ReadAll(qp)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := 1024 // Desired wrapping point.
+	tolerance := 3
+
+	if len(output) < want {
+		t.Fatalf("wanted minimum output len %v, got %v", want, len(output))
+	}
+
+	// Examine each line of output long enough to wrap.
+	for i := 0; len(output) > want; i++ {
+		got := bytes.Index(output, []byte("=\r\n"))
+		// Wrapping a few characters early is OK, but not late.
+		if got > want || want-got > tolerance {
+			t.Errorf("iteration %v: got line break at %v, wanted %v +/- %v",
+				i, got, want, tolerance)
+		}
+		if got == 0 {
+			break
+		}
+		output = output[got+3:] // Extend past =\r\n
+	}
+}
+
 var ErrPeek = errors.New("enmime test peek error")
 
 type peekBreakReader string