Repository: jhillyerd/enmime
Pull request #274 (2023-02-03 20:45:26)

parser: Add MaxStoredPartErrors option
Makes the `MaxPartErrors` global variable established in #240 available as an option.



Patch:
diff --git a/error.go b/error.go
index 64582b9..416cfda 100644
--- a/error.go
+++ b/error.go
@@ -27,7 +27,10 @@ const (
 	ErrorMalformedChildPart = "Malformed child part"
 )
 
-// MaxPartErrors limits number of part parsing errors, errors after the limit are ignored. 0 means unlimited.
+// MaxPartErrors limits number of part parsing errors, errors after the limit are ignored.
+// 0 means unlimited.
+//
+// Deprecated: This limit may be set via the `MaxStoredPartErrors` Parser option.
 var MaxPartErrors = 0
 
 // Error describes an error encountered while parsing.
@@ -71,7 +74,13 @@ func (p *Part) addWarning(name string, detailFmt string, args ...interface{}) {
 
 // addProblem adds general *Error to the Part error slice.
 func (p *Part) addProblem(err *Error) {
-	if (MaxPartErrors == 0) || (len(p.Errors) < MaxPartErrors) {
+	maxErrors := MaxPartErrors
+	if p.parser != nil && p.parser.maxStoredPartErrors != nil {
+		// Override global var.
+		maxErrors = *p.parser.maxStoredPartErrors
+	}
+
+	if (maxErrors == 0) || (len(p.Errors) < maxErrors) {
 		p.Errors = append(p.Errors, err)
 	}
 }
diff --git a/options.go b/options.go
index 573e397..9bad427 100644
--- a/options.go
+++ b/options.go
@@ -38,3 +38,16 @@ type readPartErrorPolicyOption ReadPartErrorPolicy
 func (o readPartErrorPolicyOption) apply(p *Parser) {
 	p.readPartErrorPolicy = ReadPartErrorPolicy(o)
 }
+
+// MaxStoredPartErrors limits number of part parsing errors, errors beyond the limit are discarded.
+// Zero, the default, means all errors will be kept.
+func MaxStoredPartErrors(n int) Option {
+	return maxStoredPartErrorsOption(n)
+}
+
+type maxStoredPartErrorsOption int
+
+func (o maxStoredPartErrorsOption) apply(p *Parser) {
+	max := int(o)
+	p.maxStoredPartErrors = &max
+}
diff --git a/parser.go b/parser.go
index 5013028..1bf7f6a 100644
--- a/parser.go
+++ b/parser.go
@@ -16,9 +16,10 @@ func AllowCorruptTextPartErrorPolicy(p *Part, err error) bool {
 // Parser parses MIME.
 // Default parser is a valid one.
 type Parser struct {
-	skipMalformedParts              bool
+	maxStoredPartErrors             *int // TODO: Pointer until global var removed.
 	multipartWOBoundaryAsSinglePart bool
 	readPartErrorPolicy             ReadPartErrorPolicy
+	skipMalformedParts              bool
 }
 
 // defaultParser is a Parser with default configuration.
diff --git a/part.go b/part.go
index 482842c..32ebdb7 100644
--- a/part.go
+++ b/part.go
@@ -45,6 +45,8 @@ type Part struct {
 	Errors   []*Error // Errors encountered while parsing this part.
 	Content  []byte   // Content after decoding, UTF-8 conversion if applicable.
 	Epilogue []byte   // Epilogue contains data following the closing boundary marker.
+
+	parser *Parser // Provides access to parsing options.
 }
 
 // NewPart creates a new Part object.
@@ -53,6 +55,7 @@ func NewPart(contentType string) *Part {
 		Header:            make(textproto.MIMEHeader),
 		ContentType:       contentType,
 		ContentTypeParams: make(map[string]string),
+		parser:            &defaultParser,
 	}
 }
 
@@ -372,17 +375,16 @@ func ReadParts(r io.Reader) (*Part, error) {
 // ReadParts reads a MIME document from the provided reader and parses it into tree of Part objects.
 func (p Parser) ReadParts(r io.Reader) (*Part, error) {
 	br := bufio.NewReader(r)
-	root := &Part{PartID: "0"}
+	root := &Part{PartID: "0", parser: &p}
+
 	// Read header; top-level default CT is text/plain us-ascii according to RFC 822.
-	err := root.setupHeaders(br, `text/plain; charset="us-ascii"`)
-	if err != nil {
+	if err := root.setupHeaders(br, `text/plain; charset="us-ascii"`); err != nil {
 		return nil, err
 	}
 
 	if detectMultipartMessage(root, p.multipartWOBoundaryAsSinglePart) {
 		// Content is multipart, parse it.
-		err = parseParts(root, br, p.skipMalformedParts, p.readPartErrorPolicy)
-		if err != nil {
+		if err := parseParts(root, br); err != nil {
 			return nil, err
 		}
 	} else {
@@ -395,7 +397,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, readPartErrorPolicy ReadPartErrorPolicy) error {
+func parseParts(parent *Part, reader *bufio.Reader) error {
 	firstRecursion := parent.Parent == nil
 	// Loop over MIME boundaries.
 	br := newBoundaryReader(reader, parent.Boundary)
@@ -411,28 +413,31 @@ func parseParts(parent *Part, reader *bufio.Reader, skipMalformedParts bool, rea
 		if !next {
 			break
 		}
-		p := &Part{}
+
 		// Set this Part's PartID, indicating its position within the MIME Part tree.
+		p := &Part{parser: parent.parser}
 		if firstRecursion {
 			p.PartID = strconv.Itoa(indexPartID)
 		} else {
 			p.PartID = parent.PartID + "." + strconv.Itoa(indexPartID)
 		}
+
 		// Look for part header.
 		bbr := bufio.NewReader(br)
 		if err = p.setupHeaders(bbr, ""); err != nil {
-			if skipMalformedParts {
+			if p.parser.skipMalformedParts {
 				parent.addError(ErrorMalformedChildPart, "read header: %s", err.Error())
 				continue
 			}
 
 			return err
 		}
+
 		// Insert this Part into the MIME tree.
 		if p.Boundary == "" {
 			// Content is text or data, decode it.
-			if err = p.decodeContent(bbr, readPartErrorPolicy); err != nil {
-				if skipMalformedParts {
+			if err = p.decodeContent(bbr, p.parser.readPartErrorPolicy); err != nil {
+				if p.parser.skipMalformedParts {
 					parent.addError(ErrorMalformedChildPart, "decode content: %s", err.Error())
 					continue
 				}
@@ -444,20 +449,22 @@ func parseParts(parent *Part, reader *bufio.Reader, skipMalformedParts bool, rea
 
 		parent.AddChild(p)
 		// Content is another multipart.
-		if err = parseParts(p, bbr, skipMalformedParts, readPartErrorPolicy); err != nil {
-			if skipMalformedParts {
+		if err = parseParts(p, bbr); err != nil {
+			if p.parser.skipMalformedParts {
 				parent.addError(ErrorMalformedChildPart, "parse parts: %s", err.Error())
 				continue
 			}
 			return err
 		}
 	}
+
 	// Store any content following the closing boundary marker into the epilogue.
 	epilogue, err := ioutil.ReadAll(reader)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 	parent.Epilogue = epilogue
+
 	// If a Part is "multipart/" Content-Type, it will have .0 appended to its PartID
 	// i.e. it is the root of its MIME Part subtree.
 	if !firstRecursion {


Test patch:
diff --git a/error_test.go b/error_test.go
index fb49a0a..f24c86d 100644
--- a/error_test.go
+++ b/error_test.go
@@ -4,6 +4,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestErrorStringConversion(t *testing.T) {
@@ -34,7 +36,7 @@ func TestErrorStringConversion(t *testing.T) {
 }
 
 func TestErrorAddError(t *testing.T) {
-	p := &Part{}
+	p := NewPart("text/plain")
 	p.addError(ErrorMalformedHeader, "1 %v %q", 2, "three")
 
 	if len(p.Errors) != 1 {
@@ -55,7 +57,7 @@ func TestErrorAddError(t *testing.T) {
 }
 
 func TestErrorAddWarning(t *testing.T) {
-	p := &Part{}
+	p := NewPart("text/plain")
 	p.addWarning(ErrorMalformedHeader, "1 %v %q", 2, "three")
 
 	if len(p.Errors) != 1 {
@@ -131,6 +133,7 @@ func TestErrorEnvelopeWarnings(t *testing.T) {
 	}
 }
 
+// Deprecated.
 func TestErrorLimit(t *testing.T) {
 	// Backup global variable
 	originalMaxPartErros := MaxPartErrors
@@ -139,7 +142,7 @@ func TestErrorLimit(t *testing.T) {
 	}()
 
 	addThreeErrors := func() int {
-		part := &Part{}
+		part := NewPart("text/plain")
 		part.addError("test1", "test1")
 		part.addError("test2", "test2")
 		part.addError("test3", "test3")
@@ -169,3 +172,53 @@ func TestErrorLimit(t *testing.T) {
 		t.Errorf("Expected limited errors (3), got %d", errCount)
 	}
 }
+
+func TestErrorLimitOption(t *testing.T) {
+	// Backup global variable
+	originalMaxPartErros := MaxPartErrors
+	defer func() {
+		MaxPartErrors = originalMaxPartErros
+	}()
+
+	addThreeErrors := func(parser *Parser) int {
+		part := NewPart("text/plain")
+		if parser != nil {
+			part.parser = parser
+		}
+
+		part.addError("test1", "test1")
+		part.addError("test2", "test2")
+		part.addError("test3", "test3")
+
+		return len(part.Errors)
+	}
+
+	var got, want int
+
+	// Check unlimited by default.
+	want = 3
+	got = addThreeErrors(nil)
+	assert.Equal(t, want, got, "expected unlimited errors")
+
+	// Check unlimited by default when providing Parser.
+	want = 3
+	got = addThreeErrors(NewParser())
+	assert.Equal(t, want, got, "expected unlimited errors")
+
+	// Check the default actually comes from deprecated MaxPartErrors global.
+	want = 1
+	MaxPartErrors = want
+	got = addThreeErrors(nil)
+	assert.Equal(t, want, got, "expected limited errors")
+	MaxPartErrors = 0
+
+	// Check limit.
+	want = 1
+	got = addThreeErrors(NewParser(MaxStoredPartErrors(want)))
+	assert.Equal(t, want, got, "expected limited errors")
+
+	// Check limit matching count.
+	want = 3
+	got = addThreeErrors(NewParser(MaxStoredPartErrors(want)))
+	assert.Equal(t, want, got, "expected limited errors")
+}