diff --git a/lib/IMAP/MessageMapper.php b/lib/IMAP/MessageMapper.php index f63c28f9b6..66a62e5a69 100644 --- a/lib/IMAP/MessageMapper.php +++ b/lib/IMAP/MessageMapper.php @@ -428,6 +428,25 @@ public function save(Horde_Imap_Client_Socket $client, return (int)$uids->current(); } + /** + * Returns true if a message with the given Message-ID header already exists in $mailbox. + * Used to detect server-side automatic sent-message saving (e.g. Exchange). + * + * @throws Horde_Imap_Client_Exception + */ + public function existsInMailboxByMessageId( + Horde_Imap_Client_Socket $client, + Mailbox $mailbox, + string $messageId, + ): bool { + $query = new Horde_Imap_Client_Search_Query(); + $query->headerText('Message-ID', $messageId); + $result = $client->search($mailbox->getName(), $query, [ + 'results' => [Horde_Imap_Client::SEARCH_RESULTS_COUNT], + ]); + return ($result['count'] ?? 0) > 0; + } + /** * @throws Horde_Imap_Client_Exception */ diff --git a/lib/Send/CopySentMessageHandler.php b/lib/Send/CopySentMessageHandler.php index dffe3b08b9..778f99c269 100644 --- a/lib/Send/CopySentMessageHandler.php +++ b/lib/Send/CopySentMessageHandler.php @@ -10,10 +10,13 @@ use Horde_Imap_Client_Exception; use Horde_Imap_Client_Socket; use OCA\Mail\Account; +use OCA\Mail\AppInfo\Application; use OCA\Mail\Db\LocalMessage; +use OCA\Mail\Db\Mailbox; use OCA\Mail\Db\MailboxMapper; use OCA\Mail\IMAP\MessageMapper; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IConfig; use Psr\Log\LoggerInterface; class CopySentMessageHandler extends AHandler { @@ -21,9 +24,27 @@ public function __construct( private MailboxMapper $mailboxMapper, private LoggerInterface $logger, private MessageMapper $messageMapper, + private IConfig $config, ) { } + private function isAlreadySavedByServer( + Horde_Imap_Client_Socket $client, + Mailbox $mailbox, + string $rawMessage, + ): bool { + [$headers] = preg_split("/\r?\n\r?\n/", $rawMessage, 2); + if (!preg_match('/^Message-ID:\s*(<[^>\r\n]+>)/im', $headers, $m)) { + return false; + } + try { + return $this->messageMapper->existsInMailboxByMessageId($client, $mailbox, $m[1]); + } catch (Horde_Imap_Client_Exception $e) { + $this->logger->warning('Could not search for existing sent message, proceeding with APPEND', ['exception' => $e]); + return false; + } + } + #[\Override] public function process( Account $account, @@ -65,6 +86,16 @@ public function process( return $localMessage; } + // Some servers (e.g. Exchange) auto-save sent messages, so skip the APPEND when the + // message is already present to avoid duplicates in the Sent folder. + // Opt-in via: occ config:app:set mail smtp_saves_sent --value=true + if ($this->config->getAppValue(Application::APP_ID, 'smtp_saves_sent', 'false') === 'true' + && $this->isAlreadySavedByServer($client, $sentMailbox, $rawMessage)) { + $this->logger->debug('Sent message already present in sent mailbox (server auto-saved), skipping APPEND'); + $localMessage->setStatus(LocalMessage::STATUS_PROCESSED); + return $this->processNext($account, $localMessage, $client); + } + try { $this->messageMapper->save( $client, diff --git a/tests/Integration/Service/MailTransmissionIntegrationTest.php b/tests/Integration/Service/MailTransmissionIntegrationTest.php index ca124f63d4..ea448dfbc6 100644 --- a/tests/Integration/Service/MailTransmissionIntegrationTest.php +++ b/tests/Integration/Service/MailTransmissionIntegrationTest.php @@ -42,6 +42,7 @@ use OCA\Mail\Tests\Integration\Framework\ImapTest; use OCA\Mail\Tests\Integration\TestCase; use OCP\EventDispatcher\IEventDispatcher; +use OCP\IConfig; use OCP\IUser; use OCP\Security\ICrypto; use OCP\Server; @@ -243,6 +244,38 @@ public function testSendReplyWithoutReplySubject() { $this->assertMessageCount(1, 'Sent'); } + public function testSkipsAppendWhenServerAlreadySavedSentMessage(): void { + $messageId = ''; + $rawMessage = implode("\r\n", [ + 'From: user@domain.tld', + 'To: recipient@domain.com', + 'Message-ID: ' . $messageId, + 'Subject: server auto-save test', + '', + 'Body text', + ]); + + // Enable the opt-in feature flag + Server::get(IConfig::class)->setAppValue('mail', 'smtp_saves_sent', 'true'); + + // Simulate what an Exchange-style server does: save to Sent before the client does + $this->saveMimeMessage('Sent', $rawMessage); + $this->assertMessageCount(1, 'Sent'); + + $this->message->setRaw($rawMessage); + + /** @var IMAPClientFactory $clientFactory */ + $clientFactory = Server::get(IMAPClientFactory::class); + $client = $clientFactory->getClient($this->account); + /** @var CopySentMessageHandler $handler */ + $handler = Server::get(CopySentMessageHandler::class); + $handler->process($this->account, $this->message, $client); + + Server::get(IConfig::class)->deleteAppValue('mail', 'smtp_saves_sent'); + + $this->assertMessageCount(1, 'Sent'); + } + public function testSaveNewDraft() { $message = NewMessageData::fromRequest($this->account, 'greetings', 'hello there', 'recipient@domain.com', null, null, [], false); [,,$uid] = $this->transmission->saveDraft($message); diff --git a/tests/Unit/Send/CopySendMessageHandlerTest.php b/tests/Unit/Send/CopySendMessageHandlerTest.php index 9e193bbf60..a72e56a53f 100644 --- a/tests/Unit/Send/CopySendMessageHandlerTest.php +++ b/tests/Unit/Send/CopySendMessageHandlerTest.php @@ -19,6 +19,7 @@ use OCA\Mail\Send\CopySentMessageHandler; use OCA\Mail\Send\FlagRepliedMessageHandler; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IConfig; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -27,6 +28,7 @@ class CopySendMessageHandlerTest extends TestCase { private LoggerInterface|MockObject $loggerInterface; private MockObject|MessageMapper $messageMapper; private MockObject|FlagRepliedMessageHandler $flagRepliedMessageHandler; + private IConfig|MockObject $config; private CopySentMessageHandler $handler; protected function setUp(): void { @@ -35,10 +37,13 @@ protected function setUp(): void { $this->loggerInterface = $this->createMock(LoggerInterface::class); $this->messageMapper = $this->createMock(MessageMapper::class); $this->flagRepliedMessageHandler = $this->createMock(FlagRepliedMessageHandler::class); + $this->config = $this->createMock(IConfig::class); + $this->config->method('getAppValue')->willReturn('false'); $this->handler = new CopySentMessageHandler( $this->mailboxMapper, $this->loggerInterface, $this->messageMapper, + $this->config, ); $this->handler->setNext($this->flagRepliedMessageHandler); } @@ -208,6 +213,83 @@ public function testProcessAlreadyProcessed(): void { $this->handler->process($account, $mock, $client); } + public function testProcessSkipsAppendWhenServerAlreadySaved(): 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(); + $mailbox = new Mailbox(); + $client = $this->createStub(Horde_Imap_Client_Socket::class); + $rawWithMessageId = "Message-ID: \r\nSubject: Test\r\n\r\nBody"; + + $this->config->method('getAppValue')->willReturn('true'); + + $mock->expects(self::once()) + ->method('getStatus') + ->willReturn(LocalMessage::STATUS_RAW); + $mock->expects(self::once()) + ->method('getRaw') + ->willReturn($rawWithMessageId); + $this->mailboxMapper->expects(self::once()) + ->method('findById') + ->willReturn($mailbox); + $this->messageMapper->expects(self::once()) + ->method('existsInMailboxByMessageId') + ->with($client, $mailbox, '') + ->willReturn(true); + $this->messageMapper->expects(self::never()) + ->method('save'); + $mock->expects(self::once()) + ->method('setStatus') + ->with(LocalMessage::STATUS_PROCESSED); + $this->flagRepliedMessageHandler->expects(self::once()) + ->method('process'); + + $this->handler->process($account, $mock, $client); + } + + public function testProcessFallsBackToAppendWhenSearchFails(): 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(); + $mailbox = new Mailbox(); + $client = $this->createStub(Horde_Imap_Client_Socket::class); + $rawWithMessageId = "Message-ID: \r\nSubject: Test\r\n\r\nBody"; + + $this->config->method('getAppValue')->willReturn('true'); + + $mock->expects(self::once()) + ->method('getStatus') + ->willReturn(LocalMessage::STATUS_RAW); + $mock->expects(self::once()) + ->method('getRaw') + ->willReturn($rawWithMessageId); + $this->mailboxMapper->expects(self::once()) + ->method('findById') + ->willReturn($mailbox); + $this->messageMapper->expects(self::once()) + ->method('existsInMailboxByMessageId') + ->willThrowException(new Horde_Imap_Client_Exception()); + $this->loggerInterface->expects(self::once()) + ->method('warning'); + $this->messageMapper->expects(self::once()) + ->method('save'); + $mock->expects(self::once()) + ->method('setStatus') + ->with(LocalMessage::STATUS_PROCESSED); + $this->flagRepliedMessageHandler->expects(self::once()) + ->method('process'); + + $this->handler->process($account, $mock, $client); + } + public function testProcessNoRawMessage(): void { $mailAccount = new MailAccount(); $mailAccount->setSentMailboxId(1);