diff options
| -rw-r--r-- | docs/superpowers/specs/2026-08-20-compose-and-send-design.md | 39 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/composecontext.cpp | 517 | ||||
| -rw-r--r-- | src/composecontext.h | 177 | ||||
| -rw-r--r-- | src/messagebuilder.cpp | 55 | ||||
| -rw-r--r-- | src/mimeparser.cpp | 2 | ||||
| -rw-r--r-- | src/mimeparser.h | 16 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_composecontext.cpp | 1051 | ||||
| -rw-r--r-- | tests/test_messagebuilder.cpp | 33 |
10 files changed, 1884 insertions, 8 deletions
diff --git a/docs/superpowers/specs/2026-08-20-compose-and-send-design.md b/docs/superpowers/specs/2026-08-20-compose-and-send-design.md index aade2d1..9533602 100644 --- a/docs/superpowers/specs/2026-08-20-compose-and-send-design.md +++ b/docs/superpowers/specs/2026-08-20-compose-and-send-design.md @@ -430,7 +430,7 @@ Two structs cross boundaries, in `types.h` beside the existing ones. | `originalPath` | the `.eml` being replied to or forwarded; empty for New | | `inReplyTo` | Message-ID of the original | | `references` | the original's References plus its Message-ID | -| `to`, `cc` | pre-filled recipients, the user's own addresses already stripped | +| `to`, `cc` | pre-filled recipients, the user's own addresses already stripped; a reply to the user's OWN message is addressed to that message's recipients instead of back to the user, mirroring its To/Cc split (see Replying to oneself) | | `subject` | `Re:` / `Fwd:` prefixed, an existing prefix not doubled | | `quotedBody` | the `>`-prefixed original; empty when the action does not quote | | `seedHtml` | did the original carry a `text/html` part | @@ -447,6 +447,13 @@ Two structs cross boundaries, in `types.h` beside the existing ones. | `attachments` | local paths | | `inReplyTo`, `references` | carried through unchanged | +Message-ids are carried BARE, without angle brackets, matching what GMime hands +back when `MimeParser` reads a `Message-ID`. `MessageBuilder` adds the brackets +when it writes the header, in one place rather than in each caller: they are wire +syntax, and GMime writes an EMPTY header for a bare addr-spec rather than +complaining, so a caller that forgets them ships a reply that threads nowhere +while nothing looks wrong locally. + `In-Reply-To` and `References` are not optional. Without them a reply appears as an orphan thread in the sender's own client. @@ -505,15 +512,38 @@ Six, each needing the five places `CLAUDE.md` enumerates: `knownActions()`, | Action | Meaning | Scope | |---|---|---| | `compose` | New message | none needed | -| `reply` | Reply to the displayed message, quoted | sender only | +| `reply` | Reply to the displayed message, quoted | sender only, except when the sender is the user (see below) | | `reply_all` | Reply to all, quoted | sender + To + Cc, own addresses removed | -| `reply_no_quote` | Reply with an empty body | sender only | +| `reply_no_quote` | Reply with an empty body | sender only, same exception | | `forward` | Forward, body quoted inline, attachments carried | none | | `save_message` | Write the raw `.eml` to a chosen path | any message | `reply_all_no_quote` is deliberately absent. Six actions is already a large menu and the combination is reached by deleting the quote. +### Replying to oneself + +A reply whose sender is entirely the user's own addresses is addressed to that +message's **original recipients** rather than to the sender. A plain reply takes +its To and Cc together, having no Cc field of its own to mirror into. A +reply-all MIRRORS THE SPLIT: the original's To becomes To and its Cc becomes Cc, +because To means "addressed to you" and Cc "for information", and promoting a +Cc'd party to To is a change every recipient can see. +This is an ordinary gesture rather than an edge case: it is reached from the +Sent view, from a follow-up on mail that went unanswered, and from any thread +whose selected row is the user's own message. Addressing the sender there +addresses the user, so the reply reaches nobody it was meant for. + +"Own" means EVERY parsed sender address is the user's. A message the user sent +together with somebody else is still a reply to that co-sender, and takes the +ordinary sender-only path. + +Mail the user sent to THEMSELVES alone leaves nothing after own addresses are +removed, and there the sender is restored: the user is the correct recipient of +their own note. The rejected alternative was to strip the sender and leave To +empty, which silently drops every recipient while the message still looks +sendable. + **Every action acts on the displayed message**, resolved with `messageScopeFor()` semantics: a thread row means the one message its card shows, a reply row means itself. Not `threadFor()`. Replying to a thread is @@ -660,7 +690,8 @@ Cases: `multipart/alternative` when `sendHtml` is on and `text/plain` alone when off; `multipart/mixed` nesting with attachments; each enabled extension rendering, and tables and raw HTML **not** rendering; RFC 2047 encoding of a non-ASCII subject and display name; quoted-printable for an accented body; -`In-Reply-To` and `References` carried; `Re:` and `Fwd:` not doubling. +`In-Reply-To` and `References` carried; `Re:` and `Fwd:` not doubling, in the +non-English spellings and counted forms as well as the English ones. **`test_messagesender`** uses stub commands, not msmtp: one exiting 0, one exiting non-zero with stderr, one that does not exist. The stub writes stdin to diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index eac2fab..a462ba3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -15,6 +15,7 @@ add_library(qtmaildir_lib STATIC maildirname.cpp draftstore.cpp messagesender.cpp + composecontext.cpp tagchip.cpp tagcolors.cpp savequerydialog.cpp diff --git a/src/composecontext.cpp b/src/composecontext.cpp new file mode 100644 index 0000000..251a028 --- /dev/null +++ b/src/composecontext.cpp @@ -0,0 +1,517 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +// gmime BEFORE any Qt header. glib declares a struct field named "signals", +// which Qt #defines to Q_SIGNALS, and the collision is a compile error whose +// message names neither library. +#include <gmime/gmime.h> + +#include "composecontext.h" + +#include "config.h" +#include "mimeparser.h" + +#include <QDir> +#include <QSet> +#include <QRegularExpression> + +namespace { + +/// GMime must be initialised exactly once per process. +/// +/// MimeParser and MessageBuilder each carry their own copy of this guard, and +/// this is a third rather than a shared one for the reason MessageBuilder +/// already records: a test may link only one of them, so neither can assume +/// another ran. Without it the first internet_address_list_parse() call +/// dereferences an uninitialised type registry and SEGVs, which is exactly what +/// this file did before the guard was added. +/// +/// A function-local static rather than the `static bool` flag the other two +/// use: C++11 guarantees the initialiser runs exactly once even under +/// concurrent entry, which a bare flag does not. +void ensureGMimeInitialised() +{ + static const bool initialised = [] { + g_mime_init(); + return true; + }(); + Q_UNUSED(initialised); +} + +/// Matches a reply prefix at the START of a subject, in the spellings clients +/// actually produce. +/// +/// Anchored, and that is load-bearing rather than tidy: an unanchored search +/// finds "re:" inside an ordinary subject ("Notes re: budget") and refuses to +/// prefix a genuine first reply, which breaks threading in the recipient's +/// client with nothing to see locally. +/// +/// **The non-English spellings are not politeness, they are the doubling bug in +/// a mixed-locale mailbox**, which this one is: the user writes Italian and +/// corresponds beyond it. German and Dutch clients send `AW:`, Scandinavian +/// ones `SV:`, Spanish and Portuguese `RES:`. An English-only pattern turns +/// every one of those into `Re: AW: subject`, and the next round into +/// `Re: Re: AW:`. +/// +/// `Re[2]:` and `Re(2):` are the counted forms Outlook and some list managers +/// emit. They mean the same thing and must not be doubled either. +/// +/// **Single-letter spellings are deliberately NOT here**, though Italian +/// clients do send `R:`. Measured 2026-08-21: with `r` in the alternation, +/// `R: report on Q3` reads as a reply prefix, so a genuine first reply to that +/// subject gets no `Re:` and threads nowhere in the recipient's client. A +/// one-letter token before a colon is an ordinary subject far more often than +/// it is a prefix, and this is the same failure the anchoring note above +/// describes. The cost of omitting it is one doubled `Re: R:`, which is +/// cosmetic; the cost of including it is broken threading, which is not. +/// +/// Ordering inside the alternation matters: `res` before `re` so the longer +/// spelling is not consumed by the shorter one, leaving a stray `S:` unmatched. +const QRegularExpression &replyPrefix() +{ + static const QRegularExpression expression( + QStringLiteral("^\\s*(res|re|aw|antw|sv|vs)\\s*(\\[\\d+\\]|\\(\\d+\\))?\\s*:"), + QRegularExpression::CaseInsensitiveOption); + return expression; +} + +/// "Fwd:" and "Fw:" mean the same thing and both are common, as do the +/// non-English spellings a mixed-locale mailbox receives: German `WG:`, Spanish +/// and Portuguese `RV:` and `ENC:`, French `TR:`. Same reasoning as +/// replyPrefix(): an English-only pattern produces `Fwd: WG: subject`. +/// +/// Italian `I:` is omitted for the reason replyPrefix() omits `R:`, and it is +/// the worse of the two: `I: notes` is an entirely ordinary subject. The +/// previous pattern was `fwd?`, which likewise matched a bare `F:`; that is not +/// a forward marker in any client and `F: results` must still get a prefix. +const QRegularExpression &forwardPrefix() +{ + static const QRegularExpression expression( + QStringLiteral("^\\s*(fwd|fw|wg|enc|rv|tr)\\s*(\\[\\d+\\]|\\(\\d+\\))?\\s*:"), + QRegularExpression::CaseInsensitiveOption); + return expression; +} + +/// The account whose maildir contains \p path, or empty. +QString accountOwning(const Config &config, const QString &path, + const QString &mailRoot) +{ + for (const Account &account : config.accounts()) { + if (account.maildir.isEmpty()) + continue; + const QString prefix = + QDir(mailRoot).absoluteFilePath(account.maildir) + QLatin1Char('/'); + // Compared as a path prefix with the separator INCLUDED: without the + // trailing slash an account "work" would also match a maildir + // "work-archive", and the reply would be sent from the wrong account. + if (path.startsWith(prefix)) + return account.key; + } + return {}; +} + +/// True when \p address is one of \p ownAddresses, compared case-insensitively. +/// +/// Compared on the addr-spec, never on a rendered "Name <addr>": a display +/// name may legitimately contain an address-looking substring, and a substring +/// test against the whole form strips a real recipient whose name happens to +/// quote one of the user's addresses. +bool isOwn(const QString &address, const QStringList &ownAddresses) +{ + for (const QString &own : ownAddresses) { + if (own.isEmpty()) + continue; + if (address.compare(own, Qt::CaseInsensitive) == 0) + return true; + } + return false; +} + +/// Appends \p recipient to \p out unless its address is already in \p seen or +/// belongs to the user. \p seen is updated. +/// +/// Deduplication is keyed on the lowercased ADDRESS, so the same mailbox under +/// two different display names counts once, which is what the original's To +/// and Cc routinely contain. +void appendUnlessSuppressed(const ComposeContextBuilder::Recipient &recipient, + const QStringList &ownAddresses, + QSet<QString> *seen, QStringList *out) +{ + if (recipient.address.isEmpty()) + return; + const QString key = recipient.address.toLower(); + if (seen->contains(key)) + return; + if (isOwn(recipient.address, ownAddresses)) + return; + seen->insert(key); + out->append(recipient.rendered); +} + +} // namespace + +QList<ComposeContextBuilder::Recipient> +ComposeContextBuilder::parseAddressHeader(const QString &rawHeader) +{ + ensureGMimeInitialised(); + + const QByteArray utf8 = rawHeader.trimmed().toUtf8(); + if (utf8.isEmpty()) + return {}; + + // Returns NULL rather than an empty list for input it can make nothing of, + // including the empty string. Guarded above and again here: the header is + // untrusted and this is the crash if it is not. + InternetAddressList *list = internet_address_list_parse(nullptr, utf8.constData()); + if (!list) + return {}; + + QList<Recipient> recipients; + const int count = internet_address_list_length(list); + for (int i = 0; i < count; ++i) { + InternetAddress *address = internet_address_list_get_address(list, i); + if (!address) + continue; + + // Only MAILBOXES. A group carries a name and no address, so keeping it + // would put "undisclosed-recipients" in a To field as though it were a + // person. It is also the injection defence: measured 2026-08-21, a raw + // newline smuggled into a header makes GMime parse the following + // "Bcc: evil@example.net" as a GROUP, and dropping non-mailboxes drops + // it rather than pre-filling a recipient the user never saw. + if (!INTERNET_ADDRESS_IS_MAILBOX(address)) + continue; + + const char *addr = + internet_address_mailbox_get_addr(INTERNET_ADDRESS_MAILBOX(address)); + if (!addr || !*addr) + continue; + + Recipient recipient; + recipient.address = QString::fromUtf8(addr).trimmed(); + if (recipient.address.isEmpty()) + continue; + + // Rendered BY GMIME rather than assembled by string. Quoting a display + // name is not a matter of wrapping it in quotes: a name containing a + // comma must come back out quoted or it re-parses as two recipients. + // + // The final argument is ENCODE, and it is a security parameter rather + // than a formatting preference. With FALSE a display name carrying a + // raw newline renders with that newline intact, which is a + // header-injection primitive: `"foo\nBcc: evil@example.net" <a@...>` + // comes back out verbatim and anything writing it into a To: line + // emits a second header the user never saw. With TRUE the same input + // renders RFC 2047 encoded as `=?iso-8859-1?q?foo=0ABcc=3A?= ...` and + // the newline can no longer terminate a header. Measured 2026-08-21; + // this shipped as FALSE and the test caught it. + char *rendered = internet_address_to_string( + address, g_mime_format_options_get_default(), TRUE); + recipient.rendered = rendered ? QString::fromUtf8(rendered).trimmed() + : QString(); + g_free(rendered); + if (recipient.rendered.isEmpty()) + recipient.rendered = recipient.address; + + recipients.append(recipient); + } + g_object_unref(list); + + return recipients; +} + +QStringList ComposeContextBuilder::ownAddresses(const Config &config) +{ + QStringList addresses; + for (const Account &account : config.accounts()) { + const QString address = account.address.trimmed(); + // An empty address is dropped rather than collected. It would match + // nothing usefully and, in any substring comparison, everything. + if (!address.isEmpty() && !addresses.contains(address, Qt::CaseInsensitive)) + addresses.append(address); + } + return addresses; +} + +void ComposeContextBuilder::recipientsForReply(const ParsedMessage &message, + bool replyAll, + const QStringList &ownAddresses, + QStringList *toOut, + QStringList *ccOut) +{ + if (toOut) + toOut->clear(); + if (ccOut) + ccOut->clear(); + if (!toOut) + return; + + // Reply-To wins over From when present (RFC 5322 3.6.2: it names where the + // author wants replies sent). Applied to reply-all as well as to a plain + // reply: a list's reply-all belongs on the list too. + QList<Recipient> sender = parseAddressHeader(message.replyTo); + if (sender.isEmpty()) + sender = parseAddressHeader(message.from); + + // A reply to the user's OWN message goes to the people that message was + // addressed to, not back to the user. Reached from the Sent view, from a + // follow-up on unanswered mail, and from any thread whose selected row is + // the user's own message, so it is an ordinary gesture rather than an edge + // case. The alternative considered and rejected was stripping the sender + // and leaving To empty, which silently drops every recipient and looks + // sendable. + // + // "Own" means EVERY parsed sender address is the user's. A message with a + // co-sender is still a reply to that co-sender. + bool senderIsSelf = !sender.isEmpty(); + for (const Recipient &recipient : sender) { + if (!isOwn(recipient.address, ownAddresses)) { + senderIsSelf = false; + break; + } + } + + QSet<QString> seen; + if (senderIsSelf) { + // The original's To, with own addresses removed. Its Cc is deliberately + // NOT taken here for a reply-all: the split is the message's meaning, + // To being "addressed to you" and Cc "for information", and promoting a + // Cc'd party to To is visible to every recipient. The Cc pass below + // carries them across unchanged, so the reply mirrors the original. + // + // A PLAIN reply has no Cc field to mirror into, so it takes To and Cc + // together: everyone who was on the message is still addressed, which + // is what a reply to a conversation the user started means. + // + // Mail the user sent to themselves alone leaves nothing after the own + // filter, which is the one case where addressing the user IS correct, + // so the sender is restored below rather than producing an empty To. + QStringList headers = { message.to }; + if (!replyAll) + headers.append(message.cc); + for (const QString &header : headers) { + const QList<Recipient> parsed = parseAddressHeader(header); + for (const Recipient &recipient : parsed) + appendUnlessSuppressed(recipient, ownAddresses, &seen, toOut); + } + } + + if (toOut->isEmpty()) { + for (const Recipient &recipient : sender) { + if (recipient.address.isEmpty()) + continue; + const QString key = recipient.address.toLower(); + if (seen.contains(key)) + continue; + // The sender is NOT filtered against the user's own addresses + // here. This branch is reached either for an ordinary reply, where + // the sender is somebody else, or for a note the user sent only to + // themselves, where they are the correct recipient. Stripping in + // either case produces a message with no recipient that still + // looks sendable. + seen.insert(key); + toOut->append(recipient.rendered); + } + } + + // A From that parses to no mailbox at all leaves To empty, and that is + // reachable from real mail rather than only from a hostile fixture: a bare + // display name with no angle brackets ("From: Mailer Daemon") is what + // bounces and some automated senders emit, and MimeParser hands it over as + // a header with zero mailboxes. An empty To is the worst outcome available, + // since MessageBuilder treats it as success: the message is handed to the + // send command with nobody to deliver to and a copy is filed in Sent that + // looks sent and reached no one. + // + // The original's recipients are the only remaining candidates. Own + // addresses are stripped, so a message the user sent AND that has an + // unparseable From still yields nothing here, which is correct: there is + // genuinely nobody to address, and the composer shows an empty To the user + // can see and fill rather than a wrong one they will not check. + if (toOut->isEmpty()) { + for (const QString &header : { message.to, message.cc }) { + const QList<Recipient> parsed = parseAddressHeader(header); + for (const Recipient &recipient : parsed) + appendUnlessSuppressed(recipient, ownAddresses, &seen, toOut); + } + } + + if (!replyAll || !ccOut) + return; + + // Everyone else goes to Cc, with the user's own addresses removed and + // duplicates suppressed ACROSS the two fields rather than within each: the + // sender is very often also in the original's To, and per-field + // deduplication lists them twice. + for (const QString &header : { message.to, message.cc }) { + const QList<Recipient> parsed = parseAddressHeader(header); + for (const Recipient &recipient : parsed) + appendUnlessSuppressed(recipient, ownAddresses, &seen, ccOut); + } +} + +QStringList ComposeContextBuilder::referencesForReply(const ParsedMessage &message) +{ + QStringList references; + QSet<QString> seen; + + const auto append = [&references, &seen](const QString &raw) { + QString id = raw.trimmed(); + if (id.startsWith(QLatin1Char('<')) && id.endsWith(QLatin1Char('>'))) + id = id.mid(1, id.size() - 2).trimmed(); + if (id.isEmpty() || seen.contains(id)) + return; + seen.insert(id); + references.append(id); + }; + + // The header is a whitespace-separated run of <message-ids>, and real mail + // wraps it across lines, so whitespace is the conformant separator. + // + // Commas are accepted BESIDES whitespace because some clients emit + // `<a@x>,<b@y>`, which RFC 5322 does not allow here. Splitting on + // whitespace alone turns that whole header into ONE token, and the bracket + // strip below then yields the garbage id `a@x>,<b@y`: not a threading + // degradation but a fabricated Message-ID sent to the recipient's client. + // A comma cannot appear inside a msg-id, so accepting it costs nothing. + const QStringList existing = message.references.split( + QRegularExpression(QStringLiteral("[\\s,]+")), Qt::SkipEmptyParts); + for (const QString &id : existing) + append(id); + + // The original's own id goes LAST, which is what makes the chain an order + // rather than a set. Appended through the same deduplication, so a message + // whose References already names it does not repeat it. + append(message.messageId); + + return references; +} + +QString ComposeContextBuilder::accountForReply(const Config &config, + const QStringList &messagePaths, + const QStringList &recipients, + const QString &mailRoot) +{ + QStringList candidates; + for (const QString &path : messagePaths) { + const QString key = accountOwning(config, path, mailRoot); + if (!key.isEmpty() && !candidates.contains(key)) + candidates.append(key); + } + + if (candidates.isEmpty()) + return {}; + if (candidates.size() == 1) + return candidates.first(); + + // Ambiguous: the same message in more than one maildir. Prefer the account + // whose own address appears among the recipients, which is the reason the + // copy landed there. + for (const QString &key : candidates) { + const Account account = config.account(key); + if (account.address.isEmpty()) + continue; + for (const QString &recipient : recipients) { + if (recipient.contains(account.address, Qt::CaseInsensitive)) + return key; + } + } + + // Arbitrary, and visible: the From field shows the choice. + return candidates.first(); +} + +QString ComposeContextBuilder::accountForNew(const Config &config, + const QString &selectedAccount) +{ + const auto canSend = [&config](const QString &key) { + if (key.isEmpty()) + return false; + for (const Account &account : config.accounts()) { + if (account.key == key) + return account.canSend(); + } + return false; + }; + + // 1. The dropdown's current account, when it is a specific one that can send. + if (canSend(selectedAccount)) + return selectedAccount; + + // 2. [compose] default_account. + if (canSend(config.compose().defaultAccount)) + return config.compose().defaultAccount; + + // 3. [general] startup_account, on the same condition. + if (canSend(config.startupAccount())) + return config.startupAccount(); + + // 4. The first account in configuration order that can send. Arbitrary, + // which is exactly why rules 2 and 3 exist. + const QList<Account> sending = config.sendingAccounts(); + if (!sending.isEmpty()) + return sending.first().key; + + // No account can send. A valid read-only installation; the caller's action + // is disabled and should never have reached this. + return {}; +} + +QString ComposeContextBuilder::replySubject(const QString &original) +{ + if (replyPrefix().match(original).hasMatch()) + return original; + return QStringLiteral("Re: ") + original; +} + +QString ComposeContextBuilder::forwardSubject(const QString &original) +{ + if (forwardPrefix().match(original).hasMatch()) + return original; + return QStringLiteral("Fwd: ") + original; +} + +QString ComposeContextBuilder::quoteBody(const ParsedMessage &message) +{ + QStringList quoted; + + // The attribution line. Deliberately NOT translated and NOT reformatted + // through a locale-dependent date format: this text is sent to a recipient + // who may not share the user's locale, and the raw Date header is what + // every other client quotes. + quoted.append(QStringLiteral("On %1, %2 wrote:") + .arg(message.date, message.from)); + quoted.append(QString()); + + // Normalised to LF first. A CRLF body split on '\n' alone leaves a + // carriage return at the end of every line, which survives into the sent + // message as a stray CR in the middle of a quoted line. + QString body = message.plainBody; + body.replace(QStringLiteral("\r\n"), QStringLiteral("\n")); + body.replace(QLatin1Char('\r'), QLatin1Char('\n')); + + const QStringList lines = body.split(QLatin1Char('\n')); + for (const QString &line : lines) { + // A blank line still carries the marker. Without it the quote visually + // ends there in every client that renders quoting. + quoted.append(line.isEmpty() ? QStringLiteral(">") + : QStringLiteral("> ") + line); + } + + return quoted.join(QLatin1Char('\n')); +} diff --git a/src/composecontext.h b/src/composecontext.h new file mode 100644 index 0000000..4027af0 --- /dev/null +++ b/src/composecontext.h @@ -0,0 +1,177 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QList> +#include <QString> +#include <QStringList> + +#include "types.h" + +struct Account; +class Config; +struct ParsedMessage; + +/// Builds the ComposeContext that opens a composer. +/// +/// Free functions in a namespace: this is pure logic over values, and keeping +/// it apart from ComposeWindow is what lets recipient derivation, subject +/// prefixing and account resolution be tested without a painter. +namespace ComposeContextBuilder { + +/// One recipient split out of a header, as both parts and as a rendered whole. +/// +/// Kept as a struct rather than a bare string because the two halves answer +/// two different questions and conflating them is how the user's own address +/// escapes a filter. `address` is what a comparison must use: a display name +/// may legitimately CONTAIN an address-looking substring, and a substring test +/// against the whole rendered form matches "not-me@example.org" for "me@example.org". +/// `rendered` is what goes in the field the user sees. +struct Recipient +{ + QString address; ///< The bare addr-spec, no display name, no angle brackets. + QString rendered; ///< "Name <addr>" or the bare address when it has no name. +}; + +/// The addresses belonging to the user, across every configured account. +/// +/// Every one of them is stripped from a reply-all's recipients. Missing one +/// means the user receives their own reply, which is the failure this is most +/// likely to have. +QStringList ownAddresses(const Config &config); + +/// Splits a raw address header into individual recipients, using GMime. +/// +/// NEVER split on commas. A display name may contain one, so +/// `"Rossi, Mario" <m@example.org>, info@example.net` is TWO addresses and a +/// naive split reports three, one of which ("Rossi") is not an address at all +/// and would be handed to the send command as a recipient. This is the same +/// reason `recipientSummary()` in mimeparser.cpp parses rather than splits. +/// +/// Groups (`undisclosed-recipients:;`) contribute NOTHING. A group carries a +/// name and no mailbox, so naming it would put "undisclosed-recipients" in a +/// To field as though it were a person. This also closes a header-injection +/// shape: a raw newline in a header value makes GMime parse the smuggled +/// `Bcc: evil@example.net` as a GROUP, measured 2026-08-21, so dropping +/// non-mailboxes drops the injected recipient rather than carrying it forward. +/// +/// An unparseable header yields an empty list rather than a partial guess. +QList<Recipient> parseAddressHeader(const QString &rawHeader); + +/// Who a reply goes to, as \p toOut and \p ccOut. +/// +/// This is the function the spec calls out as where the subtle bugs live, and +/// the rules are not interchangeable: +/// +/// - **Reply** goes to the ORIGINAL SENDER only, and Cc is empty. Reply-To +/// takes precedence over From when the original carries one (RFC 5322 +/// §3.6.2: it names where the author wants replies sent), which is what +/// makes a mailing list's reply land on the list rather than on a person who +/// never asked to be written to directly. +/// - **Reply-all** puts the sender in To, and the original's To and Cc in Cc. +/// The user's own addresses are stripped from BOTH, or they receive their +/// own reply. Comparison is case-insensitive: an address's domain is +/// case-insensitive by RFC and real mail varies the local part's case too, +/// so a case-sensitive filter lets `User@Example.org` through against a +/// configured `user@example.org`. +/// - A duplicate is suppressed ACROSS To and Cc, not within each: the sender +/// is very often also in the original's To, and listing them twice is what +/// naive per-field deduplication produces. +/// +/// \p replyAll false yields sender-only. \p ownAddresses is what +/// ownAddresses(config) returned. +/// +/// **A reply to the user's OWN message goes where that message went**, not +/// back to the user: To comes from the original's recipients instead of from +/// its sender. A plain reply takes its To and Cc together, having no Cc field +/// of its own to mirror into; a reply-all MIRRORS THE SPLIT, the original's To +/// becoming To and its Cc becoming Cc, because To means "addressed to you" and +/// Cc "for information" and promoting a Cc'd party to To is visible to every +/// recipient. This is reached from the Sent view, from a follow-up on +/// unanswered mail, and from any thread whose selected row is the user's own +/// message, so it is an ordinary gesture. "Own" means EVERY parsed sender +/// address is the user's; a co-sender is still someone to reply to. +/// +/// Mail the user sent to THEMSELVES alone leaves nothing after that filter, and +/// there the sender is restored: the user is the correct recipient of their own +/// note. Emptying To instead would produce a message with no recipient that +/// still looks sendable, which is why stripping the sender was rejected as the +/// fix. Nothing else strips an own address from a plain Reply's To. +void recipientsForReply(const ParsedMessage &message, bool replyAll, + const QStringList &ownAddresses, + QStringList *toOut, QStringList *ccOut); + +/// The References header for a reply: the original's References plus its +/// Message-ID. +/// +/// Not optional. Without it a reply appears as an orphan thread in the +/// sender's own client. A duplicate Message-ID is not appended twice. +/// +/// Ids come back BARE, without angle brackets, matching what GMime hands back +/// when MimeParser reads a `Message-ID`. The brackets are wire syntax and +/// `MessageBuilder` adds them when it writes the header, in one place rather +/// than in each caller: GMime writes an EMPTY header for a bare addr-spec +/// rather than complaining, so a caller that forgets them ships a reply that +/// threads nowhere while nothing looks wrong locally. +QStringList referencesForReply(const ParsedMessage &message); + +/// Which account replies to a message whose file lives at \p messagePaths. +/// +/// The displayed message's own maildir is the strongest available signal and +/// wins outright: mail sent to an address landed in that address's maildir, so +/// replying from it is what the recipient expects. The account dropdown is NOT +/// consulted. +/// +/// A message can be in more than one maildir: on a list twice under two +/// addresses, or duplicated across accounts by mbsync, and notmuch returns +/// several filenames for one id. \p recipients disambiguates by preferring the +/// account matching a To or Cc entry; failing that the first is taken. The From +/// field shows the choice, so an arbitrary resolution is visible rather than +/// hidden. +QString accountForReply(const Config &config, const QStringList &messagePaths, + const QStringList &recipients, const QString &mailRoot); + +/// Which account a NEW message comes from, by the four fallback rules. +/// +/// \p selectedAccount is the dropdown's current account, empty for All +/// accounts. Returns empty only when no account can send at all. +QString accountForNew(const Config &config, const QString &selectedAccount); + +/// `Re:` or `Fwd:` prefixed, without doubling an existing prefix. +/// +/// An existing prefix is recognised in the non-English spellings a mixed-locale +/// mailbox receives (`AW:`, `SV:`, `RES:`, `WG:`, `TR:`, `RV:`, `ENC:`) and in +/// the counted forms Outlook emits (`Re[2]:`, `Re(3):`), or every one of those +/// doubles into `Re: AW: subject`. +/// +/// Single-letter spellings are deliberately NOT recognised, though Italian +/// clients send `R:` and `I:`: `R: report on Q3` is an ordinary subject, and +/// treating it as a prefix means a genuine first reply gets no `Re:` and +/// threads nowhere. See the patterns in composecontext.cpp for the measurement. +QString replySubject(const QString &original); +QString forwardSubject(const QString &original); + +/// The `>`-prefixed original, with an attribution line. +/// +/// Takes a ParsedMessage, NOT a MessageNode: the node carries no body and no +/// date (it holds messageId, threadId, from, subject, tags, filePath and +/// depth), so quoting has to come from what MimeParser produced. +QString quoteBody(const ParsedMessage &message); + +} // namespace ComposeContextBuilder diff --git a/src/messagebuilder.cpp b/src/messagebuilder.cpp index f27c3c2..42a0e31 100644 --- a/src/messagebuilder.cpp +++ b/src/messagebuilder.cpp @@ -110,6 +110,35 @@ GMimePart *makeTextPart(const char *subtype, const QString &text) /// rejects what GMime cannot parse at all; it is not an address validator, and /// a typo that happens to be parseable still goes out. bool setAddressHeader(GMimeMessage *message, const char *header, const QStringList &addresses, + QString *badEntry); + +/// A message-id in the angle brackets the wire format requires, added if the +/// caller did not supply them. +/// +/// **The brackets are syntax, not decoration, and GMime enforces it by writing +/// an EMPTY HEADER for a bare addr-spec rather than by complaining.** Measured +/// 2026-08-21: `In-Reply-To: current@example.org` emits `In-Reply-To:` with no +/// value, so the reply arrives as an orphan thread in the recipient's client +/// while nothing looks wrong locally. +/// +/// Bracketing lives HERE, in the one function that composes these headers, +/// rather than in each caller. Every source of a message-id in this application +/// hands over a bare one: GMime strips the brackets when MimeParser reads +/// `Message-ID`, and `ComposeContextBuilder::referencesForReply` strips them +/// again from the References chain so the two agree. A convention spread across +/// callers is one a later caller gets wrong, and the failure is invisible +/// without inspecting a sent message. +QString bracketed(const QString &messageId) +{ + const QString id = messageId.trimmed(); + if (id.isEmpty()) + return {}; + if (id.startsWith(QLatin1Char('<')) && id.endsWith(QLatin1Char('>'))) + return id; + return QLatin1Char('<') + id + QLatin1Char('>'); +} + +bool setAddressHeader(GMimeMessage *message, const char *header, const QStringList &addresses, QString *badEntry) { if (addresses.isEmpty()) @@ -238,12 +267,30 @@ Result build(const OutgoingMessage &message, const Account &account) const QByteArray subject = message.subject.toUtf8(); g_mime_message_set_subject(mime, subject.constData(), "utf-8"); - if (!message.inReplyTo.trimmed().isEmpty()) { - const QByteArray value = message.inReplyTo.trimmed().toUtf8(); + // Both headers are bracketed HERE rather than by the caller. See bracketed() + // for why, and for what a bare id costs. + const QString inReplyTo = bracketed(message.inReplyTo); + if (!inReplyTo.isEmpty()) { + const QByteArray value = inReplyTo.toUtf8(); g_mime_object_set_header(GMIME_OBJECT(mime), "In-Reply-To", value.constData(), "utf-8"); } - if (!message.references.isEmpty()) { - const QByteArray value = message.references.join(QLatin1Char(' ')).toUtf8(); + QStringList references; + for (const QString &id : message.references) { + const QString bracketedId = bracketed(id); + // An empty entry contributes nothing rather than a stray "<>": the + // References header is a run of ids, and one malformed entry is enough + // for a strict parser to discard the whole chain. + // + // Defensive rather than a path with a fixture behind it, like the + // length check in setAddressHeader above: referencesForReply() already + // drops empty ids, so a mutation on this line SURVIVES the suite. + // Measured 2026-08-21. Kept because it costs one comparison and the + // failure it covers is a silently broken thread. + if (!bracketedId.isEmpty()) + references.append(bracketedId); + } + if (!references.isEmpty()) { + const QByteArray value = references.join(QLatin1Char(' ')).toUtf8(); g_mime_object_set_header(GMIME_OBJECT(mime), "References", value.constData(), "utf-8"); } diff --git a/src/mimeparser.cpp b/src/mimeparser.cpp index 2782a4a..c1198b8 100644 --- a/src/mimeparser.cpp +++ b/src/mimeparser.cpp @@ -412,9 +412,11 @@ ParsedMessage MimeParser::parse(const QString &filePath) const out.subject = QString::fromUtf8( g_mime_message_get_subject(message) ?: ""); out.from = headerText(message, "From"); + out.replyTo = headerText(message, "Reply-To"); out.to = headerText(message, "To"); out.cc = headerText(message, "Cc"); out.date = headerText(message, "Date"); + out.references = headerText(message, "References"); out.messageId = QString::fromUtf8( g_mime_message_get_message_id(message) ?: ""); diff --git a/src/mimeparser.h b/src/mimeparser.h index da54434..64c4585 100644 --- a/src/mimeparser.h +++ b/src/mimeparser.h @@ -119,11 +119,27 @@ struct ParsedMessage QString subject; QString from; + + /// Where the author asked for replies to go, raw and undecoded-into-parts. + /// + /// Takes precedence over `from` when building a reply (RFC 5322 3.6.2). + /// Empty on the great majority of mail; a mailing list is the common case + /// that sets it, and honouring it is what keeps a list reply on the list + /// rather than on a person who never asked to be written to directly. + QString replyTo; + QString to; QString cc; QString date; QString messageId; + /// The raw References header, a whitespace-separated run of <message-ids>. + /// + /// Carried so a reply can extend the chain. Without it the reply appears + /// as an orphan thread in the recipient's client, which is the whole + /// reason the header exists. + QString references; + QString plainBody; QString htmlBody; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e38d764..48b30fc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -73,6 +73,7 @@ add_qtmaildir_test(messagebuilder) add_qtmaildir_test(maildirname) add_qtmaildir_test(draftstore) add_qtmaildir_test(messagesender) +add_qtmaildir_test(composecontext) add_qtmaildir_test(translations) # Asserts on the tracked .ts rather than the generated .qm: an untranslated # string is dropped by lrelease, so it is invisible in the .qm and shows up diff --git a/tests/test_composecontext.cpp b/tests/test_composecontext.cpp new file mode 100644 index 0000000..fccec87 --- /dev/null +++ b/tests/test_composecontext.cpp @@ -0,0 +1,1051 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +// gmime BEFORE any Qt header. glib declares a struct field named "signals", +// which Qt #defines to Q_SIGNALS. Needed here for g_log_set_handler(), which is +// how aGroupIsDroppedByTheGuardAndNotByAFailedCast() sees the difference +// between a group dropped by the guard and one dropped by a failed cast. +#include <gmime/gmime.h> + +#include <QtTest> +#include <QTemporaryDir> + +#include "composecontext.h" +#include "config.h" +#include "mimeparser.h" +#include "types.h" + +using ComposeContextBuilder::Recipient; + +class TestComposeContext : public QObject +{ + Q_OBJECT + +private slots: + // Own addresses. + void everyOwnAddressIsCollected(); + void aBlankOwnAddressIsNotCollected(); + + // Header parsing, the foundation the recipient rules stand on. + void aDisplayNameContainingACommaIsOneRecipient(); + void aQuotedDisplayNameSurvivesRoundTripping(); + void aMalformedHeaderYieldsNoRecipients(); + void anEmptyHeaderYieldsNoRecipients(); + void aGroupContributesNoRecipient(); + void aGroupIsDroppedByTheGuardAndNotByAFailedCast(); + void anInjectedHeaderLineIsNotCarriedForward(); + + // Reply and reply-all recipient derivation. + void aPlainReplyGoesToTheSenderOnly(); + void aReplyPrefersReplyToOverFrom(); + void aReplyAllPutsTheSenderInToAndTheRestInCc(); + void aReplyAllStripsEveryOwnAddress(); + void aReplyAllStripsAnOwnAddressRegardlessOfCase(); + void aReplyAllDoesNotListTheSenderTwice(); + void aReplyAllSuppressesDuplicatesAcrossToAndCc(); + void aReplyToOneselfStillAddressesSomeone(); + void aReplyToOwnMessageGoesToItsOriginalRecipients(); + void aReplyAllToOwnMessageDoesNotRepeatToInCc(); + void aCoSenderIsStillRepliedTo(); + void anUnparseableSenderStillProducesARecipient(); + void aReplyAllPrefersReplyToForTheToField(); + void aDisplayNameContainingAnOwnAddressIsNotMistakenForIt(); + + // References. + void referencesCarryTheOriginalChainPlusItsId(); + void referencesDoNotRepeatTheMessageId(); + void aCommaSeparatedReferencesHeaderIsSplitIntoIds(); + + // Subjects. + void aReplySubjectDoesNotDoubleItsPrefix(); + void aForwardSubjectDoesNotDoubleItsPrefix(); + void anEmptySubjectStillGetsAPrefix(); + void aSubjectMentioningReLaterStillGetsAPrefix(); + void aNonEnglishPrefixIsNotDoubled(); + void aCountedPrefixIsNotDoubled(); + void aSingleLetterBeforeAColonIsNotAPrefix(); + + // Account resolution. + void theReplyAccountComesFromTheMessagesMaildir(); + void anAccountIsNotMatchedByAPrefixOfItsMaildir(); + void anAmbiguousMessagePrefersTheMatchingRecipient(); + void anAmbiguousMessageWithNoMatchTakesTheFirst(); + void aNewMessagePrefersTheSelectedAccount(); + void aNewMessageFallsThroughASelectedAccountThatCannotSend(); + void aNewMessageUsesDefaultAccountFromAllAccounts(); + void aNewMessageUsesStartupAccountWhenNoDefaultIsSet(); + void aNewMessageFallsBackToTheFirstSendingAccount(); + void aNewMessageReturnsNothingWhenNoAccountCanSend(); + + // Quoting. + void aQuotedBodyPrefixesEveryLine(); + +private: + QString writeConfig(const QString &contents); + + QTemporaryDir m_dir; +}; + +QString TestComposeContext::writeConfig(const QString &contents) +{ + // A unique name per call: Config caches nothing, but reusing one path + // across tests in one binary invites a stale read to look like a pass. + static int counter = 0; + const QString path = + m_dir.filePath(QStringLiteral("qtmaildir%1.conf").arg(++counter)); + QFile file(path); + if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate | QIODevice::Text)) + return {}; + file.write(contents.toUtf8()); + file.close(); + return path; +} + +// --------------------------------------------------------------------------- +// Own addresses +// --------------------------------------------------------------------------- + +void TestComposeContext::everyOwnAddressIsCollected() +{ + // All five of the user's addresses. Missing one means they receive their + // own reply, and with five accounts that is the likeliest bug here. + const QString path = writeConfig(QStringLiteral( + "[account.one]\nmaildir=one\ntrash=Trash\naddress=first@example.org\n" + "[account.two]\nmaildir=two\ntrash=Trash\naddress=second@example.org\n" + "[account.three]\nmaildir=three\ntrash=Trash\naddress=third@example.org\n" + "[account.four]\nmaildir=four\ntrash=Trash\naddress=fourth@example.org\n" + "[account.five]\nmaildir=five\ntrash=Trash\naddress=fifth@example.org\n")); + QVERIFY(!path.isEmpty()); + + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 5); + + const QStringList own = ComposeContextBuilder::ownAddresses(config); + QCOMPARE(own.size(), 5); + for (const QString &address : { QStringLiteral("first@example.org"), + QStringLiteral("second@example.org"), + QStringLiteral("third@example.org"), + QStringLiteral("fourth@example.org"), + QStringLiteral("fifth@example.org") }) { + QVERIFY2(own.contains(address), + qPrintable(QStringLiteral("own address %1 was not collected").arg(address))); + } +} + +void TestComposeContext::aBlankOwnAddressIsNotCollected() +{ + // An account with no address key is legal. An empty string in this list + // would match nothing usefully and, in a substring filter, everything. + const QString path = writeConfig(QStringLiteral( + "[account.one]\nmaildir=one\ntrash=Trash\naddress=first@example.org\n" + "[account.noaddress]\nmaildir=two\ntrash=Trash\n")); + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + const QStringList own = ComposeContextBuilder::ownAddresses(config); + QCOMPARE(own, QStringList{ QStringLiteral("first@example.org") }); +} + +// --------------------------------------------------------------------------- +// Header parsing +// --------------------------------------------------------------------------- + +void TestComposeContext::aDisplayNameContainingACommaIsOneRecipient() +{ + // The single most likely parsing bug: splitting on commas turns one + // recipient into two, one of which ("Rossi") is not an address at all and + // would be handed to the send command. + const QList<Recipient> parsed = ComposeContextBuilder::parseAddressHeader( + QStringLiteral("\"Rossi, Mario\" <m@example.org>, info@example.net")); + + QCOMPARE(parsed.size(), 2); + QCOMPARE(parsed.at(0).address, QStringLiteral("m@example.org")); + QCOMPARE(parsed.at(1).address, QStringLiteral("info@example.net")); +} + +void TestComposeContext::aQuotedDisplayNameSurvivesRoundTripping() +{ + // A comma in a display name must come back out QUOTED. Unquoted, the + // rendered form is not a legal single address: it happens to survive + // GMime's own lenient re-parse, but it goes into a To: header that other + // clients and MTAs read, and a bare comma there is a recipient separator. + // + // Asserted on the RENDERED TEXT rather than on a re-parse, and that is the + // point of the test: a round-trip through parseAddressHeader() passes + // against string-assembled "Rossi, Mario <m@example.org>" because GMime + // reads it back as one address anyway. Measured 2026-08-21, a mutation + // replacing the GMime rendering with `name + " <" + addr + ">"` left the + // whole suite green until this assertion was written this way. + const QList<Recipient> parsed = ComposeContextBuilder::parseAddressHeader( + QStringLiteral("\"Rossi, Mario\" <m@example.org>")); + QCOMPARE(parsed.size(), 1); + QCOMPARE(parsed.at(0).rendered, + QStringLiteral("\"Rossi, Mario\" <m@example.org>")); + + // And it still re-parses to the same one address. + const QList<Recipient> again = + ComposeContextBuilder::parseAddressHeader(parsed.at(0).rendered); + QCOMPARE(again.size(), 1); + QCOMPARE(again.at(0).address, QStringLiteral("m@example.org")); +} + +void TestComposeContext::aMalformedHeaderYieldsNoRecipients() +{ + // GMime returns NULL rather than an empty list for input it can make + // nothing of. Measured 2026-08-21: "not an address at all" and "<<<>>>" + // both return NULL. + QVERIFY(ComposeContextBuilder::parseAddressHeader( + QStringLiteral("not an address at all")).isEmpty()); + QVERIFY(ComposeContextBuilder::parseAddressHeader( + QStringLiteral("<<<>>>")).isEmpty()); +} + +void TestComposeContext::anEmptyHeaderYieldsNoRecipients() +{ + QVERIFY(ComposeContextBuilder::parseAddressHeader(QString()).isEmpty()); + QVERIFY(ComposeContextBuilder::parseAddressHeader( + QStringLiteral(" ")).isEmpty()); +} + +void TestComposeContext::aGroupContributesNoRecipient() +{ + // A group has a name and no mailbox. Carrying its name forward would put + // "undisclosed-recipients" in a To field as though it were a person. + const QList<Recipient> parsed = ComposeContextBuilder::parseAddressHeader( + QStringLiteral("undisclosed-recipients:;")); + QVERIFY2(parsed.isEmpty(), + qPrintable(QStringLiteral("a group produced %1 recipient(s)") + .arg(parsed.size()))); +} + +void TestComposeContext::aGroupIsDroppedByTheGuardAndNotByAFailedCast() +{ + // The count alone cannot see this, which is why the guard survived a + // mutation until 2026-08-21. Removing the INTERNET_ADDRESS_IS_MAILBOX check + // still yields no recipients, because the invalid cast makes GMime's own + // assertion return NULL and the address is skipped one line later. The + // count is therefore right for the wrong reason, and the reason matters: an + // invalid GObject cast is undefined behaviour papered over by an assertion + // that G_DISABLE_CHECKS compiles out and that G_DEBUG=fatal-criticals turns + // into an abort. A security property must not rest on assertions staying + // enabled. + // + // So this asserts on the CRITICAL rather than on the count. glib routes it + // through the log handler installed here, and a clean parse emits none. + struct Captured + { + static void handler(const gchar *domain, GLogLevelFlags level, + const gchar *messageText, gpointer userData) + { + Q_UNUSED(domain); + Q_UNUSED(level); + auto *messages = static_cast<QStringList *>(userData); + messages->append(QString::fromUtf8(messageText)); + } + }; + + // Registered per DOMAIN, and the domain is the trap: the two criticals this + // watches for carry "GLib-GObject" and "gmime", while a NULL domain + // registers only for the default one. A handler on nullptr alone catches + // NOTHING here and the test passes against the mutation, measured + // 2026-08-21. + QStringList criticals; + const auto levels = GLogLevelFlags(G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_WARNING + | G_LOG_FLAG_FATAL | G_LOG_FLAG_RECURSION); + QList<guint> handlerIds; + for (const char *domain : { "GLib-GObject", "gmime" }) + handlerIds.append(g_log_set_handler(domain, levels, &Captured::handler, &criticals)); + + // A group carrying MEMBERS, not the empty "undisclosed-recipients:;". The + // empty form has nothing to cast, so it cannot tell the two paths apart. + const QList<Recipient> parsed = ComposeContextBuilder::parseAddressHeader( + QStringLiteral("friends: a@example.org, b@example.net;")); + + int i = 0; + for (const char *domain : { "GLib-GObject", "gmime" }) + g_log_remove_handler(domain, handlerIds.at(i++)); + + QVERIFY2(parsed.isEmpty(), + qPrintable(QStringLiteral("a group with members produced %1 recipient(s)") + .arg(parsed.size()))); + QVERIFY2(criticals.isEmpty(), + qPrintable(QStringLiteral("GMime emitted %1 during the parse: %2") + .arg(criticals.size()) + .arg(criticals.join(QLatin1Char('|'))))); +} + +void TestComposeContext::anInjectedHeaderLineIsNotCarriedForward() +{ + // Header injection, from a stranger's message into the user's reply. + // Measured 2026-08-21: GMime parses the smuggled line as a GROUP named + // "Bcc", so dropping non-mailboxes drops it. If groups were kept, a reply + // would silently pre-fill a recipient the user never saw. + const QList<Recipient> parsed = ComposeContextBuilder::parseAddressHeader( + QStringLiteral("a@example.org\nBcc: evil@example.net")); + + QCOMPARE(parsed.size(), 1); + QCOMPARE(parsed.at(0).address, QStringLiteral("a@example.org")); + for (const Recipient &recipient : parsed) { + QVERIFY2(!recipient.rendered.contains(QStringLiteral("evil@example.net")), + qPrintable(QStringLiteral("injected address survived in: %1") + .arg(recipient.rendered))); + } + + // The other injection shape: the newline hidden INSIDE a quoted display + // name, where it does not split the header and so is not dropped as a + // group. It has to come back RFC 2047 encoded, never as a raw newline: a + // bare CR or LF in a rendered recipient is a header-injection primitive + // the moment anything writes it into a To: line. Rendering by hand rather + // than through GMime is what loses the encoding. + const QList<Recipient> inName = ComposeContextBuilder::parseAddressHeader( + QStringLiteral("\"foo\nBcc: evil@example.net\" <a@example.org>")); + QCOMPARE(inName.size(), 1); + QCOMPARE(inName.at(0).address, QStringLiteral("a@example.org")); + QVERIFY2(!inName.at(0).rendered.contains(QLatin1Char('\n')) + && !inName.at(0).rendered.contains(QLatin1Char('\r')), + qPrintable(QStringLiteral("a raw newline survived into a rendered " + "recipient: %1") + .arg(inName.at(0).rendered))); +} + +// --------------------------------------------------------------------------- +// Reply and reply-all +// --------------------------------------------------------------------------- + +void TestComposeContext::aPlainReplyGoesToTheSenderOnly() +{ + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral("me@example.org, other@example.net"); + message.cc = QStringLiteral("third@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/false, { QStringLiteral("me@example.org") }, &to, &cc); + + QCOMPARE(to.size(), 1); + QVERIFY2(to.at(0).contains(QStringLiteral("sender@example.org")), + qPrintable(QStringLiteral("To was %1").arg(to.join(QLatin1Char('|'))))); + QVERIFY2(cc.isEmpty(), + qPrintable(QStringLiteral("a plain reply put %1 in Cc") + .arg(cc.join(QLatin1Char('|'))))); +} + +void TestComposeContext::aReplyPrefersReplyToOverFrom() +{ + // RFC 5322 3.6.2: Reply-To names where the author wants replies sent. This + // is what makes a list reply land on the list rather than on a person who + // never asked to be written to directly. + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.replyTo = QStringLiteral("List <list@example.net>"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply(message, /*replyAll=*/false, {}, &to, &cc); + + QCOMPARE(to.size(), 1); + QVERIFY2(to.at(0).contains(QStringLiteral("list@example.net")), + qPrintable(QStringLiteral("To was %1, expected the Reply-To") + .arg(to.join(QLatin1Char('|'))))); + QVERIFY2(!to.at(0).contains(QStringLiteral("sender@example.org")), + "From was used despite a Reply-To being present"); +} + +void TestComposeContext::aReplyAllPutsTheSenderInToAndTheRestInCc() +{ + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral("first@example.net"); + message.cc = QStringLiteral("second@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply(message, /*replyAll=*/true, {}, &to, &cc); + + QCOMPARE(to.size(), 1); + QVERIFY(to.at(0).contains(QStringLiteral("sender@example.org"))); + + QCOMPARE(cc.size(), 2); + QVERIFY2(cc.join(QLatin1Char('|')).contains(QStringLiteral("first@example.net")), + qPrintable(QStringLiteral("Cc was %1").arg(cc.join(QLatin1Char('|'))))); + QVERIFY2(cc.join(QLatin1Char('|')).contains(QStringLiteral("second@example.com")), + qPrintable(QStringLiteral("Cc was %1").arg(cc.join(QLatin1Char('|'))))); +} + +void TestComposeContext::aReplyAllStripsEveryOwnAddress() +{ + // Five accounts, and the user's address appears in the original's To under + // THREE of them. Stripping only the first is the exact failure this guards: + // the reply-all would then be addressed to the user twice over. + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral( + "first@example.org, stranger@example.net, third@example.org"); + message.cc = QStringLiteral("fifth@example.org, another@example.com"); + + const QStringList own = { QStringLiteral("first@example.org"), + QStringLiteral("second@example.org"), + QStringLiteral("third@example.org"), + QStringLiteral("fourth@example.org"), + QStringLiteral("fifth@example.org") }; + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply(message, /*replyAll=*/true, own, &to, &cc); + + const QString all = (to + cc).join(QLatin1Char('|')); + for (const QString &address : own) { + QVERIFY2(!all.contains(address, Qt::CaseInsensitive), + qPrintable(QStringLiteral("own address %1 survived in: %2") + .arg(address, all))); + } + // And the strangers must all still be there: a filter that removed + // everything would pass the check above while producing an unsendable reply. + QVERIFY2(all.contains(QStringLiteral("stranger@example.net")), + qPrintable(QStringLiteral("a stranger was stripped too: %1").arg(all))); + QVERIFY2(all.contains(QStringLiteral("another@example.com")), + qPrintable(QStringLiteral("a stranger was stripped too: %1").arg(all))); + QVERIFY2(all.contains(QStringLiteral("sender@example.org")), + qPrintable(QStringLiteral("the sender was stripped: %1").arg(all))); +} + +void TestComposeContext::aReplyAllStripsAnOwnAddressRegardlessOfCase() +{ + // A domain is case-insensitive by RFC and real mail varies the local part's + // case too. A case-sensitive filter lets the user's own address through and + // they receive their own reply. + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral("Me@Example.ORG, stranger@example.net"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/true, { QStringLiteral("me@example.org") }, &to, &cc); + + const QString all = (to + cc).join(QLatin1Char('|')); + QVERIFY2(!all.contains(QStringLiteral("Me@Example.ORG"), Qt::CaseInsensitive), + qPrintable(QStringLiteral("a differently-cased own address survived: %1") + .arg(all))); + QVERIFY(all.contains(QStringLiteral("stranger@example.net"))); +} + +void TestComposeContext::aReplyAllDoesNotListTheSenderTwice() +{ + // The sender is very often also in their own message's To (a list posting + // reflected back). Without cross-field suppression they appear in To AND Cc. + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral("sender@example.org, stranger@example.net"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply(message, /*replyAll=*/true, {}, &to, &cc); + + const QString all = (to + cc).join(QLatin1Char('|')); + QCOMPARE(all.count(QStringLiteral("sender@example.org")), 1); + QVERIFY(all.contains(QStringLiteral("stranger@example.net"))); +} + +void TestComposeContext::aReplyAllSuppressesDuplicatesAcrossToAndCc() +{ + // The same address in the original's To and Cc, with different display + // names so a whole-string comparison would treat them as distinct. + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral("Person One <dup@example.net>"); + message.cc = QStringLiteral("P. One <dup@example.net>, other@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply(message, /*replyAll=*/true, {}, &to, &cc); + + const QString all = (to + cc).join(QLatin1Char('|')); + QCOMPARE(all.count(QStringLiteral("dup@example.net")), 1); + QVERIFY(all.contains(QStringLiteral("other@example.com"))); +} + +void TestComposeContext::aReplyToOneselfStillAddressesSomeone() +{ + // Replying to a message the user sent themselves. Stripping own addresses + // from a plain Reply's To would leave a message with no recipient that + // still looks sendable. + ParsedMessage message; + message.from = QStringLiteral("Me <me@example.org>"); + message.to = QStringLiteral("me@example.org"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/false, { QStringLiteral("me@example.org") }, &to, &cc); + + QVERIFY2(!to.isEmpty(), "a reply to oneself produced no recipient at all"); + QVERIFY(to.join(QLatin1Char('|')).contains(QStringLiteral("me@example.org"))); +} + +void TestComposeContext::aReplyToOwnMessageGoesToItsOriginalRecipients() +{ + // The Sent view, and a follow-up on unanswered mail: the user replies to a + // message they sent. Addressing the sender there addresses the user, so To + // comes from the original's own recipients instead. The Cc entry is + // included because a reply to a conversation the user started belongs to + // everyone who was on it. + ParsedMessage message; + message.from = QStringLiteral("Me <me@example.org>"); + message.to = QStringLiteral("Correspondent <them@example.net>"); + message.cc = QStringLiteral("watcher@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/false, { QStringLiteral("me@example.org") }, &to, &cc); + + const QString joined = to.join(QLatin1Char('|')); + QVERIFY2(joined.contains(QStringLiteral("them@example.net")), + qPrintable(QStringLiteral("To was %1").arg(joined))); + QVERIFY2(joined.contains(QStringLiteral("watcher@example.com")), + qPrintable(QStringLiteral("To was %1").arg(joined))); + // The whole point: the user is not written back to themselves. + QVERIFY2(!joined.contains(QStringLiteral("me@example.org")), + qPrintable(QStringLiteral("the reply addressed the user: %1").arg(joined))); + QVERIFY2(cc.isEmpty(), "a plain reply produced a Cc"); +} + +void TestComposeContext::aReplyAllToOwnMessageDoesNotRepeatToInCc() +{ + // Reply-all to your own message MIRRORS the original's split: its To + // becomes To, its Cc becomes Cc. The split is the message's meaning, To + // being "addressed to you" and Cc "for information", and promoting a Cc'd + // party to To is visible to every recipient. + // + // Asserted per FIELD, not on the union. A test counting each address once + // across to + cc passes whether the split is preserved or collapsed, which + // is how the collapse shipped and survived its first mutation check. + ParsedMessage message; + message.from = QStringLiteral("Me <me@example.org>"); + message.to = QStringLiteral("them@example.net"); + message.cc = QStringLiteral("watcher@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/true, { QStringLiteral("me@example.org") }, &to, &cc); + + const QString toJoined = to.join(QLatin1Char('|')); + const QString ccJoined = cc.join(QLatin1Char('|')); + QVERIFY2(toJoined.contains(QStringLiteral("them@example.net")), + qPrintable(QStringLiteral("To was %1").arg(toJoined))); + QVERIFY2(!toJoined.contains(QStringLiteral("watcher@example.com")), + qPrintable(QStringLiteral("a Cc recipient was promoted to To: %1").arg(toJoined))); + QVERIFY2(ccJoined.contains(QStringLiteral("watcher@example.com")), + qPrintable(QStringLiteral("Cc was %1").arg(ccJoined))); + QVERIFY2(!ccJoined.contains(QStringLiteral("them@example.net")), + qPrintable(QStringLiteral("the To address repeated in Cc: %1").arg(ccJoined))); + // The whole point of the self-reply rule. + QVERIFY2(!(toJoined + ccJoined).contains(QStringLiteral("me@example.org")), + "the reply addressed the user"); +} + +void TestComposeContext::aCoSenderIsStillRepliedTo() +{ + // A message the user sent WITH somebody else is not a message to oneself. + // Only an all-own sender diverts To to the original recipients; here the + // co-sender is a real person expecting the reply. + ParsedMessage message; + message.from = QStringLiteral("Me <me@example.org>, Other <other@example.net>"); + message.to = QStringLiteral("them@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/false, { QStringLiteral("me@example.org") }, &to, &cc); + + const QString joined = to.join(QLatin1Char('|')); + QVERIFY2(joined.contains(QStringLiteral("other@example.net")), + qPrintable(QStringLiteral("To was %1").arg(joined))); + QVERIFY2(!joined.contains(QStringLiteral("them@example.com")), + qPrintable(QStringLiteral("a plain reply reached the original's To: %1") + .arg(joined))); +} + +void TestComposeContext::anUnparseableSenderStillProducesARecipient() +{ + // "From: Mailer Daemon" is a bare display name with no angle brackets, which + // is what bounces and some automated senders emit. It parses to ZERO + // mailboxes, so the sender contributes nothing and To would otherwise come + // out empty. + // + // An empty To is the worst outcome available here, because MessageBuilder + // treats an empty recipient list as success: the message reaches the send + // command with nobody to deliver to and a copy is filed in Sent that looks + // sent and reached no one. The original's own recipients are the remaining + // candidates. + ParsedMessage message; + message.from = QStringLiteral("Mailer Daemon"); + message.to = QStringLiteral("them@example.net"); + message.cc = QStringLiteral("watcher@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/false, { QStringLiteral("me@example.org") }, &to, &cc); + + QVERIFY2(!to.isEmpty(), "an unparseable sender produced a reply with no recipient"); + QVERIFY2(to.join(QLatin1Char('|')).contains(QStringLiteral("them@example.net")), + qPrintable(QStringLiteral("To was %1").arg(to.join(QLatin1Char('|'))))); + + // Reply-all is the worse half: without the fallback it puts every recipient + // in Cc and leaves To empty, which is a message addressed to nobody. + QStringList allTo; + QStringList allCc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/true, { QStringLiteral("me@example.org") }, &allTo, &allCc); + QVERIFY2(!allTo.isEmpty(), "a reply-all to an unparseable sender left To empty"); +} + +void TestComposeContext::aReplyAllPrefersReplyToForTheToField() +{ + // Reply-To precedence is not a plain-Reply-only rule: a list's reply-all + // must also go to the list rather than to the individual poster. + ParsedMessage message; + message.from = QStringLiteral("Poster <poster@example.org>"); + message.replyTo = QStringLiteral("List <list@example.net>"); + message.to = QStringLiteral("list@example.net"); + message.cc = QStringLiteral("watcher@example.com"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply(message, /*replyAll=*/true, {}, &to, &cc); + + QCOMPARE(to.size(), 1); + QVERIFY2(to.at(0).contains(QStringLiteral("list@example.net")), + qPrintable(QStringLiteral("To was %1").arg(to.join(QLatin1Char('|'))))); + // The list is in To, so it must not repeat in Cc even though the original's + // To named it. + QVERIFY2(!cc.join(QLatin1Char('|')).contains(QStringLiteral("list@example.net")), + qPrintable(QStringLiteral("the To address repeated in Cc: %1") + .arg(cc.join(QLatin1Char('|'))))); + QVERIFY(cc.join(QLatin1Char('|')).contains(QStringLiteral("watcher@example.com"))); +} + +void TestComposeContext::aDisplayNameContainingAnOwnAddressIsNotMistakenForIt() +{ + // A stranger whose DISPLAY NAME quotes the user's address. Comparing the + // rendered whole rather than the addr-spec would strip a real recipient, + // and the reply would silently not reach them. + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.to = QStringLiteral("\"about me@example.org\" <stranger@example.net>"); + + QStringList to; + QStringList cc; + ComposeContextBuilder::recipientsForReply( + message, /*replyAll=*/true, { QStringLiteral("me@example.org") }, &to, &cc); + + QVERIFY2((to + cc).join(QLatin1Char('|')).contains(QStringLiteral("stranger@example.net")), + "a stranger was stripped because their display name quoted an own address"); +} + +// --------------------------------------------------------------------------- +// References +// --------------------------------------------------------------------------- + +void TestComposeContext::referencesCarryTheOriginalChainPlusItsId() +{ + ParsedMessage message; + message.messageId = QStringLiteral("current@example.org"); + message.references = + QStringLiteral("<first@example.org> <second@example.org>"); + + const QStringList refs = ComposeContextBuilder::referencesForReply(message); + + QCOMPARE(refs.size(), 3); + QCOMPARE(refs.at(0), QStringLiteral("first@example.org")); + QCOMPARE(refs.at(1), QStringLiteral("second@example.org")); + QCOMPARE(refs.at(2), QStringLiteral("current@example.org")); +} + +void TestComposeContext::referencesDoNotRepeatTheMessageId() +{ + ParsedMessage message; + message.messageId = QStringLiteral("current@example.org"); + message.references = QStringLiteral("<first@example.org> <current@example.org>"); + + const QStringList refs = ComposeContextBuilder::referencesForReply(message); + + QCOMPARE(refs.count(QStringLiteral("current@example.org")), 1); + QCOMPARE(refs.last(), QStringLiteral("current@example.org")); +} + +// --------------------------------------------------------------------------- +// Subjects +// --------------------------------------------------------------------------- + +void TestComposeContext::aCommaSeparatedReferencesHeaderIsSplitIntoIds() +{ + // `<a@x>,<b@y>` is not conformant, RFC 5322 has no comma here, but some + // clients emit it. Splitting on whitespace alone makes that whole header ONE + // token, and stripping its outer brackets then yields the fabricated id + // `a@x>,<b@y`, which is sent to the recipient as a Message-ID reference. A + // comma cannot occur inside a msg-id, so accepting it as a separator is free. + ParsedMessage message; + message.references = QStringLiteral("<first@example.org>,<second@example.org>"); + message.messageId = QStringLiteral("current@example.org"); + + const QStringList refs = ComposeContextBuilder::referencesForReply(message); + + QCOMPARE(refs.size(), 3); + QCOMPARE(refs.at(0), QStringLiteral("first@example.org")); + QCOMPARE(refs.at(1), QStringLiteral("second@example.org")); + QCOMPARE(refs.at(2), QStringLiteral("current@example.org")); +} + +void TestComposeContext::aReplySubjectDoesNotDoubleItsPrefix() +{ + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("Hello")), + QStringLiteral("Re: Hello")); + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("Re: Hello")), + QStringLiteral("Re: Hello")); + // Case and spacing vary between clients and neither justifies a second + // prefix. "RE:" from Outlook is the common one. + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("RE: Hello")), + QStringLiteral("RE: Hello")); + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("re:Hello")), + QStringLiteral("re:Hello")); +} + +void TestComposeContext::aForwardSubjectDoesNotDoubleItsPrefix() +{ + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("Hello")), + QStringLiteral("Fwd: Hello")); + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("Fwd: Hello")), + QStringLiteral("Fwd: Hello")); + // "Fw:" is the other common spelling and means the same thing. + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("Fw: Hello")), + QStringLiteral("Fw: Hello")); +} + +void TestComposeContext::anEmptySubjectStillGetsAPrefix() +{ + // A reply to a subjectless message is still a reply. "Re: " alone is + // correct and is what every other client produces. + QCOMPARE(ComposeContextBuilder::replySubject(QString()), + QStringLiteral("Re: ")); +} + +void TestComposeContext::aSubjectMentioningReLaterStillGetsAPrefix() +{ + // The prefix test is ANCHORED. An unanchored search would see "re:" inside + // an ordinary subject and refuse to prefix a genuine first reply, which + // breaks threading in the recipient's client. + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("Notes re: budget")), + QStringLiteral("Re: Notes re: budget")); + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("Notes fwd: budget")), + QStringLiteral("Fwd: Notes fwd: budget")); +} + +void TestComposeContext::aNonEnglishPrefixIsNotDoubled() +{ + // A mixed-locale mailbox, which this one is. An English-only pattern turns + // every one of these into "Re: AW: subject", and the round after that into + // "Re: Re: AW:". + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("AW: Angebot")), + QStringLiteral("AW: Angebot")); + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("SV: innkalling")), + QStringLiteral("SV: innkalling")); + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("RES: pedido")), + QStringLiteral("RES: pedido")); + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("WG: Angebot")), + QStringLiteral("WG: Angebot")); + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("TR: document")), + QStringLiteral("TR: document")); +} + +void TestComposeContext::aCountedPrefixIsNotDoubled() +{ + // Outlook and some list managers count the rounds. Same meaning, and + // prefixing again produces "Re: Re[2]:". + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("Re[2]: thread")), + QStringLiteral("Re[2]: thread")); + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("Re(3): thread")), + QStringLiteral("Re(3): thread")); +} + +void TestComposeContext::aSingleLetterBeforeAColonIsNotAPrefix() +{ + // Italian clients do send "R:" and "I:", and they are deliberately NOT + // recognised. Measured 2026-08-21: with them in the pattern, "R: report on + // Q3" reads as an existing prefix, so a genuine FIRST reply gets no "Re:" + // and threads nowhere in the recipient's client, with nothing wrong to see + // locally. A doubled "Re: R:" is cosmetic; broken threading is not. + // + // "F:" is here for the same reason: the pattern was once `fwd?`, which + // matched it. + QCOMPARE(ComposeContextBuilder::replySubject(QStringLiteral("R: report on Q3")), + QStringLiteral("Re: R: report on Q3")); + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("I: notes")), + QStringLiteral("Fwd: I: notes")); + QCOMPARE(ComposeContextBuilder::forwardSubject(QStringLiteral("F: results")), + QStringLiteral("Fwd: F: results")); +} + +// --------------------------------------------------------------------------- +// Account resolution +// --------------------------------------------------------------------------- + +void TestComposeContext::theReplyAccountComesFromTheMessagesMaildir() +{ + // The dropdown is NOT consulted: replying from the All accounts view to a + // message that arrived at account B sends from B. + const QString path = writeConfig(QStringLiteral( + "[account.work]\nmaildir=work\ntrash=Trash\naddress=work@example.org\n" + "send_command=/bin/true\n" + "[account.home]\nmaildir=home\ntrash=Trash\naddress=home@example.org\n" + "send_command=/bin/true\n")); + QVERIFY(!path.isEmpty()); + + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + const QString account = ComposeContextBuilder::accountForReply( + config, { QStringLiteral("/mail/home/INBOX/cur/123") }, + { QStringLiteral("home@example.org") }, QStringLiteral("/mail")); + + QCOMPARE(account, QStringLiteral("home")); +} + +void TestComposeContext::anAccountIsNotMatchedByAPrefixOfItsMaildir() +{ + // "work" must not claim a message living in "work-archive". Without the + // separator in the comparison it does, and the reply is sent from the + // wrong account. + // + // The account KEYS are chosen so the wrong answer is reached FIRST. + // Config builds its list from QSettings::childGroups(), which returns + // groups ALPHABETICALLY rather than in file order, so the section order + // here decides nothing and only the keys do. With "archive" before "work" + // the loop happens upon the correct account before it can mismatch, and + // the test passes against the bug: measured, a mutation dropping the + // separator left the suite fully green. "a-work" (maildir "work") sorts + // before "b-archive" (maildir "work-archive") and puts the prefix + // candidate first, where a textual comparison matches it. + const QString path = writeConfig(QStringLiteral( + "[account.a-work]\nmaildir=work\ntrash=Trash\naddress=work@example.org\n" + "send_command=/bin/true\n" + "[account.b-archive]\nmaildir=work-archive\ntrash=Trash\n" + "address=archive@example.org\nsend_command=/bin/true\n")); + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + // The ordering the mutation depends on, asserted rather than assumed: if + // Config ever sorts differently this test silently stops testing anything. + QCOMPARE(config.accounts().at(0).key, QStringLiteral("a-work")); + + const QString account = ComposeContextBuilder::accountForReply( + config, { QStringLiteral("/mail/work-archive/INBOX/cur/1") }, + {}, QStringLiteral("/mail")); + + QCOMPARE(account, QStringLiteral("b-archive")); +} + +void TestComposeContext::anAmbiguousMessagePrefersTheMatchingRecipient() +{ + // One message, two maildirs: on a list twice under two addresses. The + // recipient headers are the tiebreak. + const QString path = writeConfig(QStringLiteral( + "[account.work]\nmaildir=work\ntrash=Trash\naddress=work@example.org\n" + "send_command=/bin/true\n" + "[account.home]\nmaildir=home\ntrash=Trash\naddress=home@example.org\n" + "send_command=/bin/true\n")); + QVERIFY(!path.isEmpty()); + + Config config; + config.load(path); + + const QString account = ComposeContextBuilder::accountForReply( + config, + { QStringLiteral("/mail/work/Lists/cur/1"), + QStringLiteral("/mail/home/Lists/cur/1") }, + { QStringLiteral("home@example.org") }, QStringLiteral("/mail")); + + QCOMPARE(account, QStringLiteral("home")); +} + +void TestComposeContext::anAmbiguousMessageWithNoMatchTakesTheFirst() +{ + // Arbitrary, and deliberately so: the From field shows the choice, which + // makes an arbitrary resolution visible rather than hidden. + const QString path = writeConfig(QStringLiteral( + "[account.work]\nmaildir=work\ntrash=Trash\naddress=work@example.org\n" + "send_command=/bin/true\n" + "[account.home]\nmaildir=home\ntrash=Trash\naddress=home@example.org\n" + "send_command=/bin/true\n")); + Config config; + config.load(path); + + const QString account = ComposeContextBuilder::accountForReply( + config, + { QStringLiteral("/mail/work/Lists/cur/1"), + QStringLiteral("/mail/home/Lists/cur/1") }, + { QStringLiteral("someone-else@example.org") }, QStringLiteral("/mail")); + + QVERIFY2(!account.isEmpty(), "an ambiguous message resolved to no account"); + QCOMPARE(account, QStringLiteral("work")); +} + +void TestComposeContext::aNewMessagePrefersTheSelectedAccount() +{ + const QString path = writeConfig(QStringLiteral( + "[account.work]\nmaildir=work\ntrash=Trash\nsend_command=/bin/true\n" + "[account.home]\nmaildir=home\ntrash=Trash\nsend_command=/bin/true\n")); + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + QCOMPARE(ComposeContextBuilder::accountForNew(config, QStringLiteral("home")), + QStringLiteral("home")); +} + +void TestComposeContext::aNewMessageFallsThroughASelectedAccountThatCannotSend() +{ + // Rule 1 requires the selected account CAN send. Viewing a receive-only + // account and pressing compose must produce a working composer from + // another account, not a broken one from this. + const QString path = writeConfig(QStringLiteral( + "[account.listsonly]\nmaildir=listsonly\ntrash=Trash\n" + "[account.work]\nmaildir=work\ntrash=Trash\nsend_command=/bin/true\n")); + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + QCOMPARE(ComposeContextBuilder::accountForNew(config, QStringLiteral("listsonly")), + QStringLiteral("work")); +} + +void TestComposeContext::aNewMessageUsesDefaultAccountFromAllAccounts() +{ + // The All accounts view has no selected account and falls through to rule 2. + // + // The named account must NOT also be what rule 4 would answer, or the test + // passes with rule 2 deleted outright: measured, a mutation removing it + // left the suite green because the account list is ALPHABETICAL (Config + // builds it from QSettings::childGroups()) and the section order in this + // string decides nothing. "zeta" sorts last, so rule 4 would answer + // "alpha" and only rule 2 can produce "zeta". + const QString path = writeConfig(QStringLiteral( + "[account.alpha]\nmaildir=alpha\ntrash=Trash\nsend_command=/bin/true\n" + "[account.zeta]\nmaildir=zeta\ntrash=Trash\nsend_command=/bin/true\n" + "[compose]\ndefault_account=zeta\n")); + Config config; + config.load(path); + QCOMPARE(config.compose().defaultAccount, QStringLiteral("zeta")); + // Asserted rather than assumed, so the test stops silently proving nothing + // if Config ever changes its ordering. + QCOMPARE(config.sendingAccounts().first().key, QStringLiteral("alpha")); + + QCOMPARE(ComposeContextBuilder::accountForNew(config, QString()), + QStringLiteral("zeta")); +} + +void TestComposeContext::aNewMessageUsesStartupAccountWhenNoDefaultIsSet() +{ + // Rule 3. Same ordering trap as rule 2: "zeta" must not be what rule 4 + // would answer, or a test for this rule passes with the rule deleted. + const QString path = writeConfig(QStringLiteral( + "[general]\nstartup_account=zeta\n" + "[account.alpha]\nmaildir=alpha\ntrash=Trash\nsend_command=/bin/true\n" + "[account.zeta]\nmaildir=zeta\ntrash=Trash\nsend_command=/bin/true\n")); + Config config; + config.load(path); + QCOMPARE(config.startupAccount(), QStringLiteral("zeta")); + QVERIFY(config.compose().defaultAccount.isEmpty()); + QCOMPARE(config.sendingAccounts().first().key, QStringLiteral("alpha")); + + QCOMPARE(ComposeContextBuilder::accountForNew(config, QString()), + QStringLiteral("zeta")); +} + +void TestComposeContext::aNewMessageFallsBackToTheFirstSendingAccount() +{ + // Rule 4, arbitrary, and the reason rules 2 and 3 exist. The receive-only + // account is FIRST, so "the first account" and "the first sending account" + // are different answers and the test distinguishes them. + const QString path = writeConfig(QStringLiteral( + "[account.listsonly]\nmaildir=listsonly\ntrash=Trash\n" + "[account.work]\nmaildir=work\ntrash=Trash\nsend_command=/bin/true\n")); + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + QCOMPARE(ComposeContextBuilder::accountForNew(config, QString()), + QStringLiteral("work")); +} + +void TestComposeContext::aNewMessageReturnsNothingWhenNoAccountCanSend() +{ + // A valid read-only installation. The compose action is disabled, so this + // should be unreachable, and returning empty rather than a random account + // is what makes a mistake visible instead of silent. + const QString path = writeConfig(QStringLiteral( + "[account.listsonly]\nmaildir=listsonly\ntrash=Trash\n")); + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 1); + + QVERIFY(ComposeContextBuilder::accountForNew(config, QString()).isEmpty()); +} + +// --------------------------------------------------------------------------- +// Quoting +// --------------------------------------------------------------------------- + +void TestComposeContext::aQuotedBodyPrefixesEveryLine() +{ + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.date = QStringLiteral("Thu, 20 Aug 2026 10:00:00 +0200"); + message.plainBody = QStringLiteral("first line\nsecond line\n\nafter a blank"); + + const QString quoted = ComposeContextBuilder::quoteBody(message); + + QVERIFY2(quoted.contains(QStringLiteral("> first line")), + qPrintable(QStringLiteral("first line not quoted:\n%1").arg(quoted))); + QVERIFY2(quoted.contains(QStringLiteral("> second line")), + "second line not quoted"); + // A blank line inside a quote must still carry the marker, or the quote + // visually ends there in every client that renders it. + QVERIFY2(quoted.contains(QStringLiteral("\n>\n")), + qPrintable(QStringLiteral("a blank line lost its marker:\n%1").arg(quoted))); + QVERIFY2(quoted.contains(QStringLiteral("sender@example.org")), + "no attribution line naming the sender"); + // A CRLF body must not leave a stray carriage return before every marker. + ParsedMessage crlf; + crlf.plainBody = QStringLiteral("one\r\ntwo"); + const QString quotedCrlf = ComposeContextBuilder::quoteBody(crlf); + QVERIFY2(!quotedCrlf.contains(QLatin1Char('\r')), + qPrintable(QStringLiteral("a carriage return survived quoting: %1") + .arg(quotedCrlf))); +} + +QTEST_MAIN(TestComposeContext) +#include "test_composecontext.moc" diff --git a/tests/test_messagebuilder.cpp b/tests/test_messagebuilder.cpp index 1f94784..73d388c 100644 --- a/tests/test_messagebuilder.cpp +++ b/tests/test_messagebuilder.cpp @@ -49,6 +49,7 @@ private slots: void anAccentedBodyIsUtf8QuotedPrintable(); void anAccentedSubjectIsRfc2047Utf8(); void inReplyToAndReferencesAreCarried(); + void bareMessageIdsAreBracketedRatherThanEmittedEmpty(); void attachmentsProduceMultipartMixed(); void aMissingAttachmentFailsTheBuild(); void aDirectoryAttachmentFailsRatherThanHangingTheProcess(); @@ -218,6 +219,38 @@ void TestMessageBuilder::inReplyToAndReferencesAreCarried() QVERIFY2(text.contains(QStringLiteral("<older@example.org>")), qPrintable(text)); } +/// **The brackets are syntax, and a bare id ships an EMPTY header rather than a +/// malformed one.** This is what every real caller supplies: GMime strips the +/// brackets when MimeParser reads Message-ID, and +/// ComposeContextBuilder::referencesForReply strips them from the References +/// chain so the two agree, so both values arrive here bare. +/// +/// Measured 2026-08-21: handed `orig@example.org`, GMime wrote `In-Reply-To:` +/// with no value at all and did not complain. Every reply would have arrived as +/// an orphan thread in the recipient's client, with nothing wrong to see +/// locally. Asserted on the FULL header line, since a test for the id alone +/// passes against an empty header that merely contains the name. +void TestMessageBuilder::bareMessageIdsAreBracketedRatherThanEmittedEmpty() +{ + OutgoingMessage m = baseMessage(); + m.inReplyTo = QStringLiteral("orig@example.org"); + m.references = QStringList{QStringLiteral("older@example.org"), + QStringLiteral("orig@example.org")}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("In-Reply-To: <orig@example.org>")), qPrintable(text)); + QVERIFY2(text.contains( + QStringLiteral("References: <older@example.org> <orig@example.org>")), + qPrintable(text)); + // The failure this exists for: the header present and empty. + QVERIFY2(!text.contains(QStringLiteral("In-Reply-To:\r\n")) + && !text.contains(QStringLiteral("In-Reply-To:\n")), + qPrintable(text)); +} + /// The attachment wrapper must NEST the body, not sit beside it: multipart/mixed /// outermost, with the multipart/alternative as its first part. Beside it, a /// client would show the alternatives as attachments and the body would be |