From 831f7992f683669cbdd4d017a8f2cc8d39915606 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Mon, 20 Feb 2023 10:49:24 +1000
Subject: [PATCH] Issue #3226117 by MegaChriz, marthinal, longwave, rschwab,
sinn, dcam, cilefen, ankithashetty, itaran, larowlan, catch: [Needs backport]
Uncaught RfcComplianceException when email From name contains a comma
---
.../Drupal/Core/Mail/Plugin/Mail/PhpMail.php | 45 +++++++--
.../system/tests/src/Kernel/Mail/MailTest.php | 21 ++--
.../tests/src/Kernel/UserMailNotifyTest.php | 2 +
.../Core/Action/EmailActionTest.php | 2 +
.../Core/Mail/Plugin/Mail/PhpMailTest.php | 96 +++++++++++++++++++
5 files changed, 151 insertions(+), 15 deletions(-)
create mode 100644 core/tests/Drupal/Tests/Core/Mail/Plugin/Mail/PhpMailTest.php
diff --git a/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php b/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
index 138257b8da48..cae954c49e00 100644
--- a/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
+++ b/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
@@ -87,7 +87,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 +106,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 (substr(PHP_OS, 0, 3) != 'WIN') {
// 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 +114,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 +127,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 +139,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 $additional_headers
+ * (optional) 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.
*
diff --git a/core/modules/system/tests/src/Kernel/Mail/MailTest.php b/core/modules/system/tests/src/Kernel/Mail/MailTest.php
index 1eed7c500777..76167feaa394 100644
--- a/core/modules/system/tests/src/Kernel/Mail/MailTest.php
+++ b/core/modules/system/tests/src/Kernel/Mail/MailTest.php
@@ -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
diff --git a/core/modules/user/tests/src/Kernel/UserMailNotifyTest.php b/core/modules/user/tests/src/Kernel/UserMailNotifyTest.php
index 2038d18c0ec0..a8ac6824847b 100644
--- a/core/modules/user/tests/src/Kernel/UserMailNotifyTest.php
+++ b/core/modules/user/tests/src/Kernel/UserMailNotifyTest.php
@@ -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']);
diff --git a/core/tests/Drupal/KernelTests/Core/Action/EmailActionTest.php b/core/tests/Drupal/KernelTests/Core/Action/EmailActionTest.php
index 81d8aad8956e..f71fcd34a7fc 100644
--- a/core/tests/Drupal/KernelTests/Core/Action/EmailActionTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Action/EmailActionTest.php
@@ -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 = [
diff --git a/core/tests/Drupal/Tests/Core/Mail/Plugin/Mail/PhpMailTest.php b/core/tests/Drupal/Tests/Core/Mail/Plugin/Mail/PhpMailTest.php
new file mode 100644
index 000000000000..bdf97280cc0e
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Mail/Plugin/Mail/PhpMailTest.php
@@ -0,0 +1,96 @@
+<?php
+
+namespace Drupal\Tests\Core\Mail\Plugin\Mail;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Mail\Plugin\Mail\PhpMail;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Mail\Plugin\Mail\PhpMail
+ * @group Mail
+ */
+class PhpMailTest extends UnitTestCase {
+
+ /**
+ * The configuration factory.
+ *
+ * @var \Drupal\Core\Config\ConfigFactoryInterface|\PHPUnit\Framework\MockObject\MockObject
+ */
+ protected $configFactory;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ // Use the provided config for system.mail.interface settings.
+ $this->configFactory = $this->getConfigFactoryStub([
+ 'system.mail' => [
+ 'interface' => [],
+ ],
+ 'system.site' => [
+ 'mail' => 'test@example.com',
+ ],
+ ]);
+
+ $container = new ContainerBuilder();
+ $container->set('config.factory', $this->configFactory);
+ \Drupal::setContainer($container);
+ }
+
+ /**
+ * Creates a mocked PhpMail object.
+ *
+ * The method "doMail()" gets overridden to avoid a mail() call in tests.
+ *
+ * @return \Drupal\Core\Mail\Plugin\Mail\PhpMail|\PHPUnit\Framework\MockObject\MockObject
+ * A PhpMail instance.
+ */
+ protected function createPhpMailInstance(): PhpMail {
+ $mailer = $this->getMockBuilder(PhpMail::class)
+ ->onlyMethods(['doMail'])
+ ->getMock();
+
+ $mailer->expects($this->once())->method('doMail')
+ ->willReturn(TRUE);
+
+ return $mailer;
+ }
+
+ /**
+ * Tests sending a mail using a From address with a comma in it.
+ *
+ * @covers ::testMail
+ */
+ public function testMail() {
+ // Setup a mail message.
+ $message = [
+ 'id' => 'example_key',
+ 'module' => 'example',
+ 'key' => 'key',
+ 'to' => 'to@example.org',
+ 'from' => 'from@example.org',
+ 'reply-to' => 'from@example.org',
+ 'langcode' => 'en',
+ 'params' => [],
+ 'send' => TRUE,
+ 'subject' => '',
+ 'body' => '',
+ 'headers' => [
+ 'MIME-Version' => '1.0',
+ 'Content-Type' => 'text/plain; charset=UTF-8; format=flowed; delsp=yes',
+ 'Content-Transfer-Encoding' => '8Bit',
+ 'X-Mailer' => 'Drupal',
+ 'Return-Path' => 'from@example.org',
+ 'From' => '"Foo, Bar, and Baz" <from@example.org>',
+ 'Reply-to' => 'from@example.org',
+ ],
+ ];
+
+ $mailer = $this->createPhpMailInstance();
+ $this->assertTrue($mailer->mail($message));
+ }
+
+}
--
GitLab