summaryrefslogtreecommitdiffstats
path: root/docs
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-03 17:18:10 +0200
committerDanilo M. <danix@danix.xyz>2026-08-03 17:18:10 +0200
commitabed584d02fdd89a22b37ef3da0b41f8d77f48b8 (patch)
tree305c0e3d8af9905a06c95d8d72428e6665991721 /docs
parente45f68b04b5ee2400a7885d5f5b054a889061df5 (diff)
downloadqtmaildir-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 'docs')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md48
1 files changed, 47 insertions, 1 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 0e0a22f..3def99d 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -53,7 +53,7 @@ taking that too literally.
| 11 | Icon, `.desktop` file, SlackBuild | packaging | M | **partly done**: icon and `.desktop` landed, SlackBuild open |
| 13 | No visual feedback that an action stuck | feedback | S | **done** |
| 14 | Tag column unreadable, tags need another home | presentation | M | **done** |
-| 15 | Attachments are parsed but unreachable from the UI | information | M | open |
+| 15 | Attachments are parsed but unreachable from the UI | information | M | **done** |
| 16 | Delete on an already-deleted thread should undelete | behavior | S | open |
| 17 | No completion for tags in the query bar | workflow | M | open |
@@ -557,6 +557,52 @@ This is a gap in the UI only. The backend is complete and already hardened:
file. A message with an attachment named `../../etc/passwd` writes inside the
chosen directory under a sanitised name and nowhere else.
+### Outcome (done)
+
+Both halves built. The paperclip column needed no new worker query, as the
+plan expected: notmuch applies the `attachment` tag itself, so
+`ThreadSummary::hasAttachment()` reads what is already there.
+
+**The bar holds ONE button, not one per file.** The plan's "one button showing
+the filename and size" was built first and was wrong: a thread with sixteen
+attachments made the bar as wide as the window, pushed the splitter over, and
+left the thread list a few pixels wide. It is now `Attachments (N)...` opening
+a dialog that lists message number, filename and size with a `Save...` each,
+plus `Save all...` when there is more than one. No filename reaches the bar,
+so no filename length can resize anything.
+
+**`Save all` writes into a new subdirectory** named `<date> <subject>`, inside
+a parent the user picks. Chosen over zipping: Qt ships no zip API, so a real
+`.zip` meant either a new build dependency (quazip, libzip) or shelling to
+`/usr/bin/zip` at runtime, and the actual requirement was "do not drop sixteen
+files loose among hundreds of others". The picker's title names the subfolder
+before the user commits to a location.
+
+**Two defects found only by using it, both silent:**
+
+- **`saveTo()` overwrites, which destroyed six of sixteen files.** Several
+ messages in one thread commonly attach the same filename; each write landed
+ on the previous one and every one reported success, so the status line said
+ 16 while the directory held 10. `saveWithoutOverwriting()` now backs the
+ batch path, appending " (2)" before the extension and keeping a compound
+ extension whole. `saveTo()` still overwrites, which is right for a single
+ save the user just chose a location for.
+- **`Qt::RFC2822Date` rejects a date carrying a timezone comment.** A header
+ ending `+0200 (CEST)` is legal per RFC 5322 and common in real mail, and Qt
+ refuses the whole string rather than the comment, so every such message lost
+ its date prefix. Comments are stripped before parsing.
+
+**The subject is untrusted and becomes a directory name.**
+`attachmentFolderName()` lives beside the other guards in `mimeparser.cpp`,
+strips separators, control characters and leading dots, caps length at 120,
+and falls back to a generated name. Its test drives `../../etc`,
+`/etc/passwd`, `..`, `.hidden`, a backslash and a null byte, then asserts each
+result still resolves inside the parent through `isPathInsideDirectory()`.
+
+**Deferred, as the plan required:** opening an attachment in its default
+application. That hands a stranger's file to `xdg-open` and is a separate
+decision from writing it to a directory the user chose.
+
## 16. Delete on an already-deleted thread should undelete
**Observed:** hitting Delete twice on the same message is a natural way to