Merge pull request #9733 from nextcloud/fix/outbox/handle-missing-raw-message-gracefully

fix(outbox): handle missing raw message gracefully

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) {

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);
+	}
 }