From 66d5e602ac207a908a0931cb66abdc5c64ed2b3d Mon Sep 17 00:00:00 2001
From: Richard Steinmetz <richard@steinmetz.cloud>
Date: Wed, 12 Jun 2024 08:28:38 +0200
Subject: [PATCH] fix(outbox): handle missing raw message gracefully

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
---
 lib/Send/CopySentMessageHandler.php           |  9 +++-
 .../Unit/Send/CopySendMessageHandlerTest.php  | 41 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/lib/Send/CopySentMessageHandler.php b/lib/Send/CopySentMessageHandler.php
index 8a26005de..f335b4727 100644
--- a/lib/Send/CopySentMessageHandler.php
+++ b/lib/Send/CopySentMessageHandler.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
  * @copyright 2024 Anna Larch <anna.larch@gmx.net>
  *
  * @author Anna Larch <anna.larch@gmx.net>
+ * @author 2024 Richard Steinmetz <richard@steinmetz.cloud>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -43,6 +44,12 @@ class CopySentMessageHandler extends AHandler {
 			return $this->processNext($account, $localMessage);
 		}
 
+		$rawMesage = $localMessage->getRaw();
+		if ($rawMesage === null) {
+			$localMessage->setStatus(LocalMessage::STATUS_IMAP_SENT_MAILBOX_FAIL);
+			return $localMessage;
+		}
+
 		$sentMailboxId = $account->getMailAccount()->getSentMailboxId();
 		if ($sentMailboxId === null) {
 			// We can't write the "sent mailbox" status here bc that would trigger an additional send.
@@ -73,7 +80,7 @@ class CopySentMessageHandler extends AHandler {
 			$this->messageMapper->save(
 				$client,
 				$sentMailbox,
-				$localMessage->getRaw()
+				$rawMesage,
 			);
 			$localMessage->setStatus(LocalMessage::STATUS_PROCESSED);
 		} catch (Horde_Imap_Client_Exception $e) {
diff --git a/tests/Unit/Send/CopySendMessageHandlerTest.php b/tests/Unit/Send/CopySendMessageHandlerTest.php
index 6794e0d99..8f8c0418f 100644
--- a/tests/Unit/Send/CopySendMessageHandlerTest.php
+++ b/tests/Unit/Send/CopySendMessageHandlerTest.php
@@ -107,7 +107,7 @@ class CopySendMessageHandlerTest extends TestCase {
 		$mailAccount->setUserId('bob');
 		$account = new Account($mailAccount);
 		$localMessage = $this->getMockBuilder(LocalMessage::class);
-		$localMessage->addMethods(['getStatus','setStatus']);
+		$localMessage->addMethods(['getStatus', 'setStatus', 'getRaw']);
 		$mock = $localMessage->getMock();
 
 		$this->loggerInterface->expects(self::once())
@@ -115,6 +115,9 @@ class CopySendMessageHandlerTest extends TestCase {
 		$mock->expects(self::once())
 			->method('getStatus')
 			->willReturn(LocalMessage::STATUS_RAW);
+		$mock->expects(self::once())
+			->method('getRaw')
+			->willReturn('Test');
 		$mock->expects(self::once())
 			->method('setStatus')
 			->with(LocalMessage::STATUS_IMAP_SENT_MAILBOX_FAIL);
@@ -138,7 +141,7 @@ class CopySendMessageHandlerTest extends TestCase {
 		$mailAccount->setSentMailboxId(1);
 		$account = new Account($mailAccount);
 		$localMessage = $this->getMockBuilder(LocalMessage::class);
-		$localMessage->addMethods(['getStatus','setStatus']);
+		$localMessage->addMethods(['getStatus', 'setStatus', 'getRaw']);
 		$mock = $localMessage->getMock();
 
 		$this->loggerInterface->expects(self::never())
@@ -146,6 +149,9 @@ class CopySendMessageHandlerTest extends TestCase {
 		$mock->expects(self::once())
 			->method('getStatus')
 			->willReturn(LocalMessage::STATUS_RAW);
+		$mock->expects(self::once())
+			->method('getRaw')
+			->willReturn('Test');
 		$mock->expects(self::once())
 			->method('setStatus')
 			->with(LocalMessage::STATUS_IMAP_SENT_MAILBOX_FAIL);
@@ -229,4 +235,35 @@ class CopySendMessageHandlerTest extends TestCase {
 
 		$this->handler->process($account, $mock);
 	}
+
+	public function testProcessNoRawMessage(): void {
+		$mailAccount = new MailAccount();
+		$mailAccount->setSentMailboxId(1);
+		$mailAccount->setUserId('bob');
+		$account = new Account($mailAccount);
+		$localMessage = $this->getMockBuilder(LocalMessage::class);
+		$localMessage->addMethods(['getStatus','setStatus', 'getRaw']);
+		$mock = $localMessage->getMock();
+
+		$mock->expects(self::once())
+			->method('getStatus')
+			->willReturn(LocalMessage::STATUS_RAW);
+		$mock->expects(self::once())
+			->method('getRaw')
+			->willReturn(null);
+		$mock->expects(self::once())
+			->method('setStatus')
+			->willReturn(LocalMessage::STATUS_IMAP_SENT_MAILBOX_FAIL);
+		$this->mailboxMapper->expects(self::never())
+			->method('findById');
+		$this->imapClientFactory->expects(self::never())
+			->method('getClient');
+		$this->messageMapper->expects(self::never())
+			->method('save');
+		$this->flagRepliedMessageHandler->expects(self::never())
+			->method('process');
+
+		$result = $this->handler->process($account, $mock);
+		$this->assertEquals($mock, $result);
+	}
 }