Skip to content

Commit 09a5b4c

Browse files
ChristophWurstChristophWurst
committed
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>
1 parent 40cc7c2 commit 09a5b4c

4 files changed

Lines changed: 144 additions & 0 deletions

File tree

‎lib/IMAP/MessageMapper.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,25 @@ public function save(Horde_Imap_Client_Socket $client,
428428
return (int)$uids->current();
429429
}
430430

431+
/**
432+
* Returns true if a message with the given Message-ID header already exists in $mailbox.
433+
* Used to detect server-side automatic sent-message saving (e.g. Exchange).
434+
*
435+
* @throws Horde_Imap_Client_Exception
436+
*/
437+
public function existsInMailboxByMessageId(
438+
Horde_Imap_Client_Socket $client,
439+
Mailbox $mailbox,
440+
string $messageId,
441+
): bool {
442+
$query = new Horde_Imap_Client_Search_Query();
443+
$query->headerText('Message-ID', $messageId);
444+
$result = $client->search($mailbox->getName(), $query, [
445+
'results' => [Horde_Imap_Client::SEARCH_RESULTS_COUNT],
446+
]);
447+
return ($result['count'] ?? 0) > 0;
448+
}
449+
431450
/**
432451
* @throws Horde_Imap_Client_Exception
433452
*/

‎lib/Send/CopySentMessageHandler.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Horde_Imap_Client_Socket;
1212
use OCA\Mail\Account;
1313
use OCA\Mail\Db\LocalMessage;
14+
use OCA\Mail\Db\Mailbox;
1415
use OCA\Mail\Db\MailboxMapper;
1516
use OCA\Mail\IMAP\MessageMapper;
1617
use OCP\AppFramework\Db\DoesNotExistException;
@@ -24,6 +25,22 @@ public function __construct(
2425
) {
2526
}
2627

