SymfonyMailer RfcComplianceException when sending to multiple comma-separated addresses

parse $message['to'] into an array of addresses

It appears Symfony devs have explored possible solutions for this, although it is unclear if they implemented anything: https://github.com/symfony/symfony/issues/40205

Notably, it appears the same exception was encountered in Drupal core in the past, but in a slightly different context: 🐛 Uncaught RfcComplianceException when email From name contains a comma Fixed

The exception comes from Symfony\Component\Mime\Address

Drupal core uses Address to check that the "From" address in MailManager in order to "Make sure the site-name is a RFC-2822 compliant 'display-name'":

https://git.drupalcode.org/project/drupal/-/blob/f8ede99aabdf39e95aeb259...

But it does not check the "To" address(es) with Address at any point. Symfony mailer does, however, which is why this issue starts to happen once you start using Symfony mailer.

Updated the description to include the full error message.

Would it be more elegant or a better API for $message['to'] to contain either a string (one address) or an array (multiple addresses)?

@cilefen Yes that would be more elegant, but I think it would require much bigger changes to Drupal mail handling outside the scope of this specific SymfonyMailer class. So unfortunately, in order to fix this bug, we need to deal with the fact that $message['to'] is a string, and figure out how to parse it into an array. As it stands right now, SymfonyMailer is broken when sending to multiple email addresses.

So this is like a two-line fix.

Well... based on the comments in the Symfony issue I linked to above, it may not be as simple as adding an explode(). 😅

@althaus says:

My main concern would be the how as the RFCs (with https://tools.ietf.org/html/rfc5322 being the latest) seems quite nasty. Especially the part with quoting and the comma separation seem to be tricky.

@javiereguiluz says:

We could get inspiration from Go which parses addresses according to RFC 5322 but makes some needed simplifications. See https://github.com/golang/go/blob/master/src/net/mail/message.go

@fabpot suggests he would be open to that approach:

I'm ok with adding such a feature in Symfony Mime and getting inspiration from Go might help.

But as far as I can tell this has not been done yet. So we are left with the problem, because Drupal core is starting with a string of multiple comma-separated addresses.

We could consider using imap_rfc822_parse_adrlist(), but it sounds like that has some problems, which is what prompted the Symfony issue...

We're currently evaluating to replace the unreliable function imap_rfc822_parse_adrlist (https://www.php.net/manual/function.imap-rfc822-parse-adrlist.php) from our codebase as it has some real flaws on its error handling.

Maybe it's the best way to start, and we could potentially leverage something better provided by Symfony in the future (if they ever implement it)?

We could consider using imap_rfc822_parse_adrlist()

Scratch that idea. It requires the imap extension to be installed.

Here is a simple MR+patch that uses str_getcsv() (instead of explode()), because it should ignore commas that are enclosed in double-quotes.

str_getcsv() was also used in 🐛 Uncaught RfcComplianceException when email From name contains a comma Fixed ...

Instead of using explode() to split From addresses by comma, use str_getcsv() instead. That function supports ignoring comma's encapsulated in double quotes. Besides that, it seems to have the same effect as explode().

https://www.drupal.org/project/drupal/issues/3226117#comment-14192545 🐛 Uncaught RfcComplianceException when email From name contains a comma Fixed

This might not work for all cases, but it's a first step to get the ball rolling at least. Needs tests too, of course.

Thanks, I agree with the approach. Left a small note in the MR.

A simple test case could go into SymfonyMailerTest.php