diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-03 17:18:10 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-03 17:18:10 +0200 |
| commit | abed584d02fdd89a22b37ef3da0b41f8d77f48b8 (patch) | |
| tree | 305c0e3d8af9905a06c95d8d72428e6665991721 /src/mimeparser.cpp | |
| parent | e45f68b04b5ee2400a7885d5f5b054a889061df5 (diff) | |
| download | qtmaildir-abed584d02fdd89a22b37ef3da0b41f8d77f48b8.tar.gz qtmaildir-abed584d02fdd89a22b37ef3da0b41f8d77f48b8.zip | |
feat: make attachments reachable from the message panefeature/attachments
The attachment bar had been an empty placeholder since it was written:
MessageView created it and added it to the layout, and nothing ever put
anything in it. MimeParser had been extracting attachments the whole
time and Attachment::saveTo() already carried the path-traversal guard,
so the backend needed calling rather than writing.
The bar holds one "Attachments (N)..." button whatever the count. One
button per file was built first and was wrong: a thread with sixteen of
them made the bar as wide as the window, pushed the splitter over and
left the thread list a few pixels wide. The button opens a dialog
listing message number, filename and size with a Save each, and a
"Save all..." when there is more than one.
Save all writes into a new subdirectory named "<date> <subject>" inside
a parent the user picks, rather than dropping sixteen files loose among
whatever is already there. Zipping was considered and rejected: Qt ships
no zip API, so a real archive meant a new build dependency or shelling
out to /usr/bin/zip at runtime, and a subdirectory answers the actual
requirement. The picker names the subfolder before the user commits to a
location.
The subject is attacker-controlled and becomes a directory name, so
attachmentFolderName() sits beside the other guards in mimeparser.cpp:
it strips separators, control characters and leading dots, caps the
length, and falls back to a generated name. Its test asserts that every
hostile subject still resolves inside the parent directory.
Two defects surfaced while using it, both silent:
saveTo() overwrites an existing file, and several messages in one thread
commonly attach the same filename. Saving that thread destroyed six of
sixteen files while reporting all sixteen as saved. The batch path now
uses saveWithoutOverwriting(), which appends " (2)" before the extension
and keeps a compound extension whole.
Qt::RFC2822Date rejects a Date header that carries a timezone comment,
"+0200 (CEST)", which is legal per RFC 5322 and common in real mail. Qt
refuses the entire string rather than ignoring the comment, so every
such message lost its date prefix. Comments are stripped before parsing.
Opening an attachment in its default application is deliberately not
included: handing a file from a stranger to xdg-open is a different
security decision from writing it where the user asked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src/mimeparser.cpp')
| -rw-r--r-- | src/mimeparser.cpp | 103 |
1 files changed, 103 insertions, 0 deletions
diff --git a/src/mimeparser.cpp b/src/mimeparser.cpp index 71cfc64..f5ef38a 100644 --- a/src/mimeparser.cpp +++ b/src/mimeparser.cpp @@ -164,6 +164,109 @@ QString Attachment::safeFilename() const return name; } +QString Attachment::saveWithoutOverwriting(const QString &directory, + QString *error) const +{ + const QString name = safeFilename(); + const QFileInfo info(name); + const QString base = info.completeBaseName(); + // Kept whole: "archive.tar.gz" must not become "archive (2).gz". + const QString suffix = info.suffix().isEmpty() + ? QString() + : QLatin1Char('.') + info.suffix(); + + const QDir dir(directory); + QString candidate = name; + for (int n = 2; dir.exists(candidate); ++n) + candidate = QStringLiteral("%1 (%2)%3").arg(base).arg(n).arg(suffix); + + // The containment check still applies: candidate is derived from + // safeFilename(), but the guarantee belongs at the write, not upstream. + const QString target = dir.absoluteFilePath(candidate); + if (!isPathInsideDirectory(directory, target)) { + if (error) { + *error = QStringLiteral("Refusing to write outside %1") + .arg(QDir::cleanPath(QDir(directory).absolutePath())); + } + return {}; + } + + QFile file(target); + if (!file.open(QIODevice::WriteOnly)) { + if (error) + *error = file.errorString(); + return {}; + } + if (file.write(data) != data.size()) { + if (error) + *error = file.errorString(); + return {}; + } + file.close(); + return target; +} + +QString attachmentFolderName(const QString &rfc822Date, const QString &subject) +{ + // The date prefix sorts chronologically in a file manager. A Date: header + // that does not parse is simply dropped rather than guessed at. + // A trailing timezone comment, "... +0200 (CEST)", is legal per RFC 5322 + // and common in the wild, but Qt::RFC2822Date rejects the whole string + // when one is present (verified on Qt 6.11). Strip comments before + // parsing, or every such message silently loses its date prefix. + QString cleaned = rfc822Date; + cleaned.remove(QRegularExpression(QStringLiteral("\\s*\\([^)]*\\)"))); + cleaned = cleaned.trimmed(); + + QString prefix; + const QDateTime parsed = QDateTime::fromString(cleaned, Qt::RFC2822Date); + if (parsed.isValid()) + prefix = parsed.toString(QStringLiteral("yyyy-MM-dd")); + + // The subject is attacker-controlled and is about to become a directory + // name. Everything that could make it more than one plain component goes: + // separators, and the control characters that can hide what a name really + // is when it is displayed. + QString name = subject.simplified(); + name.remove(QLatin1Char('/')); + name.remove(QLatin1Char('\\')); + QString stripped; + stripped.reserve(name.size()); + for (const QChar c : name) { + if (!c.isNull() && c.category() != QChar::Other_Control) + stripped.append(c); + } + // Leading dots would make a hidden directory, and a name of "." or ".." + // would escape or alias the parent; removing them handles every case. + while (stripped.startsWith(QLatin1Char('.'))) + stripped.remove(0, 1); + stripped = stripped.trimmed(); + + QString combined; + if (!prefix.isEmpty() && !stripped.isEmpty()) + combined = prefix + QLatin1Char(' ') + stripped; + else if (!prefix.isEmpty()) + combined = prefix; + else + combined = stripped; + + // A subject can be far longer than a filesystem component allows. Cut to + // a conservative 120 characters, well under the usual 255-byte limit even + // once multi-byte characters are counted as bytes. + constexpr int maxLength = 120; + if (combined.size() > maxLength) + combined = combined.left(maxLength).trimmed(); + + // Nothing usable survived: no parseable date and a subject that was empty, + // punctuation, or control characters only. + if (combined.isEmpty()) { + return QStringLiteral("attachments-%1").arg( + QUuid::createUuid().toString(QUuid::Id128).left(8)); + } + + return combined; +} + bool Attachment::isPathInsideDirectory(const QString &directory, const QString &candidatePath) { // Compare candidatePath itself, not QFileInfo(candidatePath).absolutePath() |
