worker/lib/parse: be more tolerant with parsing email addresses v1 PROPOSED

Timmy Douglas: 1
 worker/lib/parse: be more tolerant with parsing email addresses

 1 files changed, 8 insertions(+), 21 deletions(-)

I was talking about aerc's parseAddressList function in
worker/lib/parse.go, which calls Header.AddressList from emersion's
go-message library, which in turn calls AddressParser.ParseList from the
standard library.

Both the standard library and emersion's go-message are pretty strict in
their adherence to the RFC 5322. Aerc's parseAddressList function is
meant to attempt parsing with go-message, but if an unparsable address
list is found for whatever reason, it should still return something
useful for the UI.

So I think the solution is 2-fold:

1) update aerc's parseAddressList to fix the err == nil check and remove
   strings.Contains condition, and

Timmy Douglas

11 months ago

Created a v2 patch for this.

Next

2) see if go would accept an upstream patch to its parseAddressList
   function which ignores an empty address.

Timmy Douglas

11 months ago

https://github.com/golang/go/pull/36966

[PATCH] worker/lib/parse: be more tolerant with parsing email addresses Export this patch

This is a hack I needed to do to get aerc to open up my email folder,
otherwise it would display [..] and I could not select or open
email. Rather than using less/mv to read my maildir, I thought I'd
make a small change to make aerc a little more tolerant.

I'm sort of creating this to have a discussion rather than to say this
would be the right way to actually fix it. It looked like the parsing
code was in another library and I didn't review it.

The email at fault had a CC line like:
"Cc: , emacs-devel@gnu.org"
---
 worker/lib/parse.go | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/worker/lib/parse.go b/worker/lib/parse.go
index 288dade..ff5bace 100644
--- a/worker/lib/parse.go
+++ b/worker/lib/parse.go
@@ -101,26 +101,11 @@ func parseEnvelope(h *mail.Header) (*models.Envelope, error) {
 	if err != nil {
 		return nil, fmt.Errorf("could not parse date header: %v", err)
 	}
-	from, err := parseAddressList(h, "from")
-	if err != nil {
-		return nil, fmt.Errorf("could not read from address: %v", err)
-	}
-	to, err := parseAddressList(h, "to")
-	if err != nil {
-		return nil, fmt.Errorf("could not read to address: %v", err)
-	}
-	cc, err := parseAddressList(h, "cc")
-	if err != nil {
-		return nil, fmt.Errorf("could not read cc address: %v", err)
-	}
-	bcc, err := parseAddressList(h, "bcc")
-	if err != nil {
-		return nil, fmt.Errorf("could not read bcc address: %v", err)
-	}
-	replyTo, err := parseAddressList(h, "reply-to")
-	if err != nil {
-		return nil, fmt.Errorf("could not read reply-to address: %v", err)
-	}
+	from, _ := parseAddressList(h, "from")
+	to, _ := parseAddressList(h, "to")
+	cc, _ := parseAddressList(h, "cc")
+	bcc, _ := parseAddressList(h, "bcc")
+	replyTo, _ := parseAddressList(h, "reply-to")
 	subj, err := h.Subject()
 	if err != nil {
 		return nil, fmt.Errorf("could not read subject: %v", err)
@@ -150,7 +135,9 @@ func parseAddressList(h *mail.Header, key string) ([]*models.Address, error) {
 				Name: hdr,
 			}}, nil
 		}
-		return nil, err
+		return []*models.Address{&models.Address{
+			Name: err.Error(),
+		}}, err
 	}
 	for _, addr := range addrs {
 		parts := strings.Split(addr.Address, "@")
-- 
2.20.1

Simon Ser

11 months ago

I think this is already supposed to be handled by parseAddressList.
The condition is inverted on line 148:

    if hdr, err := h.Text(key); err != nil && strings.Contains(hdr, "@") {

Should read like this:

    if hdr, err := h.Text(key); err == nil && strings.Contains(hdr, "@") {

Drew DeVault

11 months ago

We need to send these fixes to Go upstream imo.