From 09a5b4cc52f484346715da8ee6ade71a8273ba49 Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Wed, 13 May 2026 11:24:52 +0200 Subject: [PATCH 1/2] fix(send): skip IMAP APPEND when server already saved sent message Some mail servers (e.g. Microsoft Exchange/Office 365) automatically save a copy of sent messages to the Sent folder upon SMTP submission. When Nextcloud Mail also does an IMAP APPEND, the user ends up with duplicates. After SMTP delivery succeeds, search the Sent mailbox for the outgoing message's Message-ID header. If the server already saved it, skip the APPEND. If the IMAP search fails for any reason, fall through and APPEND as normal (safe degradation). AI-assisted: Claude Code (claude-sonnet-4-6) Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/IMAP/MessageMapper.php | 19 +++++ lib/Send/CopySentMessageHandler.php | 25 +++++++ .../MailTransmissionIntegrationTest.php | 27 +++++++ .../Unit/Send/CopySendMessageHandlerTest.php | 73 +++++++++++++++++++ 4 files changed, 144 insertions(+) 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..10c95c423d 100644 --- a/lib/Send/CopySentMessageHandler.php +++ b/lib/Send/CopySentMessageHandler.php @@ -11,6 +11,7 @@ use Horde_Imap_Client_Socket; use OCA\Mail\Account; 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; @@ -24,6 +25,22 @@ public function __construct( ) { } + private function isAlreadySavedByServer( + Horde_Imap_Client_Socket $client, + Mailbox $mailbox, + string $rawMessage, + ): bool { + if (!preg_match('/^Message-ID:\s*(<[^>]+>)/im', $rawMessage, $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 +82,14 @@ 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. + if ($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..0ead880439 100644 --- a/tests/Integration/Service/MailTransmissionIntegrationTest.php +++ b/tests/Integration/Service/MailTransmissionIntegrationTest.php @@ -243,6 +243,33 @@ 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', + ]); + + // 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); + + $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..ef198c6ab4 100644 --- a/tests/Unit/Send/CopySendMessageHandlerTest.php +++ b/tests/Unit/Send/CopySendMessageHandlerTest.php @@ -208,6 +208,79 @@ 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"; + + $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"; + + $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); From 8475dea98086f9d242d7dfcbea663d93b3e15aa9 Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Wed, 13 May 2026 12:22:43 +0200 Subject: [PATCH 2/2] fixup! fix(send): skip IMAP APPEND when server already saved sent message Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/Send/CopySentMessageHandler.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Send/CopySentMessageHandler.php b/lib/Send/CopySentMessageHandler.php index 10c95c423d..06f7d2e27a 100644 --- a/lib/Send/CopySentMessageHandler.php +++ b/lib/Send/CopySentMessageHandler.php @@ -30,7 +30,8 @@ private function isAlreadySavedByServer( Mailbox $mailbox, string $rawMessage, ): bool { - if (!preg_match('/^Message-ID:\s*(<[^>]+>)/im', $rawMessage, $m)) { + [$headers] = preg_split("/\r?\n\r?\n/", $rawMessage, 2); + if (!preg_match('/^Message-ID:\s*(<[^>\r\n]+>)/im', $headers, $m)) { return false; } try {