feat: add image thumbnail generation add missing file_name scan destination - #116
Conversation
celestix
left a comment
There was a problem hiding this comment.
Thanks for this. The Scan fix in 804008d is a real bug fix: SelectMessageMediaByMessageID returns 10 columns and GetMessageWithMediaByID scanned 9, so every uncached image download failed on master. That commit is good to go on its own.
The feature commit needs rework before merge. Verified at PR head in a clean checkout: go build, go vet, go test ./... pass; vite build passes; vitest 80/80; bindings regenerate. cd systray && go build fails (see below).
Blockers
1. generateThumbnail decodes arbitrary images with no dimension guard (api/media.go:118)
image.Decode runs on the full cached file with no DecodeConfig check, byte cap, or concurrency bound. A small encoded PNG with huge dimensions (20000x20000 is ~1.7MB on disk, several GB decoded) OOM-kills the process. One goroutine per mounted row, so several decode at once. The crash happens before CacheThumbnail persists anything and the file stays cached, so reopening the chat crashes again. Guard with image.DecodeConfig plus a pixel budget, cap len(data), and serialize decodes.
2. WebP as sticker silently drops caption, mentions and reply (frontend/src/screens/ChatDetail.tsx:812)
Every image/webp picked in the attachment dialog is now sent as type sticker. The frontend still passes text, quotedMessageId and mentions (ChatDetail.tsx:558-569), but the backend sticker branch (api/message.go:427-460) builds StickerMessage{Mimetype} only. StickerMessage has no caption field and ContextInfo/MentionedJID are never set, so the recipient gets a bare, arbitrary-size sticker and the composer has already been cleared. The paste path still sends the same WebP as an image, so behaviour differs by input method, and FILE_TYPE_ICONS in ChatInput.tsx has no sticker key so the attachment preview shows no icon. On master this was sent as a captioned image. Suggest mapping WebP to image by default and offering sticker as an explicit choice, or making the sticker branch carry ContextInfo and mentions.
3. Document download now saved as .jpg through the image cache (api/media.go:419)
The new GetCachedImage fallback in DownloadImageToFile is reachable from the document bubble's download button (MessageItem.tsx:306 -> handleImageDownload -> DownloadImageToFile). On master that was a silent no-op because of the Scan bug. Now it downloads the whole document (no size cap), stores it in the image cache as images/<sha>.jpg with mime application/pdf, builds a base64 data URL of the whole file, decodes it again, and getFileExtension("application/pdf") returns .jpg, so ~/Downloads/<id>.jpg contains PDF bytes and doc.fileName is ignored. Afterwards GetCachedImages ships that PDF data URL to the frontend on every chat open. Gate the fallback on image media types, and share a bytes-returning fetch helper with GetCachedImage instead of round-tripping through the data URL.
4. Wails bump breaks the systray build (systray/go.mod:8)
go.mod moves wails/v2 from 2.13.0 to 2.15.0 but systray/go.mod and systray/go.sum (which pull the root module through replace ../) still pin 2.13.0. cd systray && go build . fails with go: updates to go.mod needed; to update it: go mod tidy, so scripts/build.sh fails for local contributors. CI only survives because release.yaml runs go mod tidy first. Please commit the tidy result (go.mod bump plus 8 go.sum lines). The Wails bump is also unrelated to this feature and would be cleaner as its own PR.
5. CacheThumbnail bypasses the serialized writer (internal/store/message.go:1606)
Writes message_media directly on a pooled connection outside the store's single-writer goroutine and discards the error. Reproduced with the same driver, pragmas and DSN: while a writer transaction holds the WAL write lock (MigrateLIDToPN runs in one tx across all messages on every Connected event, history-sync bursts too), the UPDATE blocked 5s (busy_timeout) then returned database is locked and nothing was stored; the frontend promise for that row waits the whole time. The reverse also happens: a CacheThumbnail commit landing between MigrateLIDToPN's SELECT and its first UPDATE makes every UPDATE in that tx fail with SQLITE_BUSY_SNAPSHOT, the loop logs and continues, and the app logs "migration completed successfully" having migrated nothing. Route through enqueueWrite like the rest of the store. CacheLinkPreviewThumbnail has the same pre-existing pattern and could be fixed at the same time.
6. Download path built from the unsanitised message ID (api/media.go:442, pre-existing)
fileName := messageID + ext; filePath := filepath.Join(downloadsDir, fileName). The stanza id comes from the sender verbatim. An id like ../.ssh/authorized_keys resolves to ~/.ssh/authorized_keys.jpg and os.WriteFile writes there. The line predates this PR, but the PR rewrites the function and the new fallback widens reachability, so it is worth fixing here. Sanitise with filepath.Base or an allow-list, or derive the name from stored metadata.
Batch preload does not achieve its goal
7. Whole page in one IPC string, evicts on-screen images (MediaContent.tsx:49)
preloadImages requests the full base64 data URL of every cached image among all 50 IDs of a page (not just image messages) through one uncapped callback string. Measured 50 x 300KB gives a 19MB JS string and ~200MB transient Go allocation; 50 x 2MB gives 127MB and a ~3s UI freeze. The results are bulk-set into the 48-entry / 32MB imagePathCache in sorted-key order, so with 8 visible images plus a 30-image batch none of the visible entries survive and a Virtuoso remount re-fetches them, which is the flicker the cache exists to prevent. Filter to image/sticker IDs near the viewport, cap the batch by bytes, or batch the small message_media thumbnails instead.
8. Mounted rows never read the batch result, so visible images are fetched twice (MediaContent.tsx:142)
The batch only feeds the useState initializer. Rows already mounted when GetCachedImages resolves (all first-screen rows) still call GetCachedImage themselves, because handleDownload goes straight to loadMediaOnce without checking imagePathCache or joining the batch. Net effect for open-and-read is more IPC than master. Register per-id promises through loadMediaOnce so mounted rows await the batch, and short-circuit handleDownload on imagePathCache.get(id).
9. preloadInFlight gate drops every overlapping batch (MediaContent.tsx:48)
if (preloadInFlight) return discards any call made while a previous batch is pending, with no queue, merge, or retry. Verified with a deferred mock: preloadImages(['a','b']) then preloadImages(['c','d']) while pending results in one call and c,d are never fetched. Triggers: load-more while the initial batch is in flight, the page-by-page loop in handleQuotedClick, and switching chats while the previous chat's batch is pending. Queue or merge pending IDs instead of gating on a single module-level promise.
10. Thumbnail effect fires on every mount, ungated and undeduped (MediaContent.tsx:260)
GetImageThumbnail is called on every mount of every image row, not gated by the IntersectionObserver or the 200ms debounce, without loadMediaOnce dedup, and an empty result is never cached. Optimistic temp-... rows also call it. Own-sent images (JPEGThumbnail nil) and pre-thumbnail-column history are exactly the rows that return empty, so every scroll-through costs an IPC plus two SQLite reads per row. Gate on visibility, route through loadMediaOnce("thumb:" + id), and store a miss sentinel in imageThumbCache.
Thumbnail generator correctness
11. Wrong previews are cached permanently (api/media.go:146)
Verified with the function as written: a 1600x4 PNG gives dstH = int(4 * 0.2) = 0 and jpeg.Encode emits a JPEG with height 0 that WebKitGTK refuses (the thumbnail <img> has no onError, so the row shows a broken glyph); an 800x800 PNG of (255,255,255,0) produces black pixels on both paths because alpha is premultiplied then dropped; a JPEG with EXIF Orientation=6 produces an unrotated preview while WebKit renders the full image upright. CacheThumbnail stores the result and GetThumbnail short-circuits afterwards, so there is no regeneration path. Clamp destination dims to at least 1, composite onto white before encoding, and apply EXIF orientation before resampling.
12. GIF/WebP never get a thumbnail and the PNG retry is dead code (api/media.go:120)
Only image/jpeg and image/png decoders are linked. GIF and WebP images fail image.Decode every time, return empty before CacheThumbnail, and the frontend caches nothing, so the IPC, two queries and full file read repeat on each remount. The png.Decode retry re-runs the decoder image.Decode already tried. Register image/gif and golang.org/x/image/webp (or skip by mime before reading the file), delete the retry, and store a negative result.
13. INSERT OR REPLACE wipes generated thumbnails on re-delivery (internal/query/message_media.go:34)
InsertMessage always passes the wire JPEGThumbnail (nil for images sent from this app) and InsertMessageMedia is INSERT OR REPLACE, so any re-delivery of a stored message NULLs the generated thumbnail. processHistorySync re-inserts every message in every batch with no existence check, so re-pairing after an external logout replays history over old rows and clears every generated thumbnail. Self-healing, but it repeats the expensive path per image per re-delivery. Use an upsert that preserves a non-null thumbnail (COALESCE(excluded.thumbnail, thumbnail)) or store generated previews separately.
Smaller items
14. Download button in the thumbnail branch is unreachable (MediaContent.tsx:372)
showDownloadButton is only set by the mouse handlers on the mediaSrc branch (lines 286-287); the thumbnail wrapper has no onMouseEnter/onMouseLeave, so lines 373-384 never mount. If hover handlers are added, void handleDownload().then(() => onDownload()) also fires onDownload when handleDownload early-returned or swallowed an error, triggering a second concurrent backend download. Move the handlers onto the thumbnail wrapper and chain onDownload only on success.
15. Sent sticker echo drops _tempFile (ChatDetail.tsx:627)
The wa:new_message carry-over loop copies _tempFile only for image/video/audio bodies, and sentMediaCache (now passed to stickers) is never .set() anywhere, so a just-sent sticker flickers to a placeholder and re-downloads the sticker that was just uploaded. Add stickerMessage to the loop or cache the uploaded bytes in the sticker branch.
16. Unused use import (MediaContent.tsx:1)
use is never referenced and does not exist in the installed react@18.3.1 runtime (types come from @types/react 19). It is the only new tsc -p tsconfig.app.json error in files touched by this PR. Remove it.
Nits
DownloadImageToFile:fmt.Errorf("image not available: %v", fetchErr)prints<nil>whenfetchErris nil anddataURLis empty.- The doc comment on
DownloadImageToFilewas removed. handleDownloadRef.current = ...is assigned during render; auseEffectoruseLatest-style ref keeps it out of the render path.fileNameis scanned inGetMessageWithMediaByIDbut not propagated into the returnedMedia, so the name is still unavailable to callers.generateThumbnailis a pure function and would be easy to cover with a table test (aspect ratio, alpha, EXIF, unsupported format) alongside the existingapi/media_test.go.
image/webpas "sticker" type instead of generic "image"