fixup! fix(MailTransmission): Handle clients/servers picky about empty headers

Author: kesselb

lib/Service/MailTransmission.php

@@ -14,6 +14,8 @@
 use Horde_Imap_Client_Data_Fetch;
 use Horde_Imap_Client_Fetch_Query;
 use Horde_Imap_Client_Ids;
+use Horde_Mail_Rfc822_Address;
+use Horde_Mail_Rfc822_List;
 use Horde_Mail_Transport_Null;
 use Horde_Mail_Transport_Smtphorde;
 use Horde_Mime_Exception;
@@ -42,7 +44,6 @@
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\IMAP\IMAPClientFactory;
 use OCA\Mail\IMAP\MessageMapper;
-use OCA\Mail\Model\Message as ModelMessage;
 use OCA\Mail\Model\NewMessageData;
 use OCA\Mail\Service\DataUri\DataUriParser;
 use OCA\Mail\SMTP\SmtpClientFactory;
@@ -107,13 +108,13 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void
 
 		$transport = $this->smtpClientFactory->create($account);
 		// build mime body
-		$headers = [
-			'From' => $from->toHorde(),
-			'To' => $to->toHorde(),
-			'Cc' => $cc->toHorde(),
-			'Bcc' => $bcc->toHorde(),
-			'Subject' => $localMessage->getSubject(),
-		];
+		$headers = $this->buildHeaders(
+			$from,
+			$to,
+			$cc,
+			$bcc,
+			$localMessage->getSubject()
+		);
 
 		// The table (oc_local_messages) currently only allows for a single reply to message id
 		// but we already set the 'references' header for an email so we could support multiple references
@@ -127,11 +128,6 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void
 			$headers[Horde_Mime_Mdn::MDN_HEADER] = $from->toHorde();
 		}
 
-		// Exclude headers where the value is empty, null, or just whitespace (to avoid confusing picky servers/clients)
-		$headers = array_filter($headers, function($value) {
-			return !empty($value) && (is_string($value) ? trim($value) !== '' : true);
-		});
-		
 		$mail = new Horde_Mime_Mail();
 		$mail->addHeaders($headers);
 
@@ -202,41 +198,27 @@ public function saveLocalDraft(Account $account, LocalMessage $message): void {
 
 		$perfLogger = $this->performanceLogger->start('save local draft');
 
-		$imapMessage = new ModelMessage();
-		$imapMessage->setTo($to);
-		$imapMessage->setSubject($message->getSubject());
-		$from = new AddressList([
-			Address::fromRaw($account->getName(), $account->getEMailAddress()),
-		]);
-		$imapMessage->setFrom($from);
-		$imapMessage->setCC($cc);
-		$imapMessage->setBcc($bcc);
-		if ($message->isHtml() === true) {
-			$imapMessage->setContent($message->getBodyHtml());
-		} else {
-			$imapMessage->setContent($message->getBodyPlain());
-		}
+		$from = Address::fromRaw($account->getName(), $account->getEMailAddress());
 
 		foreach ($attachments as $attachment) {
 			$this->transmissionService->handleAttachment($account, $attachment);
 		}
 
 		// build mime body
-		$headers = [
-			'From' => $imapMessage->getFrom()->first()->toHorde(),
-			'To' => $imapMessage->getTo()->toHorde(),
-			'Cc' => $imapMessage->getCC()->toHorde(),
-			'Bcc' => $imapMessage->getBCC()->toHorde(),
-			'Subject' => $imapMessage->getSubject(),
-			'Date' => Horde_Mime_Headers_Date::create(),
-		];
+		$headers = $this->buildHeaders(
+			$from,
+			$to,
+			$cc,
+			$bcc,
+			$message->getSubject()
+		);
 
 		$mail = new Horde_Mime_Mail();
 		$mail->addHeaders($headers);
 		if ($message->isHtml()) {
-			$mail->setHtmlBody($imapMessage->getContent());
+			$mail->setHtmlBody($message->getBodyHtml());
 		} else {
-			$mail->setBody($imapMessage->getContent());
+			$mail->setBody($message->getBodyPlain());
 		}
 		$mail->addHeaderOb(Horde_Mime_Headers_MessageId::create());
 		$perfLogger->step('build local draft message');
@@ -302,33 +284,22 @@ public function saveDraft(NewMessageData $message, ?Message $previousDraft = nul
 		$perfLogger->step('emit pre event');
 
 		$account = $message->getAccount();
-		$imapMessage = new ModelMessage();
-		$imapMessage->setTo($message->getTo());
-		$imapMessage->setSubject($message->getSubject());
-		$from = new AddressList([
-			Address::fromRaw($account->getName(), $account->getEMailAddress()),
-		]);
-		$imapMessage->setFrom($from);
-		$imapMessage->setCC($message->getCc());
-		$imapMessage->setBcc($message->getBcc());
-		$imapMessage->setContent($message->getBody());
-
-		// build mime body
-		$headers = [
-			'From' => $imapMessage->getFrom()->first()->toHorde(),
-			'To' => $imapMessage->getTo()->toHorde(),
-			'Cc' => $imapMessage->getCC()->toHorde(),
-			'Bcc' => $imapMessage->getBCC()->toHorde(),
-			'Subject' => $imapMessage->getSubject(),
-			'Date' => Horde_Mime_Headers_Date::create(),
-		];
+		$from = Address::fromRaw($account->getName(), $account->getEMailAddress());
+
+		$headers = $this->buildHeaders(
+			$from,
+			$message->getTo(),
+			$message->getCc(),
+			$message->getBcc(),
+			$message->getSubject()
+		);
 
 		$mail = new Horde_Mime_Mail();
 		$mail->addHeaders($headers);
 		if ($message->isHtml()) {
-			$mail->setHtmlBody($imapMessage->getContent());
+			$mail->setHtmlBody($message->getBody());
 		} else {
-			$mail->setBody($imapMessage->getContent());
+			$mail->setBody($message->getBody());
 		}
 		$mail->addHeaderOb(Horde_Mime_Headers_MessageId::create());
 		$perfLogger->step('build draft message');
@@ -370,6 +341,32 @@ public function saveDraft(NewMessageData $message, ?Message $previousDraft = nul
 		return [$account, $draftsMailbox, $newUid];
 	}
 
+	/**
+	 * @return array{
+	 *     From: Horde_Mail_Rfc822_Address,
+	 *     To: Horde_Mail_Rfc822_List,
+	 *     Subject: string|null,
+	 *     Cc?: Horde_Mail_Rfc822_List,
+	 *     Bcc?: Horde_Mail_Rfc822_List,
+	 * }
+	 */
+	private function buildHeaders(Address $from, AddressList $to, AddressList $cc, AddressList $bcc, ?string $subject): array {
+		$headers = [
+			'From' => $from->toHorde(),
+			'To' => $to->toHorde(),
+			'Subject' => $subject,
+		];
+
+		if (count($cc) > 0) {
+			$headers['Cc'] = $cc->toHorde();
+		}
+		if (count($bcc) > 0) {
+			$headers['Bcc'] = $bcc->toHorde();
+		}
+
+		return $headers;
+	}
+
 	#[\Override]
 	public function sendMdn(Account $account, Mailbox $mailbox, Message $message): void {
 		$query = new Horde_Imap_Client_Fetch_Query();