📌 go-guerrilla cannot properly handle some valid addresses.

[issuefiler]

https://github.com/flashmob/go-guerrilla/blob/51f7dda326b1e9878e5f679ccb34a134127951b0/mail/envelope.go#L32-L48

What this little structure can do is rather limited. Address.String() is and may be used in many places throughout the code (e.g. populating headers, storing into a database.), yet it sometimes screws things up; it is not reliable.

Test cases

Quoted Local-parts ^{✅ VALID INPUTS}

MAIL FROM: <"  yo-- man wazz'''up? surprise surprise, this is POSSIBLE@fake.com "@example.com>
250 2.1.0 OK

• (return_path).User: [SPACE][SPACE]yo--[SPACE]man[SPACE]wazz'''up?[SPACE]surprise[SPACE]surprise,[SPACE]this[SPACE]is[SPACE]POSSIBLE@fake.com[SPACE]. ^{❌ INVALID Local-part}
• (return_path).Host: example.com.
• (return_path).String(): [SPACE][SPACE]yo--[SPACE]man[SPACE]wazz'''up?[SPACE]surprise[SPACE]surprise,[SPACE]this[SPACE]is[SPACE]POSSIBLE@fake.com[SPACE]@example.com. ^{❌ INVALID Mailbox}

address-literal mailboxes ^{✅ VALID INPUTS}

MAIL FROM: <hi@[1.1.1.1]>
250 2.1.0 OK

• (return_path).User: hi.
• (return_path).Host: 1.1.1.1. ^{❌ INVALID address-literal}
• (return_path).String(): hi@1.1.1.1. ^{❌ INVALID Mailbox}

MAIL FROM: <hi@[IPv6:2001:db8::8a2e:370:7334]>
250 2.1.0 OK

• (return_path).User: hi.
• (return_path).Host: 2001:db8::8a2e:370:7334. ^{❌ INVALID address-literal; NEEDS TO BE NORMALIZED FOR CORRECT COMPARISONS.}
• (return_path).String(): hi@2001:db8::8a2e:370:7334. ^{❌ INVALID Mailbox}

MAIL FROM: <hi@[IPv6:2001:DB8::8A2E:370:7334]>
250 2.1.0 OK

• (return_path).User: hi.
• (return_path).Host: 2001:DB8::8A2E:370:7334. ^{❌ INVALID address-literal; NEEDS TO BE NORMALIZED FOR CORRECT COMPARISONS.}
• (return_path).String(): hi@2001:DB8::8A2E:370:7334. ^{❌ INVALID Mailbox}

Null return-path and Postmaster recipient ^{✅ VALID INPUTS}

MAIL FROM: <>
250 2.1.0 OK
RCPT TO: <Postmaster>
250 2.1.5 OK

• (return_path).IsEmpty(): true. ^✅
• (return_path).String(): @. ^{❌ INVALID Mailbox; and p_sql.go doesn’t care about it. Would be better to make it return an empty string instead.}
• (recipient).String(): Postmaster@ ^{❌ SHOULD JUST BE Postmaster.}

---

Notes

For the User part

1. Minimize quoting, but preserve quotes if needed. This normalization can be done by getting the unquoted & unescaped form and quoting & escaping it if needed.
   • " al\ph\a "@grr.la should be " alpha "@grr.la.
   • "alpha"@grr.la should be alpha@grr.la.
   • "alp\h\a"@grr.la should be alpha@grr.la.

From RFC 5321:

For any purposes that require generating or comparing
   Local-parts (e.g., to specific mailbox names), all quoted forms MUST
   be treated as equivalent, and the sending system SHOULD transmit the
   form that uses the minimum quoting possible.

2. The Postmaster mailbox is case-insensitive (by RFC). go-guerrilla may safely normalize it into Postmaster.

https://github.com/flashmob/go-guerrilla/blob/51f7dda326b1e9878e5f679ccb34a134127951b0/mail/rfc5321/parse.go#L96-L99

That’s the code doing that, but what’s inconsistent there is that it only does that when it has no host part: <posTMAstEr> → <Postmaster> but <pOstMastEr@example.com> → <pOstMastEr@example.com>. Postmaster is case-insensitive regardless of having a host. So why don’t we normalize it for the both cases? Hold on, what should we do for <"P\O\STMASTER"@example.com>?

For the Host part

If an address-literal is given, it should normalize it into bytes, and give them some appropriate brackets, [IPv6:……] or [……]. Such normalization also helps to resolve #197.

Considerations

• How to get the unquoted & unescaped form of the Local-part: as shown above, the current go-guerrilla does unquote it, but does it unescape it, too? ⸺ No, it doesn’t……. "moto\y\a\s\u" becomes just moto\y\a\s\u. Could we modify the parser not to accumulate the backslash, so that we can use it for the normalization steps? https://github.com/flashmob/go-guerrilla/blob/51f7dda326b1e9878e5f679ccb34a134127951b0/mail/rfc5321/parse.go#L487-L490
• When to quote: when the unquoted & unescaped form of the Local-part has a character that is neither atext nor ..

  atext           =   ALPHA / DIGIT /    ; Printable US-ASCII
                     "!" / "#" /        ;  characters not including
                     "$" / "%" /        ;  specials.  Used for atoms.
                     "&" / "'" /
                     "*" / "+" /
                     "-" / "/" /
                     "=" / "?" /
                     "^" / "_" /
                     "`" / "{" /
                     "|" / "}" /
                     "~"

• What to escape when quoting the unquoted & unescaped: \ and ".

  qtextSMTP      = %d32-33 / %d35-91 / %d93-126
                ; i.e., within a quoted string, any
                ; ASCII graphic or space is permitted
                ; without blackslash-quoting except
                ; double-quote and the backslash itself.

[issuefiler]

Personally this is the number one priority for me. I can tackle everything else as I have my own set of processors, but this……, this, unfortunately, is a job of the core, which I have zero knowledge of. I need your insight and help on this if you don’t mind.

Unlike internationalized emails (SMTPUTF8) (#152), of which go-guerrilla lacks support to begin with, go-guerrilla doesn’t decline those addresses above (as they are valid), and it may cause some processors to malfunction.

[flashmob]

Thanks!

The evaluation of quoted/escaped strings in the todo list https://github.com/flashmob/go-guerrilla/issues/201 , so that one should be done first.

TODO

• issue #201
• add support for address-literals to mail.Address. Add a new property, AddressType, with 0 = none, 1 = ipv4, 2 = ipv6). The literal will be stored in the Host, but when String() is used, the literal will be output inside square brackets []
• Normalize ipv6. It looks like we are already normalizing it, but throwing away the result! See parse.go ipv6AddressLiteral. Instead of using ipstr, we should use v.String() and save that result to our s.accept buffer
• Fix the null return path so that String() returns an empty string
• Fix String() return if the local-part is just postmaster. (The string should be just postmaster

[flashmob]

Changes made in PR #202

• Quoted Local-parts now should be quoted when returned from String()
• A new Quoted bool property added to Address, to flag if local part's quoted
• A new IP net.IPproperty added added to Address to store the IP in binary form. (It's nil if no valid address matched)
• address-literals should now format with braces when returned from String()
• ipv6 address-literals are now normalized
• if local part is just postmaster, String() returns postmaster
• Null return path returns empty string