Repository: emersion/go-imap
Pull request #394 (2020-09-22 21:05:42)

add support for empty groups in address lists
Currently when an email contains `To: undisclosed-recipients:;` in the header, `go-imap` parses this as one address with just a mailbox field, and one with all `nill` fields. Calling `.Address()` on them creates an array with `<undisclosed-recipients@>, <@>`.

The reason is that `go-imap` does not support empty groups, see e.g. :

```
Appendix A.1.3.  Group Addresses

   ----
   From: Pete <pete@silly.example>
   To: A Group:Ed Jones <c@a.test>,joe@where.test,John <jdoe@one.test>;
   Cc: Undisclosed recipients:;
   Date: Thu, 13 Feb 1969 23:32:54 -0330
   Message-ID: <testabcd.1234@silly.example>

   Testing.
   ----

   In this message, the "To:" field has a single group recipient named
   "A Group", which contains 3 addresses, and a "Cc:" field with an
   empty group recipient named Undisclosed recipients.
``` 

in https://www.ietf.org/rfc/rfc5322.txt

The proposed solution is to check if an address has a valid mailbox and host name, otherwise return an error and avoid adding the address to the list (instead of ignoring it like now).

Alternatively, the group could be parsed properly, but I have no idea where to start for that (it seems that the server directly returns it in fields, so my solution seems like the first check on content so it's appropriate?).

y0ast: No problem, I've just added tests for the two cases.
y0ast: @emersion I was wondering if you had time to take a look at the above? This problem comes up in `aerc` and deals with a case that's described in the RFC but not handled in go-imap.

Patch:
diff --git a/message.go b/message.go
index 3d762c7..c9beb82 100644
--- a/message.go
+++ b/message.go
@@ -674,12 +674,18 @@ func (addr *Address) Parse(fields []interface{}) error {
 	if s, err := ParseString(fields[1]); err == nil {
 		addr.AtDomainList, _ = decodeHeader(s)
 	}
-	if s, err := ParseString(fields[2]); err == nil {
-		addr.MailboxName, _ = decodeHeader(s)
+
+	s, err := ParseString(fields[2])
+	if err != nil {
+		return errors.New("Mailbox name could not be parsed")
 	}
-	if s, err := ParseString(fields[3]); err == nil {
-		addr.HostName, _ = decodeHeader(s)
+	addr.MailboxName, _ = decodeHeader(s)
+
+	s, err = ParseString(fields[3])
+	if err != nil {
+		return errors.New("Host name could not be parsed")
 	}
+	addr.HostName, _ = decodeHeader(s)
 
 	return nil
 }
@@ -706,13 +712,11 @@ func (addr *Address) Format() []interface{} {
 
 // Parse an address list from fields.
 func ParseAddressList(fields []interface{}) (addrs []*Address) {
-	addrs = make([]*Address, len(fields))
-
-	for i, f := range fields {
+	for _, f := range fields {
 		if addrFields, ok := f.([]interface{}); ok {
 			addr := &Address{}
 			if err := addr.Parse(addrFields); err == nil {
-				addrs[i] = addr
+				addrs = append(addrs, addr)
 			}
 		}
 	}


Test patch:
diff --git a/message_test.go b/message_test.go
index aeae249..e81a3b0 100644
--- a/message_test.go
+++ b/message_test.go
@@ -372,6 +372,24 @@ func TestAddress_Format(t *testing.T) {
 	}
 }
 
+func TestEmptyAddress(t *testing.T) {
+	fields := []interface{}{nil, nil, nil, nil}
+	addr := &Address{}
+	err := addr.Parse(fields)
+	if err == nil {
+		t.Error("A nil address did not return an error")
+	}
+}
+
+func TestEmptyGroupAddress(t *testing.T) {
+	fields := []interface{}{nil, nil, "undisclosed-recipients", nil}
+	addr := &Address{}
+	err := addr.Parse(fields)
+	if err == nil {
+		t.Error("An empty group did not return an error when parsed as address")
+	}
+}
+
 func TestAddressList(t *testing.T) {
 	fields := make([]interface{}, len(addrTests))
 	addrs := make([]*Address, len(addrTests))
@@ -400,6 +418,7 @@ func TestEmptyAddressList(t *testing.T) {
 	}
 }
 
+
 var paramsListTest = []struct {
 	fields []interface{}
 	params map[string]string