Issue #2936032 by gambry, nlisgo, alexpott, cwells, cilefen, Darvanen, DamienGR: Sites named with special characters cannot send mail

core/lib/Drupal/Component/Utility/Mail.php

@@ -0,0 +1,67 @@
+<?php
+
+namespace Drupal\Component\Utility;
+
+/**
+ * Provides helpers to ensure emails are compliant with RFCs.
+ *
+ * @ingroup utility
+ */
+class Mail {
+
+  /**
+   * RFC-2822 "specials" characters.
+   */
+  const RFC_2822_SPECIALS = '()<>[]:;@\,."';
+
+  /**
+   * Return a RFC-2822 compliant "display-name" component.
+   *
+   * The "display-name" component is used in mail header "Originator" fields
+   * (From, Sender, Reply-to) to give a human-friendly description of the
+   * address, i.e. From: My Display Name <xyz@example.org>. RFC-822 and
+   * RFC-2822 define its syntax and rules. This method gets as input a string
+   * to be used as "display-name" and formats it to be RFC compliant.
+   *
+   * @param string $string
+   *   A string to be used as "display-name".
+   *
+   * @return string
+   *   A RFC compliant version of the string, ready to be used as
+   *   "display-name" in mail originator header fields.
+   */
+  public static function formatDisplayName($string) {
+    // Make sure we don't process html-encoded characters. They may create
+    // unneeded trouble if left encoded, besides they will be correctly
+    // processed if decoded.
+    $string = Html::decodeEntities($string);
+
+    // If string contains non-ASCII characters it must be (short) encoded
+    // according to RFC-2047. The output of a "B" (Base64) encoded-word is
+    // always safe to be used as display-name.
+    $safe_display_name = Unicode::mimeHeaderEncode($string, TRUE);
+
+    // Encoded-words are always safe to be used as display-name because don't
+    // contain any RFC 2822 "specials" characters. However
+    // Unicode::mimeHeaderEncode() encodes a string only if it contains any
+    // non-ASCII characters, and leaves its value untouched (un-encoded) if
+    // ASCII only. For this reason in order to produce a valid display-name we
+    // still need to make sure there are no "specials" characters left.
+    if (preg_match('/[' . preg_quote(Mail::RFC_2822_SPECIALS) . ']/', $safe_display_name)) {
+
+      // If string is already quoted, it may or may not be escaped properly, so
+      // don't trust it and reset.
+      if (preg_match('/^"(.+)"$/', $safe_display_name, $matches)) {
+        $safe_display_name = str_replace(['\\\\', '\\"'], ['\\', '"'], $matches[1]);
+      }
+
+      // Transform the string in a RFC-2822 "quoted-string" by wrapping it in
+      // double-quotes. Also make sure '"' and '\' occurrences are escaped.
+      $safe_display_name = '"' . str_replace(['\\', '"'], ['\\\\', '\\"'], $safe_display_name) . '"';
+
+    }
+
+    return $safe_display_name;
+  }
+
+}

core/lib/Drupal/Core/Mail/MailManager.php

@@ -5,7 +5,7 @@ namespace Drupal\Core\Mail;
 use Drupal\Component\Render\MarkupInterface;
 use Drupal\Component\Render\PlainTextOutput;
 use Drupal\Component\Utility\Html;
-use Drupal\Component\Utility\Unicode;
+use Drupal\Component\Utility\Mail as MailHelper;
 use Drupal\Core\Logger\LoggerChannelFactoryInterface;
 use Drupal\Core\Messenger\MessengerTrait;
 use Drupal\Core\Plugin\DefaultPluginManager;
