Verified Commit 056f7f3f authored by Lee Rowlands's avatar Lee Rowlands
Browse files

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

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

(cherry picked from commit d18b6ccb)
parent 2db48c51
Loading
Loading
Loading
Loading
+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, $additional_headers = [], string $additional_params = ''): bool {
    return @mail(
      $to,
      $subject,
      $message,
      $additional_headers,
      $additional_params
    );
  }

  /**
   * Disallows potentially unsafe shell characters.
   *
+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
+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);
@@ -173,6 +174,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']);
+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 = [
+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);
  }

Loading