From address not properly quoted. Causes errors if comma in site name.

I tested the patch #5 and it fixes the point.

I wonder if we should consider a case that a site name has some double quote characters in it. If I understand it correctly, I'd like someone to review this revised patch. This patch adds a logic to escape double quotes and a test for that to the patch #5.

This looks like a duplicate of #2717965: Site name is not UTF-8 encoded in email headers. Please try the latest patch in that issue.

Hello Liam,

This looks like a duplicate of #2717965: Site name is not UTF-8 encoded in email headers. Please try the latest patch in that issue.

For better or for worse, this issue is not a duplicate of #2717965, and the latest patch there does not solve this "comma in site name" issue. I just tested it with Drupal 8.3.2.

That said, the patches for both issues would involve modifying lines of code in the same vicinity.

OK. I think this would be easier to fix if the other issue is fixed first to avoid overlapping patches. Could you do a review of the other patch?

I just ran into this myself, and the problem is independent of the mailer module used. i can duplicate this on stock D8, though then whether or not the email goes through depends on the target mail server (e.g. in my case, emails to @gmail.com addresses get bounced for having multiple from addresses, as the local Postfix takes each word before the comma as a local username and tacks @~local server name~ on it).

The workaround, of course, is to take the comma out of the site name, for now.

Working on triaging this issue during Vienna2017. Together with @Anna_Radulovski.

We have trouble recreating this bug, because we don't know which mailservices are using an RFC 822 compatible mailbox parser.
It would be great to get some suggestions for which mailservice to use (Gmail, Hotmail, etc.), or instructions on how to find out if a mailservice is using an RFC 822 compatible mailbox parser.

If anyone has suggestions for a local mailservice (like mailcatcher for instance) that is using an RFC 822 compatible mailbox parser, that would be even better.

Updated the Issue Summary as suggested by comment #14

You could also add some debugging output to MailManager::doMail() and observe that the headers are not encoded properly.

It looks as though the patches are no longer valid with the unicode work being pulled into core. Updating issue.

Updated patch will pass the test with interdiff.

One more try

I think if you are always using mimeHeaderEncode(), then you don't need to replace '"' with '\"'; the quotes will be escaped anyway.

But, do we want all site names, even ones with nothing but letters and numbers, to go through mimeHeaderEncode()?

I don't think the actual case of a comma is tested in the test in #26, the site name has a period but not a comma.

Should we change the description or the test?

1. +++ b/core/modules/system/tests/src/Functional/Mail/MailTest.php
   @@ -102,6 +102,21 @@ public function testFromAndReplyToHeader() {
   +    // Check that the From-header contains the site name which is properly quoted.
   

   Goes over 80 cols.

2. +++ b/core/modules/system/tests/src/Functional/Mail/MailTest.php
   @@ -102,6 +102,21 @@ public function testFromAndReplyToHeader() {
   +    $site_name_to_be_quoted = 'Drupal.site"';
   

   This does not have a comma, and the title of the issue is about a comma, so we should have a testcase that also has a comma.

There are a couple duplicates out there, which I've just associated with this issue.

There's a critical point made by longwave in comment #10 of issue 2898750

in that MIME encoded headers / words should not be quoted.

http://www.rfc-editor.org/rfc/rfc2047.txt says
+ An 'encoded-word' MUST NOT appear within a 'quoted-string'.

I shared a couple variations on that thread of how to detect an ASCII-only site name and then quote and quote-escape the value only if it's ASCII.

RFC2047 is focused on "textual header information in character sets other than US-ASCII" and the MIME standards all seem to address non-ASCII formats so tampering with mimeHeaderEncode() seems inappropriate since the method and the RFC it references are geared to non-ASCII. So, this would mean a fix that involves MailManager::doMail().

$encoded_site_name = Unicode::mimeHeaderEncode($site_config->get('name'), TRUE);

// if the encoded site name contains only ASCII characters
if (preg_match('/[^\\x20-\\x7E]/', $encoded_site_name)) {
    // wrap the ASCII in double-quotes to account for special ASCII characters, like commas
    $encoded_site_name = '"' . addcslashes($encoded_site_name, '"') . '"';
}

$headers['From'] =  $encoded_site_name. ' <' . $site_mail . '>';

or a variation

$encoded_site_name = Unicode::mimeHeaderEncode($site_config->get('name'), TRUE);

// if the encoded site name matches the original value then it contains only ASCII characters
if ( $site_config->get('name') == $encoded_site_name)
{
    // wrap the ASCII in double-quotes to account for special ASCII characters, like commmas
    $encoded_site_name = '"' . addcslashes($encoded_site_name, '"') . '"';
}

$headers['From'] =  $encoded_site_name. ' <' . $site_mail . '>';

This also begs the question of if the $reply variable in doMail() also needs to get the same treatment as the From because Reply-To also allows for a name portion before the email address.

This seems to have been fixed in #2936032: Sites named with special characters cannot send mail. Closing this issue as duplicate.

The need for a test with a comma in it needed to be brought over to the other issue before this was closed as a duplicate.

Exact same problem causing fatal errors on site due to #3226117: Uncaught RfcComplianceException when email From name contains a comma

That one does have the test, so nothing to do here although i found this issue 20 minutes before finding the other, so, leaving the link here for future me and the note about getting the experienced problem cases into tests for everybody.