Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/IMAP/MessageMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
26 changes: 26 additions & 0 deletions lib/Send/CopySentMessageHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,6 +25,23 @@ public function __construct(
) {
}

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,
Expand Down Expand Up @@ -65,6 +83,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,
Expand Down
27 changes: 27 additions & 0 deletions tests/Integration/Service/MailTransmissionIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,33 @@ public function testSendReplyWithoutReplySubject() {
$this->assertMessageCount(1, 'Sent');
}

public function testSkipsAppendWhenServerAlreadySavedSentMessage(): void {
$messageId = '<server-saved-' . microtime(true) . '@domain.tld>';
$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);
Expand Down
73 changes: 73 additions & 0 deletions tests/Unit/Send/CopySendMessageHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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: <abc@example.com>\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, '<abc@example.com>')
->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: <xyz@example.com>\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);
Expand Down
Loading