diff --git a/src/Command/ReclassifyAttachmentsCommand.php b/src/Command/ReclassifyAttachmentsCommand.php new file mode 100644 index 00000000..12ec0c31 --- /dev/null +++ b/src/Command/ReclassifyAttachmentsCommand.php @@ -0,0 +1,141 @@ +addOption('dry-run', null, InputOption::VALUE_NONE, 'Report changes without writing them'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $dryRun = true === $input->getOption('dry-run'); + + $messageIds = array_column( + $this->em->createQuery( + 'SELECT DISTINCT IDENTITY(p.message) AS id FROM ' . MessagePart::class . ' p' + )->getArrayResult(), + 'id', + ); + + $io->text(sprintf('%d messages with parts to inspect.', count($messageIds))); + + $changedParts = 0; + $changedMessages = 0; + $processed = 0; + + foreach (array_chunk($messageIds, self::BATCH_SIZE) as $chunk) { + /** @var list $messages */ + $messages = $this->em->getRepository(Message::class)->findBy(['id' => $chunk]); + + foreach ($messages as $message) { + $bodyHtml = $message->getBodyHtml(); + $hasAttachments = false; + $touched = false; + + foreach ($message->getMessageParts() as $part) { + $isInline = $this->inlineDetector->isInline( + $part->getDisposition(), + $part->getContentId(), + $bodyHtml, + ); + + if ($isInline !== $part->isInline()) { + $part->setIsInline($isInline); + $part->setDisposition($isInline ? 'inline' : 'attachment'); + ++$changedParts; + $touched = true; + } + + if (false === $isInline) { + $hasAttachments = true; + } + } + + if ($hasAttachments !== $message->hasAttachments()) { + $message->setHasAttachments($hasAttachments); + $touched = true; + } + + if (true === $touched) { + ++$changedMessages; + } + } + + $processed += count($messages); + + if (false === $dryRun) { + $this->em->flush(); + } + + $this->em->clear(); + $io->text(sprintf(' … %d/%d', $processed, count($messageIds))); + } + + if (false === $dryRun) { + $this->recountThreads(); + } + + $io->success(sprintf( + '%s%d parts reclassified across %d messages.', + $dryRun ? '[dry-run] ' : '', + $changedParts, + $changedMessages, + )); + + return Command::SUCCESS; + } + + /** + * Thread counters are derived, so rebuild them from the repaired messages + * instead of trying to patch the deltas. + */ + private function recountThreads(): void + { + $this->em->getConnection()->executeStatement(<<<'SQL' + UPDATE message_thread t + SET attachment_count = COALESCE(( + SELECT COUNT(*) FROM message m + WHERE m.thread_id = t.id AND m.has_attachments = true + ), 0) + SQL); + } +} diff --git a/src/Service/Gmail/GmailMessageBuilder.php b/src/Service/Gmail/GmailMessageBuilder.php index fc568922..3d9aaef3 100644 --- a/src/Service/Gmail/GmailMessageBuilder.php +++ b/src/Service/Gmail/GmailMessageBuilder.php @@ -9,6 +9,7 @@ use App\Entity\Label; use App\Entity\Message; use App\Entity\MessagePart; use App\Service\Label\LabelResolver; +use App\Service\Mail\InlineAttachmentDetector; use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; @@ -38,6 +39,7 @@ final class GmailMessageBuilder private readonly EntityManagerInterface $em, private readonly GmailLabelResolver $labelResolver, private readonly LabelResolver $localLabelResolver, + private readonly InlineAttachmentDetector $inlineDetector, ) { } @@ -137,14 +139,18 @@ final class GmailMessageBuilder // ── Body + attachments ──────────────────────────────────────────────── - [$bodyText, $bodyHtml, $hasAttachments] = $this->extractBody( + // Attachment parts are collected first and persisted afterwards: the + // inline/attachment decision needs the HTML body, which the same walk + // is still assembling. + [$bodyText, $bodyHtml, $attachmentParts] = $this->extractBody( $payload['payload'] ?? [], - $message, ); $message->setBodyText($bodyText); $message->setBodyHtml($bodyHtml); - $message->setHasAttachments($hasAttachments); + $message->setHasAttachments( + $this->persistAttachmentStubs($attachmentParts, $message, $bodyHtml) + ); $message->setSyncedAt(new DateTimeImmutable()); return $message; @@ -210,16 +216,13 @@ final class GmailMessageBuilder * Walk the MIME tree and extract text/html body parts and attachments. * * @param array $part - * @return array{string, string, bool} [bodyText, bodyHtml, hasAttachments] + * @return array{string, string, list>} [bodyText, bodyHtml, attachmentParts] */ - private function extractBody( - array $part, - Message $message, - ): array + private function extractBody(array $part): array { $bodyText = ''; $bodyHtml = ''; - $hasAttachments = false; + $attachmentParts = []; $mimeType = strtolower((string)($part['mimeType'] ?? '')); @@ -239,65 +242,68 @@ final class GmailMessageBuilder $hasContentId = '' !== trim(($partHeaders['content-id'] ?? ''), '<> '); if ('' !== $filename || true === $hasContentId) { - $isInline = $this->persistAttachmentStub($part, $message); - - if (false === $isInline) { - $hasAttachments = true; - } + $attachmentParts[] = $part; } } foreach ($part['parts'] ?? [] as $subPart) { - [$t, $h, $a] = $this->extractBody($subPart, $message); + [$t, $h, $a] = $this->extractBody($subPart); if ('' === $bodyText) { $bodyText = $t; } if ('' === $bodyHtml) { $bodyHtml = $h; } - if (true === $a) { + + $attachmentParts = array_merge($attachmentParts, $a); + } + + return [$bodyText, $bodyHtml, $attachmentParts]; + } + + /** + * Persist MessagePart stubs for the collected attachment parts. Bytes are + * fetched lazily by AttachmentResolver on first access. + * + * @param list> $parts + * @return bool true if at least one part is a real (non-inline) attachment + */ + private function persistAttachmentStubs(array $parts, Message $message, string $bodyHtml): bool + { + $hasAttachments = false; + + foreach ($parts as $part) { + $partHeaders = $this->indexHeaders($part['headers'] ?? []); + $filename = (string)($part['filename'] ?? 'attachment'); + $contentType = (string)($part['mimeType'] ?? 'application/octet-stream'); + $attachmentId = (string)($part['body']['attachmentId'] ?? ''); + $size = (int)($part['body']['size'] ?? 0); + + $contentId = $this->inlineDetector->normalizeContentId($partHeaders['content-id'] ?? null); + $isInline = $this->inlineDetector->isInline( + $partHeaders['content-disposition'] ?? null, + $contentId, + $bodyHtml, + ); + + $mp = new MessagePart() + ->setMessage($message) + ->setContentType($contentType) + ->setFilename($filename) + ->setContentId('' !== $contentId ? $contentId : null) + ->setDisposition($isInline ? 'inline' : 'attachment') + ->setSize($size) + ->setStoragePath('gmail://' . $attachmentId) + ->setIsInline($isInline); + + $this->em->persist($mp); + + if (false === $isInline) { $hasAttachments = true; } } - return [$bodyText, $bodyHtml, $hasAttachments]; - } - - /** - * Persist a MessagePart stub for an attachment. Bytes are fetched lazily by - * AttachmentResolver on first access. - * - * @param array $part - * @return bool true if the part is inline (referenced from the body) - */ - private function persistAttachmentStub( - array $part, - Message $message, - ): bool - { - $partHeaders = $this->indexHeaders($part['headers'] ?? []); - $filename = (string)($part['filename'] ?? 'attachment'); - $contentType = (string)($part['mimeType'] ?? 'application/octet-stream'); - $attachmentId = (string)($part['body']['attachmentId'] ?? ''); - $size = (int)($part['body']['size'] ?? 0); - - $contentId = trim($partHeaders['content-id'] ?? '', '<> '); - $disposition = strtolower($partHeaders['content-disposition'] ?? ''); - $isInline = true === str_contains($disposition, 'inline') || '' !== $contentId; - - $mp = new MessagePart() - ->setMessage($message) - ->setContentType($contentType) - ->setFilename($filename) - ->setContentId('' !== $contentId ? $contentId : null) - ->setDisposition($isInline ? 'inline' : 'attachment') - ->setSize($size) - ->setStoragePath('gmail://' . $attachmentId) - ->setIsInline($isInline); - - $this->em->persist($mp); - - return $isInline; + return $hasAttachments; } private function decodeMimeHeader(string $value): string diff --git a/src/Service/Graph/GraphMessageBuilder.php b/src/Service/Graph/GraphMessageBuilder.php index aa931de2..da4181cd 100644 --- a/src/Service/Graph/GraphMessageBuilder.php +++ b/src/Service/Graph/GraphMessageBuilder.php @@ -7,6 +7,7 @@ namespace App\Service\Graph; use App\Entity\Account; use App\Entity\Message; use App\Entity\MessagePart; +use App\Service\Mail\InlineAttachmentDetector; use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; @@ -46,6 +47,7 @@ final class GraphMessageBuilder public function __construct( private readonly EntityManagerInterface $em, private readonly GraphFolderResolver $folderResolver, + private readonly InlineAttachmentDetector $inlineDetector, ) {} /** @@ -210,8 +212,12 @@ final class GraphMessageBuilder continue; } - $contentId = trim((string) ($attachment['contentId'] ?? ''), '<> '); - $isInline = true === ($attachment['isInline'] ?? false) || '' !== $contentId; + $contentId = $this->inlineDetector->normalizeContentId($attachment['contentId'] ?? null); + $isInline = $this->inlineDetector->isInline( + true === ($attachment['isInline'] ?? false) ? 'inline' : 'attachment', + $contentId, + $message->getBodyHtml(), + ); $part = new MessagePart() ->setMessage($message) diff --git a/src/Service/Imap/MessageSyncer.php b/src/Service/Imap/MessageSyncer.php index ce8fa16c..d2dbf928 100644 --- a/src/Service/Imap/MessageSyncer.php +++ b/src/Service/Imap/MessageSyncer.php @@ -12,6 +12,7 @@ use App\Jmap\State\StateManager; use App\Repository\ContactRepository; use App\Repository\MailboxRepository; use App\Repository\MessageRepository; +use App\Service\Mail\InlineAttachmentDetector; use App\Service\Mail\MailBodySanitizer; use App\Service\Mail\MessageCategorizer; use App\Service\Mail\RawMessageResolver; @@ -37,6 +38,7 @@ class MessageSyncer private readonly ContactRepository $contactRepository, private readonly StateManager $stateManager, private readonly RawMessageResolver $rawResolver, + private readonly InlineAttachmentDetector $inlineDetector, ) {} public function syncMailbox(Mailbox $mailbox, Client $client): void @@ -335,18 +337,26 @@ class MessageSyncer // Attachments $attachments = $imapMessage->getAttachments(); - $message->setHasAttachments($attachments->isNotEmpty()); $message->setSyncedAt(new DateTimeImmutable()); + $hasAttachments = false; + foreach ($attachments as $attachment) { - $this->persistAttachment($attachment, $message, $accountId); + if (false === $this->persistAttachment($attachment, $message, $accountId)) { + $hasAttachments = true; + } } + $message->setHasAttachments($hasAttachments); + return $message; } - private function persistAttachment(mixed $attachment, Message $message, int $accountId): void + /** + * @return bool true if the part is inline (embedded in the HTML body) + */ + private function persistAttachment(mixed $attachment, Message $message, int $accountId): bool { $filename = $attachment->getFilename() ?? ('attachment_' . uniqid()); $content = $attachment->getContent(); @@ -363,19 +373,25 @@ class MessageSyncer $part->setMessage($message); $part->setContentType($attachment->getContentType() ?? 'application/octet-stream'); $part->setFilename($filename); - $part->setDisposition($attachment->getDisposition() ?? 'attachment'); $part->setSize(strlen($content)); $part->setStoragePath($storagePath); - $contentId = $attachment->getId(); - $normalizedCid = null !== $contentId ? trim((string) $contentId, '<> ') : ''; - $part->setContentId('' !== $normalizedCid ? $normalizedCid : null); - - $part->setIsInline( - ($attachment->getDisposition() ?? '') === 'inline' || '' !== $normalizedCid + // getId() falls back to a content hash when the part has no Content-ID, + // so it only counts as a cid when the HTML body actually references it. + $normalizedCid = $this->inlineDetector->normalizeContentId((string) $attachment->getId()); + $isInline = $this->inlineDetector->isInline( + $attachment->getDisposition(), + $normalizedCid, + $message->getBodyHtml(), ); + $part->setContentId('' !== $normalizedCid ? $normalizedCid : null); + $part->setDisposition($isInline ? 'inline' : 'attachment'); + $part->setIsInline($isInline); + $this->em->persist($part); + + return $isInline; } private function formatAddresses(mixed $attribute): array diff --git a/src/Service/Mail/InlineAttachmentDetector.php b/src/Service/Mail/InlineAttachmentDetector.php new file mode 100644 index 00000000..de9b468e --- /dev/null +++ b/src/Service/Mail/InlineAttachmentDetector.php @@ -0,0 +1,61 @@ +" on every attachment it uploads, and Webklex's + * Attachment::getId() falls back to a content hash when the part carries no + * Content-ID at all. Treating "has a cid" as "is inline" therefore hides + * ordinary attachments from the UI. + * + * The body is the authority: a part is inline when the HTML actually + * references its cid. Content-Disposition only decides the cases the body + * cannot (no HTML part, or a cid the body never mentions). + */ +final class InlineAttachmentDetector +{ + public function isInline( + ?string $disposition, + ?string $contentId, + ?string $bodyHtml, + ): bool { + $contentId = $this->normalizeContentId($contentId); + + if (true === $this->isReferencedBy($contentId, (string) $bodyHtml)) { + return true; + } + + $disposition = strtolower(trim((string) $disposition)); + + if (true === str_contains($disposition, 'attachment')) { + return false; + } + + return str_contains($disposition, 'inline'); + } + + public function normalizeContentId(?string $contentId): string + { + return trim((string) $contentId, "<> \t\r\n"); + } + + /** + * True when the HTML body embeds this part via a cid: URL. + */ + private function isReferencedBy(string $contentId, string $bodyHtml): bool + { + if ('' === $contentId || '' === $bodyHtml) { + return false; + } + + return 1 === preg_match( + '/cid:\s*' . preg_quote($contentId, '/') . '\b/i', + $bodyHtml, + ); + } +} diff --git a/templates/_partials/_thread_row.html.twig b/templates/_partials/_thread_row.html.twig index d0933260..387790f1 100644 --- a/templates/_partials/_thread_row.html.twig +++ b/templates/_partials/_thread_row.html.twig @@ -34,6 +34,8 @@ inside the reading pane. {% set draft = latest and latest.isDraft and item.messageCount == 1 %} {% set draftId = draft ? latest.id : null %} + {% set hasAttachment = item.attachmentCount > 0 %} + {% set href = path('app_mail_thread', { id: item.id }) %} {% set rowId = item.id %} {% else %} @@ -46,6 +48,8 @@ inside the reading pane. {% set sender = item.fromName ?: item.fromAddress %} {% set snippet = (item.bodyText ?? '')|striptags|slice(0, 100) %} + {% set hasAttachment = item.hasAttachments %} + {% set draft = item.isDraft %} {% set draftId = item.id %} @@ -109,7 +113,6 @@ inside the reading pane. {{ sender }} - {# Date — mobile only (top-right), hidden on desktop #} {% if date %} @@ -135,13 +138,19 @@ inside the reading pane. — {{ snippet }} - {# Date + hover actions slot — desktop only #}