mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Drop a header block the server repeats at the top of the body (#177)
Closes #173. Co-authored-by: Philippe Delodder <7259755+phdelodder@users.noreply.github.com>
This commit is contained in:
1 parent
dda54a3876
commit
cbe14ea5ae
11 files changed
+842
-2
No files matched your search
@@ -32,6 +32,15 @@ gets an hour, but the hour stops at midnight. Events already synced are correcte
|
||||
next changes them, or right away by removing the calendar and adding it again.
|
||||
[#170](https://github.com/karatektus/pl_mail/issues/170).
|
||||
|
||||
**Some IMAP messages no longer show their own headers above the content.** A Stalwart server
|
||||
answers some messages' body with the whole message, headers included, and plMail stored that as the
|
||||
body: the message opened with `Delivered-To:`, `Received:` and the rest of its header block printed
|
||||
as text, and its original held the headers twice. An exact repeat of the header block at the top of
|
||||
the body is now dropped before the message is read. Mail already stored that way is repaired by
|
||||
`app:backfill repeated-headers`, from the stored original and without asking the server; it reads
|
||||
the original of every IMAP message once. No migration.
|
||||
[#173](https://github.com/karatektus/pl_mail/issues/173), by @phdelodder.
|
||||
|
||||
## v0.3.6 — 2026-10-16
|
||||
|
||||
**Several Gmail accounts import without failing on a duplicate key.** With more than one Gmail
|
||||
|
||||
+1
-1
@@ -918,7 +918,7 @@ php bin/mirror-wiki.php --check
|
||||
| `app:mail:wake-snoozed` | Return snoozed conversations whose time has come. Runs every minute |
|
||||
| `app:mail:release-held` | Show mail that has been held for the assistant longer than **Wait at most** allows. Runs every minute and finds nothing on an ordinary day — the live worker releases held mail itself. It is the backstop for that worker being down, and it warns when it has to act |
|
||||
| `app:calendar:materialise [--dry-run]` | Redraw the occurrences of recurring events whose horizon no longer reaches far enough. Runs nightly; without it a long-untouched series eventually runs out of dates |
|
||||
| `app:backfill [task]` | Run a one-off backfill over stored data; with no argument it lists the tasks and asks. The tasks that re-read mail (`events`, `insights`) cover the last 24 hours unless `--since` says otherwise (`7d`, `2026-09-01`, `all`). `events` re-runs calendar extraction, `proposals` re-reads mail for dates written in prose, `safe-html-table-links` restores links that were wrapped around a table and came out empty, `safe-html-conditional-comments` removes Outlook's `<![endif]-->` from messages that show it as text |
|
||||
| `app:backfill [task]` | Run a one-off backfill over stored data; with no argument it lists the tasks and asks. The tasks that re-read mail (`events`, `insights`) cover the last 24 hours unless `--since` says otherwise (`7d`, `2026-09-01`, `all`). `events` re-runs calendar extraction, `proposals` re-reads mail for dates written in prose, `safe-html-table-links` restores links that were wrapped around a table and came out empty, `safe-html-conditional-comments` removes Outlook's `<![endif]-->` from messages that show it as text, `repeated-headers` takes out the header block some IMAP servers repeated at the top of a message body |
|
||||
| `app:imap:idle <mailbox-id>` | Hold an IMAP IDLE connection for a single mailbox |
|
||||
| `app:imap:supervise` | Spawn and watch one `app:imap:idle` process per IDLE-enabled mailbox |
|
||||
| `app:imap:test [--account=ID]` | Test an IMAP connection and folder listing |
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<!-- translated-from: providers/imap-smtp.md sha1:90f0e331560ca2cd562d56cfc01609f1a52cb275 -->
|
||||
<!-- translated-from: providers/imap-smtp.md sha1:5335652d9a9068ad87eafb41deda04a22278acba -->
|
||||
|
||||
# IMAP und SMTP
|
||||
|
||||
@@ -172,3 +172,11 @@ markiert ihn mit `\Noselect` und lässt sich nicht darauf ein, ihn zu öffnen; p
|
||||
mit ausgeschalteter Synchronisation an und fragt ihn nie ab. Die Ordner darunter sind gewöhnlich und
|
||||
werden wie üblich synchronisiert. Ein Ordner, den der Server später auswählbar macht, wird nicht von
|
||||
selbst wieder eingeschaltet.
|
||||
|
||||
**Manche Server schicken die Kopfzeilen einer Nachricht als Anfang ihres Inhalts noch einmal.** Bei
|
||||
einem Stalwart-Server wurde das bei einigen Nachrichten beobachtet, bei anderen nicht. plMail erkennt
|
||||
eine exakte Wiederholung des Kopfblocks und lässt sie weg, sodass die Nachricht einmal angezeigt und
|
||||
gespeichert wird. Mail, die vorher synchronisiert wurde, zeigt ihre Zeilen `Delivered-To:` und
|
||||
`Received:` über dem Inhalt; `app:backfill repeated-headers` repariert sie aus dem gespeicherten
|
||||
Original, ohne den Server zu fragen. Es liest dafür das Original jeder IMAP-Nachricht einmal, auf
|
||||
einem großen Postfach dauert das also eine Weile.
|
||||
@@ -159,3 +159,10 @@ mark it read here.
|
||||
Mail, Sent Mail, Drafts and the rest. Gmail marks it `\Noselect` and refuses to open it, so plMail
|
||||
creates it with sync switched off and never polls it; the folders underneath are ordinary and sync as
|
||||
usual. A folder the server later makes selectable is not switched back on by itself.
|
||||
|
||||
**Some servers send a message's headers again as the start of its body.** A Stalwart server has
|
||||
been seen doing this for some messages and not others. plMail recognises an exact repeat of the
|
||||
header block and drops it, so the message is shown and stored once. Mail synced before that was
|
||||
fixed shows its `Delivered-To:` and `Received:` lines above the content; `app:backfill
|
||||
repeated-headers` repairs it from the stored original, without asking the server. It reads every
|
||||
IMAP message's original once, so on a large mailbox it takes a while.
|
||||
@@ -0,0 +1,184 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Command\Backfill;
|
||||
|
||||
use App\Domain\Helper\CharsetHelper;
|
||||
use App\Domain\Helper\ImapConnectionFactory;
|
||||
use App\Domain\Helper\RawMessageStorage;
|
||||
use App\Domain\Helper\RepeatedHeaderHelper;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Repository\Mail\MessageRepository;
|
||||
use App\Service\Mail\MailBodySanitizer;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Symfony\Component\Console\Command\Command;
|
||||
use Symfony\Component\Console\Style\SymfonyStyle;
|
||||
use Throwable;
|
||||
use Webklex\PHPIMAP\IMAP;
|
||||
use Webklex\PHPIMAP\Message as ImapMessage;
|
||||
|
||||
/**
|
||||
* Takes the repeated header block out of IMAP mail stored before
|
||||
* MessageSyncer learned to drop it (#173).
|
||||
*
|
||||
* Those messages open with their own `Delivered-To: …` block above the
|
||||
* content, and their .eml holds the headers twice. MessageSyncer stops new
|
||||
* ones arriving; this is for the ones already here.
|
||||
*
|
||||
* ── Repairable from the .eml, which is why it exists ─────────────────────────
|
||||
* The sync wrote "header, blank line, what the server sent as the body", so
|
||||
* the file is the header literal followed by the doubled body, and splitting
|
||||
* it at the first blank line gives back exactly the pair the sync had.
|
||||
* RepeatedHeaderHelper decides the same way it does at ingest. A re-sync would
|
||||
* not help: IMAP skips UIDs it already has.
|
||||
*
|
||||
* ── What is rewritten, and what is left alone ────────────────────────────────
|
||||
* The .eml, always: it is meant to be the message, once.
|
||||
*
|
||||
* The bodies, ONLY for a single-part message. There the whole doubled text was
|
||||
* the one part, so body_html or body_text is the header block plus the body,
|
||||
* and a fresh parse of the repaired file is the right answer. A multipart
|
||||
* message is different: its parts start after the boundary, so the bodies
|
||||
* were parsed correctly, and they may hold a repair the sync made
|
||||
* (MisfiledBodyDetector) that a plain re-parse would not repeat. Overwriting
|
||||
* them would trade a correct body for a possibly worse one.
|
||||
*
|
||||
* Attachments are not touched. A single-part message has none, and a
|
||||
* multipart one's were parsed from the parts, like its bodies.
|
||||
*
|
||||
* ── Every IMAP message with an .eml is read ──────────────────────────────────
|
||||
* The doubled block is invisible in the database for a multipart message, so
|
||||
* there is no column to select the affected ones by. One file read per
|
||||
* message, once, is the price; a backfill rather than an upgrade task so an
|
||||
* administrator chooses when to pay it.
|
||||
*
|
||||
* Idempotent: a repaired file no longer starts its body with its header
|
||||
* block, so the second run changes nothing.
|
||||
*/
|
||||
final readonly class RepeatedHeaderBackfillTask implements BackfillTaskInterface
|
||||
{
|
||||
private const int BATCH_SIZE = 100;
|
||||
|
||||
public function __construct(
|
||||
private MessageRepository $messages,
|
||||
private RawMessageStorage $rawStorage,
|
||||
private MailBodySanitizer $sanitizer,
|
||||
private EntityManagerInterface $em,
|
||||
private LoggerInterface $logger,
|
||||
) {}
|
||||
|
||||
public function getName(): string
|
||||
{
|
||||
return 'repeated-headers';
|
||||
}
|
||||
|
||||
public function getDescription(): string
|
||||
{
|
||||
return 'Remove the header block some IMAP servers repeated at the top of a message body, from stored mail and its original.';
|
||||
}
|
||||
|
||||
public function run(SymfonyStyle $io): int
|
||||
{
|
||||
$total = $this->messages->countImapWithStoredRaw();
|
||||
|
||||
if (0 === $total) {
|
||||
$io->success('Nothing to read — no IMAP message has its original stored.');
|
||||
|
||||
return Command::SUCCESS;
|
||||
}
|
||||
|
||||
$io->progressStart($total);
|
||||
|
||||
$lastId = 0;
|
||||
$repaired = 0;
|
||||
|
||||
while (true) {
|
||||
$messages = $this->messages->findImapWithStoredRaw($lastId, self::BATCH_SIZE);
|
||||
|
||||
if (count($messages) === 0) {
|
||||
break;
|
||||
}
|
||||
|
||||
foreach ($messages as $message) {
|
||||
$lastId = (int) $message->id;
|
||||
|
||||
if (true === $this->repair($message)) {
|
||||
++$repaired;
|
||||
}
|
||||
|
||||
$io->progressAdvance();
|
||||
}
|
||||
|
||||
$this->em->flush();
|
||||
$this->em->clear();
|
||||
}
|
||||
|
||||
$io->progressFinish();
|
||||
$io->success(sprintf('%d message(s) repaired, %d read.', $repaired, $total));
|
||||
|
||||
return Command::SUCCESS;
|
||||
}
|
||||
|
||||
// ── Private ───────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* @return bool true when the message carried a repeated header block
|
||||
*/
|
||||
private function repair(Message $message): bool
|
||||
{
|
||||
$path = (string) $message->rawPath;
|
||||
|
||||
try {
|
||||
if (false === $this->rawStorage->exists($path)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$raw = (string) file_get_contents($this->rawStorage->getAbsolutePath($path));
|
||||
|
||||
// MessageSyncer::rawOf() joins the halves with exactly this.
|
||||
$split = strpos($raw, "\r\n\r\n");
|
||||
|
||||
if (false === $split) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$header = substr($raw, 0, $split);
|
||||
$body = RepeatedHeaderHelper::strip($header, substr($raw, $split + 4));
|
||||
|
||||
if (null === $body) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$repaired = $header . "\r\n\r\n" . $body;
|
||||
|
||||
// Parsed before anything is written, so a message the library
|
||||
// cannot read is left exactly as it was rather than half repaired.
|
||||
$parsed = ImapMessage::fromString($repaired, ImapConnectionFactory::config());
|
||||
} catch (Throwable $e) {
|
||||
// Skipped rather than fatal: one unreadable file is one message
|
||||
// that stays as it was, and stopping here would leave every later
|
||||
// one unrepaired too.
|
||||
$this->logger->warning('Could not read a stored message for the repeated-header repair', [
|
||||
'messageId' => $message->id,
|
||||
'error' => $e->getMessage(),
|
||||
]);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
$message->rawPath = $this->rawStorage->store((int) $message->account->id, (int) $message->id, $repaired);
|
||||
|
||||
if (IMAP::MESSAGE_TYPE_MULTIPART !== $parsed->getStructure()?->type) {
|
||||
$message->bodyText = CharsetHelper::ensureUtf8($parsed->getTextBody());
|
||||
$message->bodyHtml = CharsetHelper::ensureUtf8($parsed->getHTMLBody());
|
||||
|
||||
// bodyHtmlSafe is what the reading pane renders; bodyHtml alone
|
||||
// would repair the database and leave the screen as it was.
|
||||
$this->sanitizer->sanitize($message);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,72 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Domain\Helper;
|
||||
|
||||
/**
|
||||
* Finds a message's header block repeated at the top of its own body, and
|
||||
* takes it off again.
|
||||
*
|
||||
* ── The shape ────────────────────────────────────────────────────────────────
|
||||
* The IMAP sync asks for a message in two halves, RFC822.HEADER and
|
||||
* RFC822.TEXT (webklex's Query::fetch()), and keeps them apart. Some messages
|
||||
* on a Stalwart server answer the second with the WHOLE message, headers
|
||||
* included (#173). Nothing between the socket and MessageSyncer looks at what
|
||||
* the TEXT literal holds, so webklex parses "headers, blank line, body" as the
|
||||
* body: a single-part HTML mail is shown with `Delivered-To: …` above its
|
||||
* content, and the stored .eml carries the headers twice. Sometimes there is no
|
||||
* blank line between the repeated block and the `<!DOCTYPE`, which is why the
|
||||
* blank line is optional below.
|
||||
*
|
||||
* ── Why an exact match, and nothing looser ───────────────────────────────────
|
||||
* Both halves come from the same message on the same server, so a repeat is
|
||||
* byte for byte the header literal. Anything looser — "starts with something
|
||||
* that parses as headers" — would also match a plain-text body that opens with
|
||||
* "Note: …", and cutting a real body is worse than the bug. A body that
|
||||
* genuinely begins with an exact copy of its own header block does not occur.
|
||||
*
|
||||
* Static and free of webklex, so the ingest path and the backfill that repairs
|
||||
* stored mail share one rule, and a test can state it without a server.
|
||||
*/
|
||||
final class RepeatedHeaderHelper
|
||||
{
|
||||
/**
|
||||
* The body without the repeated header block, or null when it does not
|
||||
* start with one.
|
||||
*
|
||||
* @param string $rawHeader the header block as the server sent it, with or
|
||||
* without the blank line that ends it
|
||||
* @param string $rawBody what the server sent as the body
|
||||
*/
|
||||
public static function strip(string $rawHeader, string $rawBody): ?string
|
||||
{
|
||||
$header = rtrim($rawHeader, "\r\n");
|
||||
|
||||
if ('' === $header || false === str_starts_with($rawBody, $header)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$rest = substr($rawBody, strlen($header));
|
||||
|
||||
// The match has to end where a line ends. Otherwise the block is only
|
||||
// the beginning of a longer line — the last header continuing — and
|
||||
// what follows is not a body.
|
||||
if ('' !== $rest && false === str_starts_with($rest, "\r\n") && false === str_starts_with($rest, "\n")) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// The line break that ends the last header line, then the blank line
|
||||
// that separates headers from body — when there is one. Two at most:
|
||||
// a body that itself opens with an empty line keeps it.
|
||||
for ($i = 0; $i < 2; ++$i) {
|
||||
if (true === str_starts_with($rest, "\r\n")) {
|
||||
$rest = substr($rest, 2);
|
||||
} elseif (true === str_starts_with($rest, "\n")) {
|
||||
$rest = substr($rest, 1);
|
||||
}
|
||||
}
|
||||
|
||||
return $rest;
|
||||
}
|
||||
}
|
||||
@@ -1380,6 +1380,44 @@ class MessageRepository extends ServiceEntityRepository
|
||||
->getSingleScalarResult();
|
||||
}
|
||||
|
||||
/**
|
||||
* IMAP messages after $afterId whose original bytes are on disk, in id
|
||||
* order: the set RepeatedHeaderBackfillTask reads.
|
||||
*
|
||||
* Every one of them, not the ones whose body looks wrong. A repeated
|
||||
* header block is only visible in body_html for a single-part message; a
|
||||
* multipart one renders correctly and carries the doubled block in its
|
||||
* .eml all the same, and only reading the file finds it.
|
||||
*
|
||||
* @return list<Message>
|
||||
*/
|
||||
public function findImapWithStoredRaw(int $afterId, int $limit): array
|
||||
{
|
||||
return $this->imapWithStoredRawQueryBuilder($afterId)
|
||||
->orderBy('m.id', 'ASC')
|
||||
->setMaxResults($limit)
|
||||
->getQuery()
|
||||
->getResult();
|
||||
}
|
||||
|
||||
public function countImapWithStoredRaw(): int
|
||||
{
|
||||
return (int) $this->imapWithStoredRawQueryBuilder(0)
|
||||
->select('COUNT(m.id)')
|
||||
->getQuery()
|
||||
->getSingleScalarResult();
|
||||
}
|
||||
|
||||
/** Shared so the count and the walk can never disagree about the set. */
|
||||
private function imapWithStoredRawQueryBuilder(int $afterId): \Doctrine\ORM\QueryBuilder
|
||||
{
|
||||
return $this->createQueryBuilder('m')
|
||||
->andWhere('m.id > :afterId')
|
||||
->andWhere('m.imapUid IS NOT NULL')
|
||||
->andWhere('m.rawPath IS NOT NULL')
|
||||
->setParameter('afterId', $afterId);
|
||||
}
|
||||
|
||||
/** Shared so the count and the walk can never disagree about the set. */
|
||||
private function misfiledBodyPartQueryBuilder(int $afterId): \Doctrine\ORM\QueryBuilder
|
||||
{
|
||||
|
||||
@@ -11,6 +11,7 @@ use App\Domain\Helper\CharsetHelper;
|
||||
use App\Domain\Helper\ImapConnectionFactory;
|
||||
use App\Domain\Helper\MessageIdHelper;
|
||||
use App\Domain\Helper\MimeHeaderHelper;
|
||||
use App\Domain\Helper\RepeatedHeaderHelper;
|
||||
use App\Entity\Mail\Mailbox;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Entity\Mail\MessagePart;
|
||||
@@ -28,6 +29,7 @@ use Psr\Log\LoggerInterface;
|
||||
use Webklex\PHPIMAP\Attribute;
|
||||
use Webklex\PHPIMAP\Client;
|
||||
use Webklex\PHPIMAP\Message as ImapMessage;
|
||||
use Webklex\PHPIMAP\Support\AttachmentCollection;
|
||||
use App\Service\Mail\HeaderNormalizer;
|
||||
|
||||
class MessageSyncer
|
||||
@@ -531,6 +533,10 @@ class MessageSyncer
|
||||
}
|
||||
|
||||
try {
|
||||
// Before anything reads the body, so the columns, the parts and
|
||||
// the stored .eml all come from the same bytes.
|
||||
$this->dropRepeatedHeader($imapMessage, $uid, $mailbox->fullPath);
|
||||
|
||||
$message = $this->buildMessage($imapMessage, $mailbox, $accountId);
|
||||
$this->em->persist($message);
|
||||
|
||||
@@ -739,6 +745,47 @@ class MessageSyncer
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the body again without the header block a server repeated at the
|
||||
* top of it.
|
||||
*
|
||||
* Some messages on a Stalwart server answer RFC822.TEXT with the whole
|
||||
* message, headers included, while RFC822.HEADER is right (#173). webklex
|
||||
* builds its parts from whatever the TEXT literal held, so a single-part
|
||||
* HTML mail was stored with its own `Delivered-To: …` block as the first
|
||||
* lines of body_html, and rawOf() wrote the headers into the .eml twice.
|
||||
* See RepeatedHeaderHelper for how a repeat is recognised.
|
||||
*
|
||||
* Read from the structure, not getRawBody(): getRawBody() keeps the first
|
||||
* answer it gives, so asking it here would pin the doubled bytes for
|
||||
* rawOf() below. parseRawBody() appends to the bodies and attachments an
|
||||
* earlier parse left behind, which is why both are emptied first.
|
||||
*/
|
||||
private function dropRepeatedHeader(ImapMessage $imapMessage, int $uid, string $mailboxPath): void
|
||||
{
|
||||
$header = $imapMessage->getHeader();
|
||||
$structure = $imapMessage->getStructure();
|
||||
|
||||
if (null === $header || null === $structure) {
|
||||
return;
|
||||
}
|
||||
|
||||
$body = RepeatedHeaderHelper::strip($header->raw, $structure->raw);
|
||||
|
||||
if (null === $body) {
|
||||
return;
|
||||
}
|
||||
|
||||
$imapMessage->bodies = [];
|
||||
$imapMessage->setAttachments(AttachmentCollection::make());
|
||||
$imapMessage->parseRawBody($body);
|
||||
|
||||
$this->logger->info('Dropped a header block the server repeated at the top of the body', [
|
||||
'uid' => $uid,
|
||||
'mailbox' => $mailboxPath,
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reassemble the message as it arrived: raw headers, blank line, raw body.
|
||||
* webklex keeps both around after parsing, so nothing is re-fetched.
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Command\Backfill;
|
||||
|
||||
use App\Command\Backfill\RepeatedHeaderBackfillTask;
|
||||
use App\Domain\Helper\RawMessageStorage;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Tests\Support\Mail\SeedsMarkerFixtures;
|
||||
use DateTimeImmutable;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
use Symfony\Component\Console\Command\Command;
|
||||
use Symfony\Component\Console\Input\ArrayInput;
|
||||
use Symfony\Component\Console\Output\BufferedOutput;
|
||||
use Symfony\Component\Console\Style\SymfonyStyle;
|
||||
|
||||
/**
|
||||
* Mail stored with its header block repeated at the top of the body is
|
||||
* repaired from its own .eml (#173).
|
||||
*
|
||||
* The rows are seeded as the sync used to leave them: the .eml holding the
|
||||
* headers twice, and — for a single-part message — body_html starting with
|
||||
* `Delivered-To:`. No mail server is involved, because none is needed: the
|
||||
* file holds everything the repair reads.
|
||||
*
|
||||
* The multipart case is the one that would go wrong quietly. Its bodies were
|
||||
* parsed correctly, so the repair must fix the file and leave them alone; a
|
||||
* test that only looked at the single-part message would let a repair that
|
||||
* re-parses everything through.
|
||||
*/
|
||||
final class RepeatedHeaderBackfillTest extends KernelTestCase
|
||||
{
|
||||
use SeedsMarkerFixtures;
|
||||
|
||||
private Connection $connection;
|
||||
private RepeatedHeaderBackfillTask $task;
|
||||
private RawMessageStorage $rawStorage;
|
||||
|
||||
/** @var list<string> .eml files written to disk, which no rollback removes */
|
||||
private array $storedRawPaths = [];
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$container = self::getContainer();
|
||||
$this->em = $container->get(EntityManagerInterface::class);
|
||||
$this->connection = $container->get(Connection::class);
|
||||
$this->task = $container->get(RepeatedHeaderBackfillTask::class);
|
||||
$this->rawStorage = $container->get(RawMessageStorage::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
|
||||
$this->user = $this->seedUser();
|
||||
$this->account = $this->seedAccount();
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
foreach ($this->storedRawPaths as $path) {
|
||||
$this->rawStorage->delete($path);
|
||||
}
|
||||
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testASinglePartMessageGetsItsBodyBackRatherThanItsHeadersAgain(): void
|
||||
{
|
||||
$header = $this->header('text/html; charset=utf-8');
|
||||
$html = "<!DOCTYPE html>\r\n<html><body><p>Your parcel is on its way.</p></body></html>\r\n";
|
||||
|
||||
$id = $this->message($header . "\r\n\r\n" . $header . "\r\n\r\n" . $html, bodyHtml: $header . "\r\n\r\n" . $html);
|
||||
|
||||
self::assertSame(Command::SUCCESS, $this->runTask());
|
||||
|
||||
$message = $this->reload($id);
|
||||
|
||||
self::assertStringStartsWith('<!DOCTYPE html>', (string) $message->bodyHtml);
|
||||
self::assertStringNotContainsString('Delivered-To', (string) $message->bodyHtmlSafe);
|
||||
self::assertStringContainsString('Your parcel is on its way.', (string) $message->bodyHtmlSafe);
|
||||
self::assertSame($header . "\r\n\r\n" . $html, $this->raw($message));
|
||||
}
|
||||
|
||||
public function testAMultipartMessageHasItsFileRepairedAndItsBodiesLeftAlone(): void
|
||||
{
|
||||
$header = $this->header('multipart/alternative; boundary="B"');
|
||||
$parts = "--B\r\n"
|
||||
. "Content-Type: text/plain; charset=utf-8\r\n\r\nYour parcel is on its way.\r\n"
|
||||
. "--B\r\n"
|
||||
. "Content-Type: text/html; charset=utf-8\r\n\r\n<p>Your parcel is on its way.</p>\r\n"
|
||||
. "--B--\r\n";
|
||||
|
||||
// Bodies the parse would not produce, standing for a repair the sync
|
||||
// made: the backfill must not replace them with its own reading.
|
||||
$id = $this->message(
|
||||
$header . "\r\n\r\n" . $header . "\r\n\r\n" . $parts,
|
||||
bodyHtml: '<p>As the sync stored it.</p>',
|
||||
bodyText: 'As the sync stored it.',
|
||||
);
|
||||
|
||||
$this->runTask();
|
||||
|
||||
$message = $this->reload($id);
|
||||
|
||||
self::assertSame($header . "\r\n\r\n" . $parts, $this->raw($message));
|
||||
self::assertSame('<p>As the sync stored it.</p>', $message->bodyHtml);
|
||||
self::assertSame('As the sync stored it.', $message->bodyText);
|
||||
}
|
||||
|
||||
public function testAMessageStoredOnceIsLeftAsItWas(): void
|
||||
{
|
||||
$header = $this->header('text/plain; charset=utf-8');
|
||||
$raw = $header . "\r\n\r\nDelivered-To: is a line of this body.\r\n";
|
||||
|
||||
$id = $this->message($raw, bodyText: "Delivered-To: is a line of this body.\r\n");
|
||||
|
||||
$this->runTask();
|
||||
|
||||
$message = $this->reload($id);
|
||||
|
||||
self::assertSame($raw, $this->raw($message));
|
||||
self::assertSame("Delivered-To: is a line of this body.\r\n", $message->bodyText);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------- helpers
|
||||
|
||||
private function runTask(): int
|
||||
{
|
||||
return $this->task->run(new SymfonyStyle(new ArrayInput([]), new BufferedOutput()));
|
||||
}
|
||||
|
||||
private function header(string $contentType): string
|
||||
{
|
||||
return "Delivered-To: {$this->account->email}\r\n"
|
||||
. "From: Parcels <parcels@example.test>\r\n"
|
||||
. "Subject: Your parcel\r\n"
|
||||
. "Date: Mon, 21 Sep 2026 10:00:00 +0200\r\n"
|
||||
. sprintf("Message-ID: <%s@example.test>\r\n", uniqid('parcel-', true))
|
||||
. "MIME-Version: 1.0\r\n"
|
||||
. "Content-Type: {$contentType}";
|
||||
}
|
||||
|
||||
private function message(string $raw, ?string $bodyHtml = null, ?string $bodyText = null): int
|
||||
{
|
||||
$message = new Message();
|
||||
$message->account = $this->account;
|
||||
$message->subject = 'Your parcel';
|
||||
$message->fromAddress = 'parcels@example.test';
|
||||
$message->receivedAt = new DateTimeImmutable();
|
||||
$message->hasAttachments = false;
|
||||
$message->imapUid = 1;
|
||||
$message->bodyHtml = $bodyHtml;
|
||||
$message->bodyText = $bodyText;
|
||||
|
||||
$this->em->persist($message);
|
||||
$this->em->flush();
|
||||
|
||||
$message->rawPath = $this->rawStorage->store((int) $this->account->id, (int) $message->id, $raw);
|
||||
$this->storedRawPaths[] = $message->rawPath;
|
||||
|
||||
$this->em->flush();
|
||||
|
||||
return (int) $message->id;
|
||||
}
|
||||
|
||||
private function reload(int $id): Message
|
||||
{
|
||||
$this->em->clear();
|
||||
|
||||
$message = $this->em->getRepository(Message::class)->find($id);
|
||||
|
||||
self::assertInstanceOf(Message::class, $message);
|
||||
|
||||
return $message;
|
||||
}
|
||||
|
||||
private function raw(Message $message): string
|
||||
{
|
||||
return (string) file_get_contents($this->rawStorage->getAbsolutePath((string) $message->rawPath));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Domain\Helper;
|
||||
|
||||
use App\Domain\Helper\RepeatedHeaderHelper;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* A header block repeated at the top of the body comes off, and nothing else
|
||||
* does.
|
||||
*
|
||||
* Both halves of the rule are pinned. Missing a repeat is #173 — a message
|
||||
* shown with its own `Delivered-To: …` block above the content. Taking off
|
||||
* something that is not a repeat would cut lines out of a real body, which is
|
||||
* worse, so every near miss below must come back as null.
|
||||
*/
|
||||
final class RepeatedHeaderHelperTest extends TestCase
|
||||
{
|
||||
private const string HEADER = "Delivered-To: me@example.test\r\n"
|
||||
. "Subject: Your parcel\r\n"
|
||||
. "Message-ID: <parcel@example.test>\r\n"
|
||||
. "Content-Type: text/html; charset=utf-8";
|
||||
|
||||
#[DataProvider('repeats')]
|
||||
public function testARepeatedHeaderBlockIsTakenOffTheBody(string $rawHeader, string $rawBody, string $expected): void
|
||||
{
|
||||
self::assertSame($expected, RepeatedHeaderHelper::strip($rawHeader, $rawBody));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string, array{string, string, string}>
|
||||
*/
|
||||
public static function repeats(): iterable
|
||||
{
|
||||
$html = "<!DOCTYPE html>\r\n<p>Hello</p>\r\n";
|
||||
|
||||
yield 'with the blank line, header as IMAP sends it' => [self::HEADER . "\r\n\r\n", self::HEADER . "\r\n\r\n" . $html, $html];
|
||||
yield 'with the blank line, header already trimmed' => [self::HEADER, self::HEADER . "\r\n\r\n" . $html, $html];
|
||||
yield 'without a blank line before the DOCTYPE' => [self::HEADER . "\r\n\r\n", self::HEADER . "\r\n" . $html, $html];
|
||||
yield 'a body that opens with an empty line keeps it' => [self::HEADER, self::HEADER . "\r\n\r\n\r\n" . $html, "\r\n" . $html];
|
||||
yield 'bare line feeds' => [self::HEADER, self::HEADER . "\n\n" . $html, $html];
|
||||
yield 'nothing after the headers' => [self::HEADER, self::HEADER . "\r\n\r\n", ''];
|
||||
}
|
||||
|
||||
#[DataProvider('nearMisses')]
|
||||
public function testABodyThatDoesNotRepeatTheHeadersRatherThanLooksLikeItIsLeftAlone(string $rawHeader, string $rawBody): void
|
||||
{
|
||||
self::assertNull(RepeatedHeaderHelper::strip($rawHeader, $rawBody));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string, array{string, string}>
|
||||
*/
|
||||
public static function nearMisses(): iterable
|
||||
{
|
||||
yield 'an ordinary body' => [self::HEADER, "<!DOCTYPE html>\r\n<p>Hello</p>\r\n"];
|
||||
|
||||
// Header-shaped, but not this message's headers.
|
||||
yield 'a text body that opens like a header' => [self::HEADER, "Note: the parcel is late.\r\n"];
|
||||
|
||||
// The same first lines, then the block goes on: the server's header
|
||||
// literal is the whole block, so a longer one is not a repeat of it.
|
||||
yield 'the block continues past the last header line' => [self::HEADER, self::HEADER . "; format=flowed\r\n\r\nHello\r\n"];
|
||||
|
||||
yield 'an empty header' => ['', "Hello\r\n"];
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,217 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service\Imap;
|
||||
|
||||
use App\Domain\Helper\ImapConnectionFactory;
|
||||
use App\Domain\Helper\RawMessageStorage;
|
||||
use App\Entity\Mail\Mailbox;
|
||||
use App\Service\Imap\ImapMailboxImporter;
|
||||
use App\Tests\Support\Mail\SeedsMarkerFixtures;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
use Webklex\PHPIMAP\Message as ImapMessage;
|
||||
use Webklex\PHPIMAP\Support\FlagCollection;
|
||||
|
||||
/**
|
||||
* A message whose body arrives with its own header block in front is stored
|
||||
* once, with the body as its body (#173).
|
||||
*
|
||||
* Some messages on a Stalwart server answer RFC822.TEXT with the whole message.
|
||||
* webklex parsed that as the body, so a single-part HTML mail was shown with
|
||||
* `Delivered-To: …` and the rest of its headers above the content, and the
|
||||
* .eml written beside it held the headers twice. A multipart message rendered
|
||||
* correctly — its parts start after the boundary — but its .eml was doubled
|
||||
* just the same, which is why both shapes are here.
|
||||
*
|
||||
* The fixture is built the way the server's answer reaches MessageSyncer:
|
||||
* Message::fromString() splits at the first blank line, so the header is the
|
||||
* first block and the body is the second block plus the real body — exactly
|
||||
* the pair Query::fetch() hands to Message::make() for such a message.
|
||||
*
|
||||
* Through the importer, like ImapDraftSentAtTest, because buildMessage() and
|
||||
* rawOf() are private and the claim is about what is stored.
|
||||
*/
|
||||
final class RepeatedHeaderIngestTest extends KernelTestCase
|
||||
{
|
||||
use SeedsMarkerFixtures;
|
||||
|
||||
private Connection $connection;
|
||||
private ImapMailboxImporter $importer;
|
||||
private RawMessageStorage $rawStorage;
|
||||
private int $mailboxId;
|
||||
|
||||
/** @var list<string> .eml files written to disk, which no rollback removes */
|
||||
private array $storedRawPaths = [];
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$this->em = static::getContainer()->get(EntityManagerInterface::class);
|
||||
$this->connection = static::getContainer()->get(Connection::class);
|
||||
$this->importer = static::getContainer()->get(ImapMailboxImporter::class);
|
||||
$this->rawStorage = static::getContainer()->get(RawMessageStorage::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
|
||||
$this->user = $this->seedUser();
|
||||
$this->account = $this->seedAccount();
|
||||
|
||||
$mailbox = new Mailbox();
|
||||
$mailbox->account = $this->account;
|
||||
$mailbox->name = 'Deleted Items';
|
||||
$mailbox->fullPath = 'Deleted Items';
|
||||
$mailbox->isSyncEnabled = true;
|
||||
$mailbox->lastSeenUid = 3;
|
||||
$mailbox->importFloorUid = 4;
|
||||
|
||||
$this->em->persist($mailbox);
|
||||
$this->em->flush();
|
||||
|
||||
$this->mailboxId = (int) $mailbox->id;
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
foreach ($this->storedRawPaths as $path) {
|
||||
$this->rawStorage->delete($path);
|
||||
}
|
||||
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testTheBodyIsTheBodyRatherThanTheHeadersAgain(): void
|
||||
{
|
||||
$html = "<!DOCTYPE html>\r\n<html><body><p>Your parcel is on its way.</p></body></html>\r\n";
|
||||
|
||||
$this->importer->importPage($this->mailboxId, FakeImportClient::holding([
|
||||
1 => $this->doubled(1, "Content-Type: text/html; charset=utf-8\r\n", "\r\n\r\n" . $html),
|
||||
// No blank line between the repeated block and the DOCTYPE, the
|
||||
// other shape the issue reports.
|
||||
2 => $this->doubled(2, "Content-Type: text/html; charset=utf-8\r\n", "\r\n" . $html),
|
||||
]));
|
||||
|
||||
$rows = $this->rows();
|
||||
|
||||
self::assertSame([1, 2], array_keys($rows));
|
||||
|
||||
foreach ($rows as $uid => $row) {
|
||||
self::assertStringNotContainsString('Delivered-To', (string) $row['body_html'], "uid {$uid}: the headers are not body text");
|
||||
self::assertStringStartsWith('<!DOCTYPE html>', (string) $row['body_html'], "uid {$uid}");
|
||||
self::assertStringNotContainsString('Delivered-To', (string) $row['body_html_safe'], "uid {$uid}: nor on screen");
|
||||
self::assertSame(1, substr_count($this->raw($row), 'Delivered-To: '), "uid {$uid}: the .eml carries the headers once");
|
||||
}
|
||||
|
||||
// Header and body separated by exactly one blank line, which is what
|
||||
// makes the .eml a message any client opens — including the one that
|
||||
// came without a blank line at all.
|
||||
self::assertStringContainsString("<parcel-2@example.test>\r\nMIME-Version: 1.0\r\nContent-Type: text/html; charset=utf-8\r\n\r\n<!DOCTYPE html>", $this->raw($rows[2]));
|
||||
}
|
||||
|
||||
public function testAMultipartMessageIsStoredOnceRatherThanWithItsHeadersTwice(): void
|
||||
{
|
||||
$parts = "\r\n\r\n--B\r\n"
|
||||
. "Content-Type: text/plain; charset=utf-8\r\n\r\nYour parcel is on its way.\r\n"
|
||||
. "--B\r\n"
|
||||
. "Content-Type: text/html; charset=utf-8\r\n\r\n<p>Your parcel is on its way.</p>\r\n"
|
||||
. "--B--\r\n";
|
||||
|
||||
$this->importer->importPage($this->mailboxId, FakeImportClient::holding([
|
||||
3 => $this->doubled(3, "Content-Type: multipart/alternative; boundary=\"B\"\r\n", $parts),
|
||||
]));
|
||||
|
||||
$row = $this->rows()[3];
|
||||
|
||||
self::assertSame(1, substr_count($this->raw($row), 'Delivered-To: '));
|
||||
self::assertStringContainsString('<p>Your parcel is on its way.</p>', (string) $row['body_html']);
|
||||
self::assertSame('Your parcel is on its way.', trim((string) $row['body_text']));
|
||||
self::assertFalse((bool) $row['has_attachments'], 'no part became an attachment on the way');
|
||||
}
|
||||
|
||||
public function testAnOrdinaryMessageIsStoredAsItCame(): void
|
||||
{
|
||||
$header = $this->header(1, "Content-Type: text/plain; charset=utf-8\r\n");
|
||||
|
||||
$this->importer->importPage($this->mailboxId, FakeImportClient::holding([
|
||||
1 => $this->fromString($header . "\r\nDelivered-To: is a line of this body.\r\n", 1),
|
||||
]));
|
||||
|
||||
$row = $this->rows()[1];
|
||||
|
||||
// A body that opens like a header is still the body. Only an exact
|
||||
// repeat of the header block is taken off.
|
||||
self::assertSame('Delivered-To: is a line of this body.', trim((string) $row['body_text']));
|
||||
self::assertSame(2, substr_count($this->raw($row), 'Delivered-To: '));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------- helpers
|
||||
|
||||
/**
|
||||
* The message as such a server answers it: the header block, a blank line,
|
||||
* the same header block, then $rest — which starts with the blank line,
|
||||
* or with a lone line break where the server sent none.
|
||||
*/
|
||||
private function doubled(int $uid, string $contentType, string $rest): ImapMessage
|
||||
{
|
||||
$header = rtrim($this->header($uid, $contentType), "\r\n");
|
||||
|
||||
return $this->fromString($header . "\r\n\r\n" . $header . $rest, $uid);
|
||||
}
|
||||
|
||||
private function header(int $uid, string $contentType): string
|
||||
{
|
||||
return "Delivered-To: {$this->account->email}\r\n"
|
||||
. "From: Parcels <parcels@example.test>\r\n"
|
||||
. "To: {$this->account->email}\r\n"
|
||||
. "Subject: Parcel {$uid}\r\n"
|
||||
. "Date: Mon, 21 Sep 2026 10:00:00 +0200\r\n"
|
||||
. "Message-ID: <parcel-{$uid}@example.test>\r\n"
|
||||
. "MIME-Version: 1.0\r\n"
|
||||
. $contentType;
|
||||
}
|
||||
|
||||
private function fromString(string $raw, int $uid): ImapMessage
|
||||
{
|
||||
return ImapMessage::fromString($raw, ImapConnectionFactory::config())
|
||||
->setUid($uid)
|
||||
->setFlags(FlagCollection::make(['Seen' => 'Seen']));
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $row
|
||||
*/
|
||||
private function raw(array $row): string
|
||||
{
|
||||
$path = (string) $row['raw_path'];
|
||||
|
||||
self::assertNotSame('', $path, 'the sync wrote no .eml');
|
||||
|
||||
$this->storedRawPaths[] = $path;
|
||||
|
||||
return (string) file_get_contents($this->rawStorage->getAbsolutePath($path));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array<int, array<string, mixed>> keyed by IMAP uid
|
||||
*/
|
||||
private function rows(): array
|
||||
{
|
||||
$rows = [];
|
||||
|
||||
foreach ($this->connection->fetchAllAssociative(
|
||||
'SELECT imap_uid, body_html, body_html_safe, body_text, has_attachments, raw_path FROM message WHERE mailbox_id = ? ORDER BY imap_uid',
|
||||
[$this->mailboxId],
|
||||
) as $row) {
|
||||
$rows[(int) $row['imap_uid']] = $row;
|
||||
}
|
||||
|
||||
return $rows;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user