Verified Commit 9d3ea0a8 authored by Lee Rowlands's avatar Lee Rowlands
Browse files

Issue #3226117 by MegaChriz, marthinal, longwave, sinn, dcam, ankithashetty,... 

Issue #3226117 by MegaChriz, marthinal, longwave, sinn, dcam, ankithashetty, itaran, catch: Uncaught RfcComplianceException when email From name contains a comma
parent d8875803

core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php

+44 −9
Original line number Diff line number Diff line
@@ -33,11 +33,19 @@ class PhpMail implements MailInterface { 
   */

  protected $configFactory;



  /**

   * The currently active request object.

   *

   * @var \Symfony\Component\HttpFoundation\Request

   */

  protected $request;



  /**

   * PhpMail constructor.

   */

  public function __construct() {

    $this->configFactory = \Drupal::configFactory();

    $this->request = \Drupal::request();

  }



  /**
@@ -87,7 +95,9 @@ public function mail(array $message) { 
    $headers = new Headers();

    foreach ($message['headers'] as $name => $value) {

      if (in_array(strtolower($name), self::MAILBOX_LIST_HEADERS, TRUE)) {

        $value = explode(',', $value);

        // Split values by comma, but ignore commas encapsulated in double

        // quotes.

        $value = str_getcsv($value, ',');

      }

      $headers->addHeader($name, $value);

    }
@@ -104,12 +114,7 @@ public function mail(array $message) { 
    $mail_headers = str_replace("\r\n", "\n", $headers->toString());

    $mail_subject = str_replace("\r\n", "\n", $mail_subject);



    $request = \Drupal::request();



    // We suppress warnings and notices from mail() because of issues on some

    // hosts. The return value of this method will still indicate whether mail

    // was sent successfully.

    if (!$request->server->has('WINDIR') && strpos($request->server->get('SERVER_SOFTWARE'), 'Win32') === FALSE) {

    if (!$this->request->server->has('WINDIR') && !str_contains($this->request->server->get('SERVER_SOFTWARE'), 'Win32')) {

      // On most non-Windows systems, the "-f" option to the sendmail command

      // is used to set the Return-Path. There is no space between -f and

      // the value of the return path.
@@ -117,7 +122,7 @@ public function mail(array $message) { 
      // we assume to be safe.

      $site_mail = $this->configFactory->get('system.site')->get('mail');

      $additional_headers = isset($message['Return-Path']) && ($site_mail === $message['Return-Path'] || static::_isShellSafe($message['Return-Path'])) ? '-f' . $message['Return-Path'] : '';

      $mail_result = @mail(

      $mail_result = $this->doMail(

        $message['to'],

        $mail_subject,

        $mail_body,
@@ -130,7 +135,7 @@ public function mail(array $message) { 
      // Return-Path header.

      $old_from = ini_get('sendmail_from');

      ini_set('sendmail_from', $message['Return-Path']);

      $mail_result = @mail(

      $mail_result = $this->doMail(

        $message['to'],

        $mail_subject,

        $mail_body,
@@ -142,6 +147,36 @@ public function mail(array $message) { 
    return $mail_result;

  }



  /**

   * Wrapper around PHP's mail() function.

   *

   * We suppress warnings and notices from mail() because of issues on some

   * hosts. The return value of this method will still indicate whether mail was

   * sent successfully.

   *

   * @param string $to

   *   Receiver, or receivers of the mail.

   * @param string $subject

   *   Subject of the email to be sent.

   * @param string $message

   *   Message to be sent.

   * @param array|string $additional_headers

   *   (optional) String or array to be inserted at the end of the email header.

   * @param string $additional_params

   *   (optional) Can be used to pass additional flags as command line options.

   *

   * @see mail()

   */

  protected function doMail(string $to, string $subject, string $message, array|string $additional_headers = [], string $additional_params = ''): bool {

    return @mail(

      $to,

      $subject,

      $message,

      $additional_headers,

      $additional_params

    );

  }



  /**

   * Disallows potentially unsafe shell characters.

   *

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

+15 −6
Original line number Diff line number Diff line
@@ -40,6 +40,12 @@ protected function setUp(): void { 
    parent::setUp();

    $this->installEntitySchema('user');

    $this->installEntitySchema('file');



    // Set required site configuration.

    $this->config('system.site')

      ->set('mail', 'mailtest@example.com')

      ->set('name', 'Drupal')

      ->save();

  }



  /**
@@ -115,12 +121,6 @@ public function testCancelMessage() { 
  public function testFromAndReplyToHeader() {

    $language = \Drupal::languageManager()->getCurrentLanguage();



    // Set required site configuration.

    $this->config('system.site')

      ->set('mail', 'mailtest@example.com')

      ->set('name', 'Drupal')

      ->save();



    // Reset the state variable that holds sent messages.

    \Drupal::state()->set('system.test_mail_collector', []);

    // Send an email with a reply-to address specified.
@@ -153,6 +153,15 @@ public function testFromAndReplyToHeader() { 
    // Errors-to header must not be set, it is deprecated.

    $this->assertFalse(isset($sent_message['headers']['Errors-To']));



    // Test that From names containing commas work as expected.

    $this->config('system.site')->set('name', 'Foo, Bar, and Baz')->save();

    // Send an email and check that the From-header contains the site name.

    \Drupal::service('plugin.manager.mail')->mail('mail_cancel_test', 'from_test', 'from_test@example.com', $language);

    $captured_emails = \Drupal::state()->get('system.test_mail_collector');

    $sent_message = end($captured_emails);

    // From header contains the quoted site name with commas.

    $this->assertEquals('"Foo, Bar, and Baz" <mailtest@example.com>', $sent_message['headers']['From']);



    // 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

core/modules/user/tests/src/Kernel/UserMailNotifyTest.php

+2 −0
Original line number Diff line number Diff line
@@ -82,6 +82,7 @@ public function userMailsProvider() { 
   */

  public function testUserMailsSent($op, array $mail_keys) {

    $this->installConfig('user');

    $this->config('system.site')->set('mail', 'test@example.com')->save();

    $this->config('user.settings')->set('notify.' . $op, TRUE)->save();

    $return = _user_mail_notify($op, $this->createUser());

    $this->assertTrue($return);
@@ -162,6 +163,7 @@ public function testUserRecoveryMailLanguage() { 


    // Recovery email should respect user preferred langcode by default if

    // langcode not set.

    $this->config('system.site')->set('mail', 'test@example.com')->save();

    $params['account'] = $user;

    $default_email = \Drupal::service('plugin.manager.mail')->mail('user', 'password_reset', $user->getEmail(), $preferredLangcode, $params);

    $this->assertTrue($default_email['result']);

core/tests/Drupal/KernelTests/Core/Action/EmailActionTest.php

+2 −0
Original line number Diff line number Diff line
@@ -31,6 +31,8 @@ protected function setUp(): void { 
   * Tests the email action plugin.

   */

  public function testEmailAction() {

    $this->config('system.site')->set('mail', 'test@example.com')->save();



    /** @var \Drupal\Core\Action\ActionManager $plugin_manager */

    $plugin_manager = $this->container->get('plugin.manager.action');

    $configuration = [

core/tests/Drupal/Tests/Core/Mail/MailManagerTest.php

+23 −0
Original line number Diff line number Diff line
@@ -13,6 +13,8 @@ 
use Drupal\Tests\UnitTestCase;

use Drupal\Core\Mail\MailManager;

use Drupal\Component\Plugin\Discovery\DiscoveryInterface;

use Symfony\Component\HttpFoundation\Request;

use Symfony\Component\HttpFoundation\RequestStack;



/**

 * @coversDefaultClass \Drupal\Core\Mail\MailManager
@@ -62,6 +64,20 @@ class MailManagerTest extends UnitTestCase { 
   */

  protected $mailManager;



  /**

   * The request stack.

   *

   * @var \Symfony\Component\HttpFoundation\RequestStack|\Prophecy\Prophecy\ProphecyInterface

   */

  protected $requestStack;



  /**

   * The current request.

   *

   * @var \Symfony\Component\HttpFoundation\Request

   */

  protected $request;



  /**

   * A list of mail plugin definitions.

   *
@@ -115,9 +131,16 @@ protected function setUpMailManager($interface = []) { 
    $this->mailManager = new TestMailManager(new \ArrayObject(), $this->cache, $this->moduleHandler, $this->configFactory, $logger_factory, $string_translation, $this->renderer);

    $this->mailManager->setDiscovery($this->discovery);



    $this->request = new Request();



    $this->requestStack = $this->prophesize(RequestStack::class);

    $this->requestStack->getCurrentRequest()

      ->willReturn($this->request);



    // @see \Drupal\Core\Plugin\Factory\ContainerFactory::createInstance()

    $container = new ContainerBuilder();

    $container->set('config.factory', $this->configFactory);

    $container->set('request_stack', $this->requestStack->reveal());

    \Drupal::setContainer($container);

  }