Merge pull request #9242 from nupplaphil/bug/9142-message-id

Transform email header string to header array

include/enotify.php

@@ -87,12 +87,15 @@ function notification($params)
 	}
 	$nickname = $user["nickname"];
 
+	// Creates a new email builder for the notification email
+	$emailBuilder = DI::emailer()->newNotifyMail();
+
 	// with $params['show_in_notification_page'] == false, the notification isn't inserted into
 	// the database, and an email is sent if applicable.
 	// default, if not specified: true
 	$show_in_notification_page = isset($params['show_in_notification_page']) ? $params['show_in_notification_page'] : true;
 
-	$additional_mail_header = "X-Friendica-Account: <".$nickname."@".$hostname.">\n";
+	$emailBuilder->setHeader('X-Friendica-Account', '<' . $nickname . '@' . $hostname . '>');
 
 	if (array_key_exists('item', $params)) {
 		$title = $params['item']['title'];
@@ -521,13 +524,14 @@ function notification($params)
 					'receiver-uid' => $params['uid'], 'parent-item' => 0];
 				DBA::insert('notify-threads', $fields);
 
-				$additional_mail_header .= "Message-ID: " . $message_id . "\n";
+				$emailBuilder->setHeader('Message-ID', $message_id);
 				$log_msg                = "include/enotify: No previous notification found for this parent:\n" .
 				                          "  parent: ${params['parent']}\n" . "  uid   : ${params['uid']}\n";
 				Logger::log($log_msg, Logger::DEBUG);
 			} else {
 				// If not, just "follow" the thread.
-				$additional_mail_header .= "References: " . $message_id . "\nIn-Reply-To: " . $message_id . "\n";
+				$emailBuilder->setHeader('References', $message_id);
+				$emailBuilder->setHeader('In-Reply-To', $message_id);
 				Logger::log("There's already a notification for this parent.", Logger::DEBUG);
 			}
 		}
@@ -546,7 +550,6 @@ function notification($params)
 			'title'        => $title,
 			'body'         => $body,
 			'subject'      => $subject,
-			'headers'      => $additional_mail_header,
 		];
 
 		Hook::callAll('enotify_mail', $datarray);
@@ -565,13 +568,13 @@ function notification($params)
 
 		// If a photo is present, add it to the email
 		if (!empty($datarray['source_photo'])) {
-			$builder->withPhoto(
+			$emailBuilder->withPhoto(
 				$datarray['source_photo'],
 				$datarray['source_link'] ?? $sitelink,
 				$datarray['source_name'] ?? $sitename);
 		}
 
-		$email = $builder->build();
+		$email = $emailBuilder->build();
 
 		// use the Emailer class to send the message
 		return DI::emailer()->send($email);

src/Object/EMail/IEmail.php

@@ -83,11 +83,18 @@ interface IEmail extends JsonSerializable
 	function getMessage(bool $plain = false);
 
 	/**
-	 * Gets any additional mail header
+	 * Gets the additional mail header array
+	 *
+	 * @return string[][]
+	 */
+	function getAdditionalMailHeader();
+
+	/**
+	 * Gets the additional mail header as string - EOL separated
 	 *
 	 * @return string
 	 */
-	function getAdditionalMailHeader();
+	function getAdditionalMailHeaderString();
 
 	/**
 	 * Returns the current email with a new recipient

src/Object/Email.php

@@ -47,14 +47,14 @@ class Email implements IEmail
 	/** @var string */
 	private $msgText;
 
-	/** @var string */
+	/** @var string[][] */
-	private $additionalMailHeader = '';
+	private $additionalMailHeader;
 	/** @var int|null */
-	private $toUid = null;
+	private $toUid;
 
 	public function __construct(string $fromName, string $fromAddress, string $replyTo, string $toAddress,
 	                            string $subject, string $msgHtml, string $msgText,
-	                            string $additionalMailHeader = '', int $toUid = null)
+	                            array $additionalMailHeader = [], int $toUid = null)
 	{
 		$this->fromName             = $fromName;
 		$this->fromAddress          = $fromAddress;
@@ -127,6 +127,25 @@ class Email implements IEmail
 		return $this->additionalMailHeader;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public function getAdditionalMailHeaderString()
+	{
+		$headerString = '';
+
+		foreach ($this->additionalMailHeader as $name => $values) {
+			if (is_array($values)) {
+				foreach ($values as $value) {
+					$headerString .= $name . ': ' . $value . '\n';
+				}
+			} else {
+				$headerString .= $name . ': ' . $values . '\n';
+			}
+		}
+		return $headerString;
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */

src/Util/EMailer/MailBuilder.php

@@ -49,7 +49,7 @@ abstract class MailBuilder
 	/** @var LoggerInterface */
 	protected $logger;
 
-	/** @var string */
+	/** @var string[][] */
 	protected $headers;
 
 	/** @var string */
@@ -76,13 +76,14 @@ abstract class MailBuilder
 			$hostname = substr($hostname, 0, strpos($hostname, ':'));
 		}
 
-		$this->headers = "";
+		$this->headers = [
-		$this->headers .= "Precedence: list\n";
+			'Precedence'           => ['list'],
-		$this->headers .= "X-Friendica-Host: " . $hostname . "\n";
+			'X-Friendica-Host'     => [$hostname],
-		$this->headers .= "X-Friendica-Platform: " . FRIENDICA_PLATFORM . "\n";
+			'X-Friendica-Platform' => [FRIENDICA_PLATFORM],
-		$this->headers .= "X-Friendica-Version: " . FRIENDICA_VERSION . "\n";
+			'X-Friendica-Version'  => [FRIENDICA_VERSION],
-		$this->headers .= "List-ID: <notification." . $hostname . ">\n";
+			'List-ID'              => ['<notification.' . $hostname . '>'],
-		$this->headers .= "List-Archive: <" . $baseUrl->get() . "/notifications/system>\n";
+			'List-Archive'         => ['<' . $baseUrl->get() . '/notifications/system>'],
+		];
 	}
 
 	/**
@@ -159,15 +160,31 @@ abstract class MailBuilder
 	}
 
 	/**
-	 * Adds new headers to the default headers
+	 * Adds a value to a header
 	 *
-	 * @param string $headers New headers
+	 * @param string $name The header name
+	 * @param string $value The value of the header to add
 	 *
 	 * @return static
 	 */
