| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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 "<a@x>,<b@y>" became one token and the bracket strip produced
the fabricated id "a@x>,<b@y".
- Reply and forward prefixes were recognised in English only, doubling every
AW:, SV:, WG: and Re[2]: a mixed-locale mailbox receives. Single-letter
spellings are deliberately excluded: with R: recognised, "R: report on Q3"
reads as a prefix and a genuine first reply threads nowhere.
The mailbox-only guard in parseAddressHeader survived its first mutation
check, because removing it still yields no recipients: the invalid GObject
cast makes GMime's own assertion return NULL. That is undefined behaviour
papered over by an assertion G_DISABLE_CHECKS compiles out, so the test now
asserts on the emitted critical rather than on the count. Registering the log
handler on a NULL domain catches nothing; the criticals carry "GLib-GObject"
and "gmime".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
Two silent failures on the path that produces bytes for other people.
A directory passed the attachment guard, because QFileInfo reports a
directory as existing and readable, and opening one read-only is legal. GMime's
base64 encoder then looped on read() returning EISDIR without advancing:
measured at 2.1 million failed reads in twenty seconds and still going. Since
build() runs synchronously from autosave on the GUI thread, dragging a folder
into a composer froze the whole application with the draft unrecoverable.
isFile() also excludes device nodes and FIFOs, which block the same way.
An unparseable recipient was dropped rather than reported. The old code
skipped anything that failed to parse and then only wrote the header if what
survived was non-empty, so a message whose only recipient was mistyped was
built with no To: header at all and reported success. With msmtp -t taking its
recipients from the headers, that is a message handed to the send command with
nobody to deliver to, and a copy filed in Sent that looks sent and reached no
one. A recipient the user typed and this cannot understand now stops the send,
the way a missing attachment already does.
The directory test carries a timeout deliberately: a regression there hangs
the binary rather than failing it. Two details make that work and the first
draft had neither. It must not join the worker, since a thread stuck in the
defect never returns and the join reproduces the hang instead of reporting it,
verified by reverting the fix: with the join the binary had to be killed at
150s with no verdict, without it it reports a FAIL and exits in 15s. The
result is shared through a shared_ptr so the leaked thread cannot write into a
returned stack frame.
Also: the no-address error names the account, since it matters once several
exist; messageId is assigned once on the success path rather than set early
and cleared on each failure, which is an invariant the next early return would
forget; and the Bcc comment now records that keeping the header stores the
blind list in plaintext in the sent copy and any draft, which mbsync syncs to
the server. That is accepted knowingly, and saying so stops a later reader
"fixing" it and silently breaking blind delivery.
One correction to the review that prompted this. The claim that
internet_address_list_parse returns a zero-length list rather than NULL did not
reproduce: measured on GMime 3.2 with a standalone probe, every garbage input
tried returned NULL, and no input was found producing a non-null empty list.
The length check is kept as defensive code and is documented as such rather
than as observed behaviour, since no fixture reaches it and a mutation on it
survives the suite. The defect itself was real and is what the test kills.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
One built message serves three consumers: the autosaved draft, the bytes on
the send command's stdin, and the sent copy. A draft is therefore
byte-identical to what would be sent.
Three GMime defaults are wrong for this application and each is corrected
explicitly, because all three fail only on accented text and this user
writes Italian:
GMime encodes as iso-8859-1 unless told otherwise, so the subject carries an
explicit utf-8 argument. g_mime_text_part_set_text() encodes with whatever
charset is set when it is CALLED, so setting the charset afterwards produces
a part labelled utf-8 carrying latin-1 bytes; the content stream is built
directly instead. And neither Date nor Message-ID is generated unless asked
for, and a message without a Message-ID cannot be threaded by anything that
receives it.
Attachments are checked at build time rather than at attach time: a file can
vanish in between, and a message missing the thing it was written to carry
must never reach the send command.
An account with no address fails the build rather than producing a message
with an empty From. Config::account() returns a default-constructed Account
for an unknown key rather than failing, so without that guard a bad key
would produce silently malformed mail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