28+
private function isAlreadySavedByServer(
29+
Horde_Imap_Client_Socket $client,
30+
Mailbox $mailbox,
31+
string $rawMessage,
32+
): bool {
33+
if (!preg_match('/^Message-ID:\s*(<[^>]+>)/im', $rawMessage, $m)) {
34+
return false;
35+
}
36+
try {
37+
return $this->messageMapper->existsInMailboxByMessageId($client, $mailbox, $m[1]);
38+
} catch (Horde_Imap_Client_Exception $e) {
39+
$this->logger->warning('Could not search for existing sent message, proceeding with APPEND', ['exception' => $e]);
40+
return false;
41+
}
42+
}
43+
2744
#[\Override]
2845
public function process(
2946
Account $account,
@@ -65,6 +82,14 @@ public function process(
6582
return $localMessage;
6683
}
6784

85+
// Some servers (e.g. Exchange) auto-save sent messages, so skip the APPEND when the
86+
// message is already present to avoid duplicates in the Sent folder.
87+
if ($this->isAlreadySavedByServer($client, $sentMailbox, $rawMessage)) {
88+
$this->logger->debug('Sent message already present in sent mailbox (server auto-saved), skipping APPEND');
89+
$localMessage->setStatus(LocalMessage::STATUS_PROCESSED);
90+
return $this->processNext($account, $localMessage, $client);
91+
}
92+
6893
try {
6994
$this->messageMapper->save(
7095
$client,

‎tests/Integration/Service/MailTransmissionIntegrationTest.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,33 @@ public function testSendReplyWithoutReplySubject() {
243243
$this->assertMessageCount(1, 'Sent');
244244
}
245245

246+
public function testSkipsAppendWhenServerAlreadySavedSentMessage(): void {
247+
$messageId = '<server-saved-' . microtime(true) . '@domain.tld>';
248+
$rawMessage = implode("\r\n", [
249+
'From: user@domain.tld',
250+
'To: recipient@domain.com',
251+
'Message-ID: ' . $messageId,
252+
'Subject: server auto-save test',
253+
'',
254+
'Body text',
255+
]);
256+
257+
// Simulate what an Exchange-style server does: save to Sent before the client does
258+
$this->saveMimeMessage('Sent', $rawMessage);
259+
$this->assertMessageCount(1, 'Sent');
260+
261+
$this->message->setRaw($rawMessage);
262+
263+
/** @var IMAPClientFactory $clientFactory */
264+
$clientFactory = Server::get(IMAPClientFactory::class);
265+
$client = $clientFactory->getClient($this->account);
266+
/** @var CopySentMessageHandler $handler */
267+
$handler = Server::get(CopySentMessageHandler::class);
268+
$handler->process($this->account, $this->message, $client);
269+
270+
$this->assertMessageCount(1, 'Sent');
271+
}
272+
246273
public function testSaveNewDraft() {
247274
$message = NewMessageData::fromRequest($this->account, 'greetings', 'hello there', 'recipient@domain.com', null, null, [], false);
248275
[,,$uid] = $this->transmission->saveDraft($message);

‎tests/Unit/Send/CopySendMessageHandlerTest.php‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,79 @@ public function testProcessAlreadyProcessed(): void {
208208
$this->handler->process($account, $mock, $client);
209209
}
210210

211+
public function testProcessSkipsAppendWhenServerAlreadySaved(): void {
212+
$mailAccount = new MailAccount();
213+
$mailAccount->setSentMailboxId(1);
214+
$mailAccount->setUserId('bob');
215+
$account = new Account($mailAccount);
216+
$localMessage = $this->getMockBuilder(LocalMessage::class);
217+
$localMessage->addMethods(['getStatus', 'setStatus', 'getRaw']);
218+
$mock = $localMessage->getMock();
219+
$mailbox = new Mailbox();
220+
$client = $this->createStub(Horde_Imap_Client_Socket::class);
221+
$rawWithMessageId = "Message-ID: <abc@example.com>\r\nSubject: Test\r\n\r\nBody";
222+
223+
$mock->expects(self::once())
224+
->method('getStatus')
225+
->willReturn(LocalMessage::STATUS_RAW);
226+
$mock->expects(self::once())
227+
->method('getRaw')
228+
->willReturn($rawWithMessageId);
229+
$this->mailboxMapper->expects(self::once())
230+
->method('findById')
231+
->willReturn($mailbox);
232+
$this->messageMapper->expects(self::once())
233+
->method('existsInMailboxByMessageId')
234+
->with($client, $mailbox, '<abc@example.com>')
235+
->willReturn(true);
236+
$this->messageMapper->expects(self::never())
237+
->method('save');
238+
$mock->expects(self::once())
239+
->method('setStatus')
240+
->with(LocalMessage::STATUS_PROCESSED);
241+
$this->flagRepliedMessageHandler->expects(self::once())
242+
->method('process');
243+
244+
$this->handler->process($account, $mock, $client);
245+
}
246+
247+
public function testProcessFallsBackToAppendWhenSearchFails(): void {
248+
$mailAccount = new MailAccount();
249+
$mailAccount->setSentMailboxId(1);
250+
$mailAccount->setUserId('bob');
251+
$account = new Account($mailAccount);
252+
$localMessage = $this->getMockBuilder(LocalMessage::class);
253+
$localMessage->addMethods(['getStatus', 'setStatus', 'getRaw']);
254+
$mock = $localMessage->getMock();
255+
$mailbox = new Mailbox();
256+
$client = $this->createStub(Horde_Imap_Client_Socket::class);
257+
$rawWithMessageId = "Message-ID: <xyz@example.com>\r\nSubject: Test\r\n\r\nBody";
258+
259+
$mock->expects(self::once())
260+
->method('getStatus')
261+
->willReturn(LocalMessage::STATUS_RAW);
262+
$mock->expects(self::once())
263+
->method('getRaw')
264+
->willReturn($rawWithMessageId);
265+
$this->mailboxMapper->expects(self::once())
266+
->method('findById')
267+
->willReturn($mailbox);
268+
$this->messageMapper->expects(self::once())
269+
->method('existsInMailboxByMessageId')
270+
->willThrowException(new Horde_Imap_Client_Exception());
271+
$this->loggerInterface->expects(self::once())
272+
->method('warning');
273+
$this->messageMapper->expects(self::once())
274+
->method('save');
275+
$mock->expects(self::once())
276+
->method('setStatus')
277+
->with(LocalMessage::STATUS_PROCESSED);
278+
$this->flagRepliedMessageHandler->expects(self::once())
279+
->method('process');
280+
281+
$this->handler->process($account, $mock, $client);
282+
}
283+
211284
public function testProcessNoRawMessage(): void {
212285
$mailAccount = new MailAccount();
213286
$mailAccount->setSentMailboxId(1);

0 commit comments

Comments
 (0)