fix: Properly handle optional optional Group List in rfc5322 parser

We encountered an issue where incorrectly parsed headers (see unit test)
were being parsed as empty group sequences. This was mainly due us not
checking if there was something that required parsing after the initial
display name for the group.

rfc5322/address.go

@@ -166,42 +166,53 @@ func parseGroup(p *Parser) ([]*mail.Address, bool, error) {
 			}
 		}
 
-		// Mailbox
-		var parsedFirstMailbox bool
-
-		{
-			parserState := p.SaveState()
-			mailbox, err := parseMailbox(p)
-			if err != nil {
-				p.RestoreState(parserState)
-			} else {
-				parsedFirstMailbox = true
-				result = append(result, mailbox)
-			}
-		}
-
-		// *("," [mailbox / CFWS])
-		if parsedFirstMailbox {
-			for {
-				if ok, err := p.parser.Matches(rfcparser.TokenTypeComma); err != nil {
-					return nil, false, err
-				} else if !ok {
-					break
+		// This section can optionally be one of the following: mailbox-list / CFWS / obs-group-list. So if
+		// we run out of input, we see semicolon or a double quote we should skip trying to parse this bit.
+		if !(p.parser.Check(rfcparser.TokenTypeEOF) ||
+			p.parser.Check(rfcparser.TokenTypeSemicolon) ||
+			p.parser.Check(rfcparser.TokenTypeDQuote)) {
+			// Mailbox
+			var parsedFirstMailbox bool
+
+			{
+				parserState := p.SaveState()
+				mailbox, err := parseMailbox(p)
+				if err != nil {
+					p.RestoreState(parserState)
+				} else {
+					parsedFirstMailbox = true
+					result = append(result, mailbox)
 				}
+			}
 
-				if ok, err := tryParseCFWS(p.parser); err != nil {
-					return nil, false, err
-				} else if ok {
-					continue
+			// *("," [mailbox / CFWS])
+			if parsedFirstMailbox {
+				for {
+					if ok, err := p.parser.Matches(rfcparser.TokenTypeComma); err != nil {
+						return nil, false, err
+					} else if !ok {
+						break
+					}
+
+					if ok, err := tryParseCFWS(p.parser); err != nil {
+						return nil, false, err
+					} else if ok {
+						continue
+					}
+
+					// Mailbox
+					mailbox, err := parseMailbox(p)
+					if err != nil {
+						return nil, false, err
+					}
+
+					result = append(result, mailbox)
 				}
-
-				// Mailbox
-				mailbox, err := parseMailbox(p)
-				if err != nil {
+			} else {
+				// If we did not parse a mailbox then we must parse CWFS
+				if err := parseCFWS(p.parser); err != nil {
 					return nil, false, err
 				}
-
-				result = append(result, mailbox)
 			}
 		}
 

rfc5322/parser_test.go

@@ -763,6 +763,13 @@ func TestParseDisplayNameOnlyShouldBeError(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestParseInvalidHeaderValueShouldBeError(t *testing.T) {
+	// E.g: Incorrect header format causes headers fields to be combined into one, this should be invalid.
+	const input = "FooBar From:Too Subjects:x"
+	_, err := ParseAddressList(input)
+	require.Error(t, err)
+}
+
 func TestParseEmptyStringIsNotError(t *testing.T) {
 	_, err := ParseAddressList("")
 	require.NoError(t, err)