From a1ccd6778156cdfce09c52c06a1a1ef6303451bd Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Wed, 6 May 2026 18:06:05 +0200 Subject: [PATCH] perf(imap): fetch only required header fields during sync Replace full BODY.PEEK[HEADER] with BODY.PEEK[HEADER.FIELDS (...)] in findByIds(), downloading only the 5 headers actually needed by parseHeaders(). Reduces header data per message 10-30x, cutting initial sync time from minutes to seconds on slow IMAP connections and preventing web request timeouts. AI-assisted: Claude Code (claude-sonnet-4-6) Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/IMAP/ImapMessageFetcher.php | 16 +++-- lib/IMAP/MessageMapper.php | 13 ++-- .../ImapMessageFetcherIntegrationTest.php | 63 +++++++++++++++++++ 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/lib/IMAP/ImapMessageFetcher.php b/lib/IMAP/ImapMessageFetcher.php index 9ab62207bf..6041a089dd 100644 --- a/lib/IMAP/ImapMessageFetcher.php +++ b/lib/IMAP/ImapMessageFetcher.php @@ -530,10 +530,18 @@ private function decodeSubject(Horde_Imap_Client_Data_Envelope $envelope): strin } private function parseHeaders(Horde_Imap_Client_Data_Fetch $fetch): void { - /** @var resource $headersStream */ - $headersStream = $fetch->getHeaderText('0', Horde_Imap_Client_Data_Fetch::HEADER_STREAM); - $parsedHeaders = Horde_Mime_Headers::parseHeaders($headersStream); - fclose($headersStream); + // Prefer the targeted HEADER.FIELDS fetch used by the sync path (BODY.PEEK[HEADER.FIELDS (...)]). + // Fall back to the full header text when loadBody=true re-fetches with headerText(). + $parsedHeaders = $fetch->getHeaders('syncFields', Horde_Imap_Client_Data_Fetch::HEADER_PARSE); + if ($parsedHeaders === null) { + /** @var resource $headersStream */ + $headersStream = $fetch->getHeaderText('0', Horde_Imap_Client_Data_Fetch::HEADER_STREAM); + if ($headersStream === null) { + return; + } + $parsedHeaders = Horde_Mime_Headers::parseHeaders($headersStream); + fclose($headersStream); + } $references = $parsedHeaders->getHeader('references'); if ($references !== null) { diff --git a/lib/IMAP/MessageMapper.php b/lib/IMAP/MessageMapper.php index 483c783998..73b05d39bc 100644 --- a/lib/IMAP/MessageMapper.php +++ b/lib/IMAP/MessageMapper.php @@ -282,11 +282,16 @@ public function findByIds(Horde_Imap_Client_Base $client, $query->flags(); $query->uid(); $query->imapDate(); - $query->headerText( + $query->headers( + 'syncFields', [ - 'cache' => true, - 'peek' => true, - ] + 'references', + 'disposition-notification-to', + 'dkim-signature', + 'list-unsubscribe', + 'list-unsubscribe-post', + ], + ['peek' => true], ); if (is_array($ids)) { diff --git a/tests/Integration/IMAP/ImapMessageFetcherIntegrationTest.php b/tests/Integration/IMAP/ImapMessageFetcherIntegrationTest.php index 5dfa9e7c0f..a2a5c68f5b 100644 --- a/tests/Integration/IMAP/ImapMessageFetcherIntegrationTest.php +++ b/tests/Integration/IMAP/ImapMessageFetcherIntegrationTest.php @@ -9,10 +9,12 @@ namespace OCA\Mail\Tests\Integration\IMAP; +use Horde_Imap_Client_Ids; use OCA\Mail\Db\MailAccount; use OCA\Mail\Db\SmimeCertificate; use OCA\Mail\Db\SmimeCertificateMapper; use OCA\Mail\IMAP\ImapMessageFetcherFactory; +use OCA\Mail\IMAP\MessageMapper as ImapMessageMapper; use OCA\Mail\Tests\Integration\Framework\ImapTest; use OCA\Mail\Tests\Integration\Framework\ImapTestAccount; use OCA\Mail\Tests\Integration\TestCase; @@ -187,4 +189,65 @@ public function testFetchMessageWithOpaqueSignedMessage(): void { $this->assertTrue($message->isSigned()); $this->assertTrue($message->isSignatureValid()); } + + /** + * Verifies that all five header fields parsed during the loadBody=false sync path + * are correctly extracted. This is the regression guard for switching from + * BODY.PEEK[HEADER] (full headers) to BODY.PEEK[HEADER.FIELDS (...)] (selective). + */ + public function testSyncPathParsesAllRequiredHeaderFields(): void { + // Append raw bytes directly to avoid Horde_Mime_Mail rewriting the headers + $rawMime = implode("\r\n", [ + 'From: sender@test.invalid', + 'To: recipient@test.invalid', + 'Subject: Header fields test', + 'Message-ID: ', + 'References: ', + 'Disposition-Notification-To: sender@test.invalid', + 'DKIM-Signature: v=1; a=rsa-sha256; d=test.invalid; s=default;', + ' h=from:to:subject;', + ' bh=47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=;', + ' b=ZXhhbXBsZXNpZ25hdHVyZQ==', + 'List-Unsubscribe: ', + 'List-Unsubscribe-Post: List-Unsubscribe=One-Click', + 'Content-Type: text/plain; charset=utf-8', + '', + 'Test body', + ]); + + $appendClient = $this->getClient(null); + $uid = $appendClient->append('INBOX', [['data' => $rawMime]])->ids[0]; + $appendClient->logout(); + + /** @var ImapMessageMapper $mapper */ + $mapper = Server::get(ImapMessageMapper::class); + $fetchClient = $this->getClient($this->account); + try { + $messages = $mapper->findByIds( + $fetchClient, + 'INBOX', + new Horde_Imap_Client_Ids([$uid]), + $this->account->getUserId(), + ); + } finally { + $fetchClient->logout(); + } + + $this->assertCount(1, $messages); + $message = $messages[0]; + + // disposition-notification-to + $this->assertSame('sender@test.invalid', $message->getDispositionNotificationTo()); + + // dkim-signature (no public getter — check via serialization) + $this->assertTrue($message->jsonSerialize()['hasDkimSignature']); + + // list-unsubscribe + list-unsubscribe-post + $this->assertSame('https://test.invalid/unsubscribe', $message->getUnsubscribeUrl()); + $this->assertTrue($message->isOneClickUnsubscribe()); + + // references — flows through toDbMessage() which parses IDs from the raw value + $dbMessage = $message->toDbMessage(1, $this->account); + $this->assertStringContainsString('parent@test.invalid', $dbMessage->getReferences() ?? ''); + } }