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/messagebuilder.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-) (limited to 'src/messagebuilder.cpp') 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"); } -- cgit v1.2.3