From 2b32350204dfb49089c465856464a043018ca3c6 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 21 Aug 2026 15:37:54 +0200 Subject: feat(compose): derive a reply's recipients and headers, item 123 ComposeContext, task 7 of the compose-and-send plan. Address parsing, recipient derivation, the References chain, subject prefixing and account resolution, as free functions over values so they test without a painter. Recipient derivation was designed from the spec rather than transcribed: the plan's draft omitted it and its tests could not compile, calling QVERIFY(config.load(path)) against a void return. Six defects found in review, each pinned by a test checked against the mutation that breaks it: - Message-ids reached GMime bare, and GMime writes an EMPTY header for a bare addr-spec rather than complaining. In-Reply-To and References both shipped blank, so every reply would have arrived as an orphan thread with nothing wrong to see locally. MessageBuilder now brackets on write, in the one place that composes those headers rather than in each caller. - internet_address_to_string was called with FALSE for the encode flag, so a display name carrying a raw newline rendered with the newline intact. That is a header-injection primitive. - A reply to the user's own message addressed the user. It now goes to that message's original recipients, mirroring their To/Cc split, which is what the Sent view and a follow-up on unanswered mail need. - A From parsing to no mailbox left To empty, reachable from real mail ("From: Mailer Daemon"). MessageBuilder treats an empty recipient list as success, so the message would have been handed to the send command with nobody to deliver to and filed in Sent looking sent. - The References header was split on whitespace alone, so a client's non-conformant "," became one token and the bracket strip produced the fabricated id "a@x>, Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L --- src/CMakeLists.txt | 1 + src/composecontext.cpp | 517 +++++++++++++++++++++++++++++++++++++++++++++++++ src/composecontext.h | 177 +++++++++++++++++ src/messagebuilder.cpp | 55 +++++- src/mimeparser.cpp | 2 + src/mimeparser.h | 16 ++ 6 files changed, 764 insertions(+), 4 deletions(-) create mode 100644 src/composecontext.cpp create mode 100644 src/composecontext.h (limited to 'src') 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. + * + * 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 + +#include "composecontext.h" + +#include "config.h" +#include "mimeparser.h" + +#include +#include +#include + +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 ": 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 *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::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 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" ` + // 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 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 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 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 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 parsed = parseAddressHeader(header); + for (const Recipient &recipient : parsed) + appendUnlessSuppressed(recipient, ownAddresses, &seen, ccOut); + } +} + +QStringList ComposeContextBuilder::referencesForReply(const ParsedMessage &message) +{ + QStringList references; + QSet 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 , and real mail + // wraps it across lines, so whitespace is the conformant separator. + // + // Commas are accepted BESIDES whitespace because some clients emit + // `,`, 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>, 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. + * + * 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 +#include +#include + +#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 " 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" , 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 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 @@ -109,6 +109,35 @@ GMimePart *makeTextPart(const char *subtype, const QString &text) /// strict. `garbage` and `""` both parse to a one-entry list. This function /// 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) { @@ -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 . + /// + /// 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; -- cgit v1.2.3