-	public function addHeaders(string $headers)
+	public function addHeader(string $name, string $value)
 	{
-		$this->headers .= $headers;
+		$this->headers[$name][] = $value;
+
+		return $this;
+	}
+
+	/**
+	 * Sets a value to a header (overwrites existing values)
+	 *
+	 * @param string $name The header name
+	 * @param string $value The value to set
+	 *
+	 * @return static
+	 */
+	public function	setHeader(string $name, string $value)
+	{
+		$this->headers[$name] = [$value];
 
 		return $this;
 	}

src/Util/Emailer.php

@@ -151,7 +151,7 @@ class Emailer
 		                . rand(10000, 99999);
 
 		// generate a multipart/alternative message header
-		$messageHeader = $email->getAdditionalMailHeader() .
+		$messageHeader = $email->getAdditionalMailHeaderString() .
 		                 "From: $fromName <{$fromAddress}>\n" .
 		                 "Reply-To: $fromName <{$replyTo}>\n" .
 		                 "MIME-Version: 1.0\n" .

tests/src/Util/Emailer/MailBuilderTest.php

@@ -61,7 +61,7 @@ class MailBuilderTest extends MockedTest
 		$this->baseUrl->shouldReceive('getHostname')->andReturn('friendica.local');
 		$this->baseUrl->shouldReceive('get')->andReturn('http://friendica.local');
 
-		$this->defaultHeaders = "";
+		$this->defaultHeaders = [];
 	}
 
 	public function assertEmail(IEmail $email, array $asserts)

tests/src/Util/Emailer/SystemMailBuilderTest.php

@@ -41,7 +41,7 @@ class SystemMailBuilderTest extends MockedTest
 	/** @var BaseURL */
 	private $baseUrl;
 
-	/** @var string */
+	/** @var string[] */
 	private $defaultHeaders;
 
 	public function setUp()
@@ -60,7 +60,7 @@ class SystemMailBuilderTest extends MockedTest
 		$this->baseUrl->shouldReceive('getHostname')->andReturn('friendica.local');
 		$this->baseUrl->shouldReceive('get')->andReturn('http://friendica.local');
 
-		$this->defaultHeaders = "";
+		$this->defaultHeaders = [];
 	}
 
 	/**