@@ -254,12 +254,8 @@ class MailManager extends DefaultPluginManager implements MailManagerInterface {
     // Return-Path headers should have a domain authorized to use the
     // originating SMTP server.
     $headers['Sender'] = $headers['Return-Path'] = $site_mail;
-    // Headers are usually encoded in the mail plugin that implements
+    // Make sure the site-name is a RFC-2822 compliant 'display-name'.
-    // \Drupal\Core\Mail\MailInterface::mail(), for example,
+    $headers['From'] = MailHelper::formatDisplayName($site_config->get('name')) . ' <' . $site_mail . '>';
-    // \Drupal\Core\Mail\Plugin\Mail\PhpMail::mail(). The site name must be
-    // encoded here to prevent mail plugins from encoding the email address,
-    // which would break the header.
-    $headers['From'] = Unicode::mimeHeaderEncode($site_config->get('name'), TRUE) . ' <' . $site_mail . '>';
     if ($reply) {
       $headers['Reply-to'] = $reply;
     }

core/modules/system/tests/src/Functional/Mail/MailTest.php

@@ -107,6 +107,32 @@ class MailTest extends BrowserTestBase {
     $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.');
     $this->assertFalse(isset($sent_message['headers']['Reply-to']), 'Message reply-to is not set if not specified.');
     $this->assertFalse(isset($sent_message['headers']['Errors-To']), 'Errors-to header must not be set, it is deprecated.');
+
+    // Test RFC-2822 rules are respected for 'display-name' component of
+    // 'From:' header. Specials characters are not allowed, so randomly add one
+    // of them to the site name and check the string is wrapped in quotes. Also
+    // hardcode some double-quotes and backslash to validate these are escaped
+    // properly too.
+    $specials = '()<>[]:;@\,."';
+    $site_name = 'Drupal' . $specials[rand(0, strlen($specials) - 1)] . ' "si\te"';
+    $this->config('system.site')->set('name', $site_name)->save();
+    // Send an email and check that the From-header contains the site name
+    // within double-quotes. Also make sure double-quotes and "\" are escaped.
+    \Drupal::service('plugin.manager.mail')->mail('simpletest', 'from_test', 'from_test@example.com', $language);
+    $captured_emails = \Drupal::state()->get('system.test_mail_collector');
+    $sent_message = end($captured_emails);
+    $escaped_site_name = str_replace(['\\', '"'], ['\\\\', '\\"'], $site_name);
+    $this->assertEquals('"' . $escaped_site_name . '" <simpletest@example.com>', $sent_message['headers']['From'], 'From header is correctly quoted.');
+
+    // Make sure display-name is not quoted nor escaped if part on an encoding.
+    $site_name = 'Drépal, "si\te"';
+    $this->config('system.site')->set('name', $site_name)->save();
+    // Send an email and check that the From-header contains the site name.
+    \Drupal::service('plugin.manager.mail')->mail('simpletest', 'from_test', 'from_test@example.com', $language);
+    $captured_emails = \Drupal::state()->get('system.test_mail_collector');
+    $sent_message = end($captured_emails);
+    $this->assertEquals('=?UTF-8?B?RHLDqXBhbCwgInNpXHRlIg==?= <simpletest@example.com>', $sent_message['headers']['From'], 'From header is correctly encoded.');
+    $this->assertEquals($site_name . ' <simpletest@example.com>', Unicode::mimeHeaderDecode($sent_message['headers']['From']), 'From header is correctly encoded.');
   }
 
   /**

core/tests/Drupal/Tests/Component/Utility/MailTest.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace Drupal\Tests\Component\Utility;
+
+use Drupal\Component\Utility\Mail;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Test mail helpers implemented in Mail component.
+ *
+ * @group Utility
+ *
+ * @coversDefaultClass \Drupal\Component\Utility\Mail
+ */
+class MailTest extends TestCase {
+
+  /**
+   * Tests RFC-2822 'display-name' formatter.
+   *
+   * @dataProvider providerTestDisplayName
+   * @covers ::formatDisplayName
+   */
+  public function testFormatDisplayName($string, $safe_display_name) {
+    $this->assertEquals($safe_display_name, Mail::formatDisplayName($string));
+  }
+
+  /**
+   * Data provider for testFormatDisplayName().
+   *
+   * @see testFormatDisplayName()
+   *
+   * @return array
+   *   An array containing a string and its 'display-name' safe value.
+   */
+  public function providerTestDisplayName() {
+    return [
+      // Simple ASCII characters.
+      ['Test site', 'Test site'],
+      // ASCII with html entity.
+      ['Test &amp; site', 'Test & site'],
+      // Non-ASCII characters.
+      ['Tést site', '=?UTF-8?B?VMOpc3Qgc2l0ZQ==?='],
+      // Non-ASCII with special characters.
+      ['Tést; site', '=?UTF-8?B?VMOpc3Q7IHNpdGU=?='],
+      // Non-ASCII with html entity.
+      ['T&eacute;st; site', '=?UTF-8?B?VMOpc3Q7IHNpdGU=?='],
+      // ASCII with special characters.
+      ['Test; site', '"Test; site"'],
+      // ASCII with special characters as html entity.
+      ['Test &lt; site', '"Test < site"'],
+      // ASCII with special characters and '\'.
+      ['Test; \ "site"', '"Test; \\\\ \"site\""'],
+      // String already RFC-2822 compliant.
+      ['"Test; site"', '"Test; site"'],
+      // String already RFC-2822 compliant.
+      ['"Test; \\\\ \"site\""', '"Test; \\\\ \"site\""'],
+    ];
+  }
+
+}