Update parsing of recipients

[yech199]

Closes # .

Changes proposed in this pull request

The RFC 2822 §3.6.2, which the parser adheres to, states:

The originator fields of a message consist of the from field, the sender field (when applicable), and optionally the reply-to field. The from field consists of the field name "From" and a comma-separated list of one or more mailbox specifications. If the from field contains more than one mailbox specification in the mailbox-list, then the sender field, containing the field name "Sender" and a single mailbox specification, MUST appear in the message. In either case, an optional reply-to field MAY also be included, which contains the field name "Reply-To" and a comma-separated list of one or more addresses.

In other words:

• The from and reply-to fields both consist of a comma-separated list of one or more recipients.
• The sender field MUST appear if the from field contains more than one recipient. If present, the sender field contains a single recipient.

Therefore, this change proposes returning all from and reply-to recipients if present, and only the first sender recipient if present.

[bcardarella]

I'm concerned this is going to break substatial backwards compatibility. Not to say we shouldn't do it, but it needs to be taken into consderation.

@andrewtimberlake thoughts?

[andrewtimberlake]

While this is technically correct, I’m not inclined to accept this change.
The reality is that most email systems expect a single from address.
SPF, DKIM, and DMARC are all built around the expectation that there is a single from address.

[MikaelFangel]

Fair points on backwards compatibility. Would it be possible to make this parsing optional using opts, so the current API stays as-is?

We know the BCC header shouldn't be there in delivered mail, and even when it is, it may be empty (just Bcc: with no addresses) to signal BCC recipients existed without revealing who.

Our use-case for this feature is as a security company we analyze emails and want as full a picture of the email received as possible even if it includes rare edge cases. 😃

[bcardarella]

@MikaelFangel we've had one or two issues in the past where the strict adherance to the spec ran afoul to the industry usage. I think what we really require is a RFC2822 strict mode. It would be one you could opt into or a feature flag this way for those seeking to really stick 100% to the specification it meets your needs then for everybody else it would still have the current behavior.

[MikaelFangel]

@bcardarella - That makes sense. Thank you for your responses and time 😊 we'll probably just use it as is for now then.

[bcardarella]

@MikaelFangel we do support the Encoder and Renderer being swapped out as an adapter. However, I suspect this breaks in certain areas at the moment like recipiet verification. If you all are interested in this I think a PR to ensure that functions that are hard coded for the RFC2822 adapaters allow the optoinal override. The pattern is already in functions like render/2 and just needs to be applied elsewhere. This way you all can just copy out the existing RFC2822 encoder and renderer and modify it to fit your needs.

[MikaelFangel]

@bcardarella - perfect we give it a look.

[yech199]

I understand the concerns regarding backward compatibility, and that most email systems expect a single From recipient...

But since this library is intended for general email parsing and the parser is said to adhere to the RFC 2822, my understanding is that it should ensure correct parsing of all valid header fields defined by the RFC - including cases where multiple From recipients are present. While SPF, DKIM, and DMARC are all built around the expectation that there is a single from address, that expectation may not necessarily need to constrain a general-purpose mail library. It would be a shame to lose potentially important information from a valid email because it was dropped during parsing...

With that in mind, I really hope you will still give the change request some consideration. Until then, we will try swapping it out as you suggested ☺️ Thank you~

[bcardarella]

@yech199 as I mentioned a strict mode would probably be appropriate for this