Site name is not UTF-8 encoded in email headers

Nice catch! Can we have some tests please?

Same problem here. In my D8 installations, when we are using greek characters in "Site Name" (/admin/config/system/site-information) all system emails are flagged as spam by Gmail. For instance, in the site www.asfaleia-insurance.org which has the site-name "ΑΣΦΑΛΕΙΑ-INSURANCE", system emails are being treated as spam by Gmail (screenshot attached). In fact, I think ALL drupal8 sites with Greek names will experience the same problem. That problem did not exist in Drupal7 or Drupal6. The Unicode::mimeHeaderEncode patch in #2 resolves the issue.

This is ready.

I can probably go with Major priority on this one but it is not critical by our definitions.

1. +++ b/core/lib/Drupal/Core/Mail/MailManager.php
   @@ -12,6 +12,7 @@
   +use Drupal\Component\Utility\Unicode;
   

   I like imports to be in alphabetical order.

2. +++ b/core/modules/system/src/Tests/Mail/MailTest.php
   @@ -5,6 +5,7 @@
   +use Drupal\Component\Utility\Unicode;
   

   Same.

3. +++ b/core/modules/system/src/Tests/Mail/MailTest.php
   @@ -99,4 +100,23 @@ public function testFromAndReplyToHeader() {
   +  public function testFromWHeaderEncoding() {
   

   What does the "W" mean in this function name?

4. +++ b/core/modules/system/src/Tests/Mail/MailTest.php
   @@ -99,4 +100,23 @@ public function testFromAndReplyToHeader() {
   +    $from_email_enoded = Unicode::mimeHeaderEncode('Drépal'). ' <simpletest@example.com>';
   

   There is a typo in the variable name and the string concat operator must be surrounded by a single space.

5. +++ b/core/modules/system/src/Tests/Mail/MailTest.php
   @@ -99,4 +100,23 @@ public function testFromAndReplyToHeader() {
   +    $reply_email = 'someone_else@example.com';
   

   This variable is unused.

6. +++ b/core/modules/system/src/Tests/Mail/MailTest.php
   @@ -99,4 +100,23 @@ public function testFromAndReplyToHeader() {
   +  }
    }
   

   The last method in a class must have a blank line after it.

+++ b/core/modules/system/src/Tests/Mail/MailTest.php
@@ -99,4 +100,23 @@ public function testFromAndReplyToHeader() {
+    $from_email_enoded = Unicode::mimeHeaderEncode('Drépal'). ' <simpletest@example.com>';

Do not use the same function to encode the string. Use the encoded string instead. That's a better test.

Also, I think that we do not need another test method for this. A tiny modification of testFromAndReplyToHeader() would do.

I have rerolled the patch as per comment #18 & #19

Fixed the test fails.

+++ b/core/modules/system/src/Tests/Mail/MailTest.php
@@ -90,11 +91,13 @@ public function testFromAndReplyToHeader() {
+    $from_email_enoded = Unicode::mimeHeaderEncode('Drépal') . ' <simpletest@example.com>';

#18-4 and the last unnumbered comment in #18 still apply:

Do not use the same function to encode the (test) string. Use the encoded string instead. That's a better test.

Thanks @cilefen, made changes as per your suggestion #23.

+++ b/core/modules/system/src/Tests/Mail/MailTest.php
@@ -90,11 +91,13 @@ public function testFromAndReplyToHeader() {
+    $from_email_enoded = '=?UTF-8?B?RHLDqXBhbA==?= <simpletest@example.com>';

This variable name is still mistyped. Plus, since it is used only once, it doesn't need to exist. The string value can be used in the assertEqual.

Thanks @cilefen, made changes as per your suggestion #25

I'm confused because \Drupal\Core\Mail\Plugin\Mail\PhpMail::mail() encodes all the headers.

Looking at the email D8 sends without the patch it is mime-encoded....

Delivered-To: alex@example.com
Received: by 10.55.68.81 with SMTP id r78csp317952qka;
        Tue, 25 Oct 2016 15:47:13 -0700 (PDT)
X-Received: by 10.98.108.4 with SMTP id h4mr28067741pfc.11.1477435633846;
        Tue, 25 Oct 2016 15:47:13 -0700 (PDT)
Return-Path: <admin@example.com>
Received: from Alexs-MBP15.local (173-164-238-54-SFBA.hfc.comcastbusiness.net. [173.164.238.54])
        by mx.google.com with ESMTP id lt11si159909pab.140.2016.10.25.15.47.13
        for <alex@example.com>;
        Tue, 25 Oct 2016 15:47:13 -0700 (PDT)
Received-SPF: fail (google.com: domain of admin@example.com does not designate 173.164.238.54 as permitted sender) client-ip=173.164.238.54;
Authentication-Results: mx.google.com;
       spf=fail (google.com: domain of admin@example.com does not designate 173.164.238.54 as permitted sender) smtp.mailfrom=admin@example.com
Received: by Alexs-MBP15.local (Postfix, from userid 70)
	id C7E086FD8E52; Tue, 25 Oct 2016 15:47:10 -0700 (PDT)
To: alex@example.com
Subject: =?UTF-8?B?QW4gYWRtaW5pc3RyYXRvciBjcmVhdGVkIGFuIGFjY291bnQgZm9yIHlvdSBhdCA=?=  =?UTF-8?B?RHLDqXBhbERyw6lwYWxEcsOpcGFsRHLDqXBhbERyw6lwYWw=?=
X-PHP-Originating-Script: 502:PhpMail.php
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes
Content-Transfer-Encoding: 8Bit
X-Mailer: Drupal
Sender: admin@example.com
From: =?UTF-8?B?RHLDqXBhbERyw6lwYWxEcsOpcGFsRHLDqXBhbERyw6lwYWwgPGFkbWluQGV4YW0=?=@Alexs-MBP15.local,
	=?UTF-8?B?cGxlLmNvbT4=?=@Alexs-MBP15.local
Reply-to: admin@example.com
Message-Id: <20161025224712.C7E086FD8E52@Alexs-MBP15.local>
Date: Tue, 25 Oct 2016 15:47:10 -0700 (PDT)

DrépalDrépalDrépal,

A site administrator at DrépalDrépalDrépalDrépalDrépal has created an
account for you. You may now log in by clicking this link or copying and
pasting it into your browser:

http://drupal8alt.dev/user/reset/3/1477432611/yiBRA41LJ5ty6J8jXqTybY-qkiLG-Re8gIA33Rz_rxM

This link can only be used once to log in and will lead you to a page where
you can set your password.

After setting your password, you will be able to log in at
http://drupal8alt.dev/user in the future using:

username: DrépalDrépalDrépal
password: Your password

--  DrépalDrépalDrépalDrépalDrépal team

So what I think is the problem is that the email is mime encoded too.

The fix needs to be applied in \Drupal\Core\Mail\Plugin\Mail\PhpMail::mail to do something special for 'from' to extract the final things between < and > and then mime encode the rest.

Hmmm re #29...

    // Build the default headers.
    $headers = array(
      'MIME-Version' => '1.0',
      'Content-Type' => 'text/plain; charset=UTF-8; format=flowed; delsp=yes',
      'Content-Transfer-Encoding' => '8Bit',
      'X-Mailer' => 'Drupal',
    );
    // To prevent email from looking like spam, the addresses in the Sender and
    // Return-Path headers should have a domain authorized to use the
    // originating SMTP server.
    $headers['Sender'] = $headers['Return-Path'] = $site_mail;
    $headers['From'] = $site_config->get('name') . ' <' . $site_mail . '>';

Is in \Drupal\Core\Mail\MailManager::doMail() so there is some expectation about mime in the mail manager - but we the patch attached we'll be calling mimeHeaderEncode twice on the same string... does that matter?

Working on this at SprintWeek 2017 in Boston...

I independently came up with the same solution for this myself and I have tested the patch in #34.

#29 still hasn't been address. I don't think MailManager should be doing mimeEncoding. This should be the responsibility of \Drupal\Core\Mail\Plugin\Mail\PhpMail::mail() - which is atm mime encoding it is just doing it incorrectly in the case of the from header. And probably other headers.

Fixing it in PhpMail::mail() means we have to parse the from header to separate out the email address from the rest of it. While this is not particularly hard to do, it is more awkward then assembling things correctly from the start.

Yeah this is really tricky to see where the responsibility lies. All the mime encoding atm is happening in \Drupal\Core\Mail\Plugin\Mail\PhpMail::mail() but \Drupal\Core\Mail\MailManager::doMail() sets the mime version.

Looking at all the options I think this is probably the best option but I do think it is worth...

+++ b/core/lib/Drupal/Core/Mail/MailManager.php
@@ -248,7 +249,7 @@ public function doMail($module, $key, $to, $langcode, $params = [], $reply = NUL
-    $headers['From'] = $site_config->get('name') . ' <' . $site_mail . '>';
+    $headers['From'] = Unicode::mimeHeaderEncode($site_config->get('name')) . ' <' . $site_mail . '>';

... adding some commentary to the above code about why this is mime encoded here, and we aren't just letting PhpMail do it's thing. I've tried to read all the email RFCs and I think the important thing is this. The From field is specified here: https://tools.ietf.org/html/rfc2822#section-3.4 - if we mime encode the whole field we break the the format of the field being [display-name] angle-addr where angle-addr is [CFWS] "<" addr-spec ">" [CFWS] / obs-angle-addr and addr-spec is https://tools.ietf.org/html/rfc2822#section-3.4.1

tldr; mime encoding the entire from header means that the email address is not encoded correctly.

So back to needs work for improving/adding code comments.

Patch #34 with comments. Rolled against 8.4.x.

Any Update on this issue ?

It needs a review done.

I am adding credit to balagan for identifying and closing a duplicate at the Baltimore triage.

The other item is a duplicate of this. What we need is a review of the current patch.

What we need is a review of the current patch.

Based on my testing, the current patch is inadequate, namely:

1. It corrects UTF-8 display name for short names.
2. It makes the situation worse for long UTF-8 display names.

For short site name, hence short email display name, such as "Drépal" or "Fátima Maria", the patch correctly generates the following From header:

From: =?UTF-8?B?RsOhdGltYSBNYXJpYQ==?= <webmaster@example.org>

which gets rendered to:

From: "Fátima Maria" <webmaster@example.org>

But once we have something long, the =?UTF-8?B? thingy gets split into multiple lines, and then it goes terribly wrong because the domain name gets attached to every single split line:

From: =?UTF-8?B?PT9VVEYtOD9CPzQ0Q001b1dJNXErTjVvbUw1TGl0NTdlYTc3eU02WUdLNWEyUTY=?=@example.com,
	=?UTF-8?B?THFyNUxpSzZLR2o0NENDSU9pSHFPaWhqQT09Pz0KID0/VVRGLTg/Qj81YStHNWE=?=@example.com,
	=?UTF-8?B?K0c1N2lyNzd5TTVvU1A1b0dRNllHeTZZR3k1cTI0NDRDQ0lPaXFzT2lvZ09XdnU=?=@example.com,
	=?UTF-8?B?T2lOaWVXL2d3PT0/PQogPT9VVEYtOD9CPzc3eU01YUN4NWI2WDVMaUo1cGlsNXA=?=@example.com,
	=?UTF-8?B?cUo0NENDNDRDTjQ0Q001b1dJNXErTjVvbUw1TGl0NTdlYTc3eU0/PQogPT9VVEY=?=@example.com,
	=?UTF-8?B?LTg/Qj82WUdLNWEyUTZMcXI1TGlLNktHajQ0Q0NJT2lIcU9paGpPV3ZodVd2aHU=?=@example.com,
	=?UTF-8?B?ZTRxKys4ak9hRWorYUJrT21Cc2c9PT89CiA9P1VURi04P0I/NllHeTVxMjQ0NEM=?=@example.com,
	=?UTF-8?B?Q0lPaXFzT2lvZ09XdnVPaU5pZVcvZysrOGpPV2dzZVcrbCtTNGllYVlwZWFhaWU=?=@example.com,
	=?UTF-8?B?T0FnZz09Pz0KID0/VVRGLTg/Qj80NENONDRDTTVvV0k1cStONW9tTDVMaXQ1N2U=?=@example.com,
	=?UTF-8?B?YTc3eU02WUdLNWEyUTZMcXI1TGlLNktHajQ0Q0NJT2lIcUE9PT89CiA9P1VURi0=?=@example.com,
	=?UTF-8?B?OD9CPzZLR001YStHNWErRzU3aXI3N3lNNW9TUDVvR1E2WUd5NllHeTVxMjQ0NEM=?=@example.com,
	=?UTF-8?B?Q0lPaXFzT2lvZ09XdnVPaU5pUT09Pz0KID0/VVRGLTg/Qj81YitENzd5TTVhQ3g=?=@example.com,
	=?UTF-8?B?NWI2WDVMaUo1cGlsNXBxSjQ0Q0M0NENONDRDTTVvV0k1cStONW9tTDVMaXQ1N2U=?=@example.com,
	=?UTF-8?B?YT89CiA9P1VURi04P0I/Nzd5TTZZR0s/PSA8d2VibWFzdGVyQGVkbW9udG9uY2g=?=@example.com,
	=?UTF-8?B?aW5lc2VwYXJpc2gub3JnPg==?=@example.com

And we would even end up with strange email addresses like <bmaster>@example.com> instead of <webmaster@example.com>; it just randomly depends on where the lines got split.

I don't know which part of the Drupal or PHP code indiscriminately adds those domain names at the wrong places, but that needs to be dealt with too.

So, please do some more investigation, preferably with more test cases, especially with longer site names (e.g., a string with about 30 CJK characters would trigger this), so that no regression is introduced with corner cases.

#49 indicates it needs more test coverage.

Thanks. Does anyone know what code causes it to be split up?

I don't know which part of the Drupal or PHP code indiscriminately adds those domain names at the wrong places, but that needs to be dealt with too.

Those are likely added by the MTA.

We have been facing the same issue as mentioned in #49. Bigger site names get's split's into multiple lines.
I guess the following function is responsible for splitting.

public static function mimeHeaderEncode($string) {
    if (preg_match('/[^\x20-\x7E]/', $string)) {
      $chunk_size = 47; // floor((75 - strlen("=?UTF-8?B??=")) * 0.75);
      $len = strlen($string);
      $output = '';
      while ($len > 0) {
        $chunk = static::truncateBytes($string, $chunk_size);
        $output .= ' =?UTF-8?B?' . base64_encode($chunk) . "?=\n";
        $c = strlen($chunk);
        $string = substr($string, $c);
        $len -= $c;
      }
      return trim($output);
    }
    return $string;
  }

This function can found at "/core/lib/Drupal/Component/Utility/Unicode.php"

Thanks, @das.gautam27. Does anyone know why Unicode::mimeHeaderEncode() breaks up the string into 47 character chunks? I am about to write a version of this patch which doesn't do this, but perhaps there is a reason why this is done and the patch needs to do something else.

Answer: It is a requirement of RFC 2047.

RFC 2047 requires that the encoded-word (the =?UTF-8?B?...?= thing) be no longer than 75 characters in total, including the =?UTF-8?B? and ?=. Unicode::mimeHeaderEncode() enforces this length, breaking things up as needed.

What I do not know is what a From header is supposed to look like if display-name is long enough that it needs an encoded-word that is longer than 75 characters. It may be that it should do what it does with the current patch, but later on there is a bug which adds the domains, as observed in #49.

One possible partial solution: If the site name is long enough that more than one encoded-word is returned by Unicode::mimeHeaderEncode(), drop all but the first of these. This completely solves it for shorter site names and at least makes a valid header for longer ones. Only site names that require encoding and are too long would be truncated. Thoughts on this?

Note, that according to mentioned RFC 2047:

If it is
desirable to encode more text than will fit in an 'encoded-word' of
75 characters, multiple 'encoded-word's (separated by CRLF SPACE) may
be used.

While there is no limit to the length of a multiple-line header
field, each line of a header field that contains one or more
'encoded-word's is limited to 76 characters.

So CRLF SPACE charchters don't need to be encoded and I fixed that bug for my projects (with very long russian words in email headers) with simple patch attached.

What it is saying is that CRLF SPACE is needed to separate each encoded-word. Those characters still need to be encoded.

No, they don't, actually. Look at RFC 2822:

2.2.3. Long Header Fields

Each header field is logically a single line of characters comprising
the field name, the colon, and the field body. For convenience
however, and to deal with the 998/78 character limitations per line,
the field body portion of a header field can be split into a multiple
line representation; this is called "folding". The general rule is
that wherever this standard allows for folding white space (not
simply WSP characters), a CRLF may be inserted before any WSP. For
example, the header field:

Subject: This is a test

can be represented as:

Subject: This
is a test

So, CRLF is OK in header field body.

Guys, it's crazy situation.

Drupal is unusable since May 2016, more than year for today, because users can't receive emails at all, i.e. they can't register.

There is patch:
- $headers['From'] = $site_config->get('name') . ' <' . $site_mail . '>';
+ $headers['From'] = Unicode::mimeHeaderEncode($site_config->get('name')) . ' <' . $site_mail . '>';

This patch works good. But patch still is not in the code. What's happen ?

@alex73 happens that the problem it's not correctly solved when you use long names (more than 78 characters). See comments from #49 to #58, we need to add tests and fix for large names.

This patch is like #42 except that for long names, only the first encoded-word is included. The effect is that this bug is completely solved for everything except long names that include special characters. In these cases, the site name will be cut off. This patch fixes the problem for most people without making it worse for anyone.

We are working on triaging this issue with Merel van Empel @emmezali

Just commenting for notices. Whazzup!

When triaging this issue we got stuck trying to reproduce the bug.
Steps we did to try and reproduce the bug:
- Install Drupal 8.5.x locally using Acquia Desktop
- Create a new account with actual real Gmail address via /admin/people/create (ticked the box "Notify user of new account")
No e-mail received. Our hypothesis is that that is because it's locally, not online.
So:
Next step was to use simplytest.me to create a Drupal8.5.x site and repeat the same steps (Create account + tick box)
No e-mail received.

What is needed is a guide to set up a drupal site that will be able to send e-mail to real Gmail adresses.

An educated guess on how to do this is to set up a mail server, and put the drupal installation on that same server.

We will not be doing that today, because this was only triage.

To track down/test the patch, you need some mail catching tool (check https://www.drupal.org/docs/develop/local-server-setup/managing-mail-han... for the options)

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

Getting the issue here, too. Lost a lot of time debugging this.
The easy fix for me was removing non ascii characters from the site name.
But a proper fix would be welcome ....

Please try the patch in #62.

Looking at another PHP mailer implementation https://github.com/swiftmailer/swiftmailer/blob/8388605460335d4c21481eb3... - shortening is exactly what that does. I like their variable name $shorten since I have to think less about what is actually happening.

We need a test for a long site name.

Ah so there is a test for long site names. I think we should do something like:

$this->assertEquals('Drépal this is a very long test sentence to te <simpletest@example.com>', Unicode::mimeHeaderDecode($sent_message['headers']['From']), 'From header is correctly encoded.');

In the test because it makes it clearer that the truncation is successful. However we have another problem. Looking at SwiftMail when we shorten we need also to allow for the fact that From: takes some space on the line. As the RFC states:

While there is no limit to the length of a multiple-line header
field, each line of a header field that contains one or more
'encoded-word's is limited to 76 characters.

SwiftMail does this but we don't :( - in fact this is a bug in our implementation - so not something that needs to be addressed here. We should file a followup to say our PHPMail implement does not obey the RFC.

Here's a patch that does the suggestions in #71 - there's already an issue for the header length problem - #2407117: Unicode::mimeHeaderEncode() ignores header field name

I think $first_chunk_only is more clear than $shorten, but I'm good with it either way. The additional test is good. What is the difference between assertEqual() and assertEquals()?

@Liam Morland, assertEquals is the phpunit unit way of doing this, assertEqual is the older simpletest way (that still works because of a legacy trait.

New code should use the assertEquals call.

Patch in #72 looks very good, there's no bc break because the new functionality is behind a flag that defaults to FALSE. There's also sufficient new testcoverage and the existing tests don't break.

I also prefer $shorten instead of $first_chunk_only - it's clearer to me what it's doing.

error: patch failed: core/lib/Drupal/Component/Utility/Unicode.php:603
error: core/lib/Drupal/Component/Utility/Unicode.php: patch does not apply

Patch no longer applies sorry

Rerolled the patch #72 because it is failed to apply on 8.5.x branch.

+++ b/core/lib/Drupal/Core/Mail/MailManager.php
@@ -248,7 +249,10 @@ public function doMail($module, $key, $to, $langcode, $params = [], $reply = NUL
+    // Headers are encoded in MailManager::doMail(). The site name must be
+    // encoded here to prevent doMail() from encoding the email address, which
+    // would break the header.

This comment is not quite right. The headers in vanilla Drupal are encoded by \Drupal\Core\Mail\Plugin\Mail\PhpMail::mail().

Patch to address #78.

Remove tag.

Looks good.

https://www.drupal.org/pift-ci-job/814524 failed for unrelated reasons.

This issue is unlucky... another random fail due to the testbot being super slow.

Adding credit for reviewers

Committed as 66fc180 and pushed to 8.5.x.

Cherry-picked as 9702106 and pushed to 8.4.x

Thanks all

Thank you!