diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-20 19:05:07 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-20 19:05:07 +0200 |
| commit | 1dcf0a0329adfd61fcc547a976e00df412549024 (patch) | |
| tree | aa937c8830c5831ec1baeeeb1872dacca3f549c6 /src/savequerydialog.cpp | |
| parent | e35f2da3fd9c5afbbea85b432f26810c17e8318e (diff) | |
| download | qtmaildir-1dcf0a0329adfd61fcc547a976e00df412549024.tar.gz qtmaildir-1dcf0a0329adfd61fcc547a976e00df412549024.zip | |
fix(compose): refuse a directory attachment and a bad recipient, item 123
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
Diffstat (limited to 'src/savequerydialog.cpp')
0 files changed, 0 insertions, 0 deletions
