From 58dc4fa89fa96a7fd772b4ed085ce238ac8a8e63 Mon Sep 17 00:00:00 2001 From: Bill Gedney Date: Tue, 7 Apr 2026 13:33:40 -0700 Subject: [PATCH 1/3] Refactor EnvelopeFromPart()'s scope so that the functions's scope of work is more aligned to the root aspect of a provided part. Also extracted the previous functionality into a new method on the Envelope type - GatherNestedErrors(). Also implemented GatherNestedErrors() within ReadEnvelope() after the call to EnvelopeFromPart() and added supporting test cases. --- envelope.go | 22 ++++++++++++++++--- envelope_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/envelope.go b/envelope.go index 9bf720f..bd19496 100644 --- a/envelope.go +++ b/envelope.go @@ -159,7 +159,18 @@ func (p Parser) ReadEnvelope(r io.Reader) (*Envelope, error) { if err != nil { return nil, errors.WithMessage(err, "Failed to ReadParts") } - return p.EnvelopeFromPart(root) + + e, err := p.EnvelopeFromPart(root) + if err != nil { + return nil, errors.WithMessage(err, "Failed to EnvelopeFromPart") + } + + err = e.GatherNestedErrors() + if err != nil { + return nil, errors.WithMessage(err, "Failed to GatherNestedErrors") + } + + return e, nil } // EnvelopeFromPart uses the provided Part tree to build an Envelope, downconverting HTML to plain @@ -210,7 +221,12 @@ func (p Parser) EnvelopeFromPart(root *Part) (*Envelope, error) { } } - // Copy part errors into Envelope. + return e, nil +} + +// GatherNestedErrors gathers errors from the entire part tree (including the root) and adds them to the Envelope. +func (e *Envelope) GatherNestedErrors() error { + // Copy part errors from all nested/child/sibling parts into Envelope. if e.Root != nil { _ = e.Root.DepthMatchAll(func(part *Part) bool { // Using DepthMatchAll to traverse all parts, don't care about result. @@ -219,7 +235,7 @@ func (p Parser) EnvelopeFromPart(root *Part) (*Envelope, error) { }) } - return e, nil + return nil } // parseTextOnlyBody parses a plain text message in root that has MIME-like headers, but diff --git a/envelope_test.go b/envelope_test.go index a8eb9c2..b659772 100644 --- a/envelope_test.go +++ b/envelope_test.go @@ -61,6 +61,7 @@ func TestParseNonMime(t *testing.T) { func TestParseNonMimeHTML(t *testing.T) { msg := test.OpenTestData("mail", "non-mime-html.raw") + e, err := enmime.ReadEnvelope(msg) if err != nil { @@ -73,6 +74,11 @@ func TestParseNonMimeHTML(t *testing.T) { t.Errorf("e.Errors[0] got: %v, want: %v", got, want) } } else { + for _, err = range e.Errors { + fmt.Println(err.Error()) + fmt.Println(enmime.ErrorPlainTextFromHTML) + + } t.Errorf("len(e.Errors) got: %v, want: 1", len(e.Errors)) } @@ -87,6 +93,57 @@ func TestParseNonMimeHTML(t *testing.T) { } } +// TestEnvelopeFromPartDoesNotGatherErrors verifies that EnvelopeFromPart does not collect errors, +// while GatherNestedErrors does. This enforces the separation of concerns: EnvelopeFromPart builds +// the envelope structure, and GatherNestedErrors is responsible for error collection. +func TestEnvelopeFromPartDoesNotGatherErrors(t *testing.T) { + // Use a message that generates errors (HTML without plain text) + msg := test.OpenTestData("mail", "non-mime-html.raw") + parser := enmime.NewParser() + root, err := parser.ReadParts(msg) + if err != nil { + t.Fatalf("Failed to read parts: %v", err) + } + + // Call EnvelopeFromPart directly (not ReadEnvelope) + e, err := parser.EnvelopeFromPart(root) + if err != nil { + t.Fatalf("Failed to create envelope from part: %v", err) + } + + // EnvelopeFromPart should NOT have collected errors + if len(e.Errors) != 0 { + t.Errorf("EnvelopeFromPart should not gather errors, but got %d errors:", len(e.Errors)) + for _, err := range e.Errors { + t.Logf(" - %v", err) + } + } + + // Verify that the envelope structure was built correctly + if e.HTML == "" { + t.Error("Expected HTML content from envelope structure building") + } + + // Now call GatherNestedErrors + err = e.GatherNestedErrors() + if err != nil { + t.Fatalf("GatherNestedErrors failed: %v", err) + } + + // GatherNestedErrors SHOULD have collected the error + if len(e.Errors) != 1 { + t.Errorf("GatherNestedErrors should collect exactly 1 error, got %d", len(e.Errors)) + } else if e.Errors[0].Name != enmime.ErrorPlainTextFromHTML { + t.Errorf("Expected error %q, got %q", enmime.ErrorPlainTextFromHTML, e.Errors[0].Name) + } + + // Verify text was downconverted from HTML + want := "This is *a* *test* mailing" + if !strings.Contains(e.Text, want) { + t.Errorf("Expected %q to contain %q", e.Text, want) + } +} + func TestParseMimeTree(t *testing.T) { msg := test.OpenTestData("mail", "attachment.raw") e, err := enmime.ReadEnvelope(msg) From 56e636236f07316ccaac3ca3a5b3991d215c1a5a Mon Sep 17 00:00:00 2001 From: Bill Gedney Date: Tue, 7 Apr 2026 14:02:10 -0700 Subject: [PATCH 2/3] Repair linting issue. --- envelope_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/envelope_test.go b/envelope_test.go index b659772..b666250 100644 --- a/envelope_test.go +++ b/envelope_test.go @@ -74,11 +74,6 @@ func TestParseNonMimeHTML(t *testing.T) { t.Errorf("e.Errors[0] got: %v, want: %v", got, want) } } else { - for _, err = range e.Errors { - fmt.Println(err.Error()) - fmt.Println(enmime.ErrorPlainTextFromHTML) - - } t.Errorf("len(e.Errors) got: %v, want: 1", len(e.Errors)) } From 03c601c784f4ff110a03cc0eb8302162c0d1b233 Mon Sep 17 00:00:00 2001 From: Bill Gedney Date: Thu, 23 Apr 2026 15:08:25 -0700 Subject: [PATCH 3/3] Update comments on EnvelopeFromPart. --- envelope.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envelope.go b/envelope.go index bd19496..9e0d2f8 100644 --- a/envelope.go +++ b/envelope.go @@ -175,7 +175,7 @@ func (p Parser) ReadEnvelope(r io.Reader) (*Envelope, error) { // EnvelopeFromPart uses the provided Part tree to build an Envelope, downconverting HTML to plain // text if needed, and sorting the attachments, inlines and other parts into their respective -// slices. Errors are collected from all Parts and placed into the Envelopes Errors slice. +// slices. Gather errors from all parts with Envelope.GatherNestedErrors(). func EnvelopeFromPart(root *Part) (*Envelope, error) { return defaultParser.EnvelopeFromPart(root) }