Fix/http streaming image transfer#15
Merged
Merged
Conversation
The app transferred entire image files as Base64 strings over Wails IPC, causing memory spikes (~300MB peak for a 50MB file) that could hang or crash the UI. Changes: - Add handler.go with AssetServer Middleware serving /api/image (GET) and /api/save (POST) endpoints - OpenImage: return an HTTP URL instead of a Base64 data URL; only EXIF metadata is sent over IPC - SaveImage: open native dialog via IPC, then receive binary via HTTP POST (toBlob + fetch) instead of toDataURL IPC - Add post-write MIME verification on save to preserve the security check from the old Base64 flow - Pass detected MIME type to frontend for reliable PNG/JPEG format detection Uses Middleware (not Handler) because Wails v2 dev mode proxies all requests to Vite first, whose SPA fallback returns 200 for unknown paths, preventing the Handler fallback from ever being reached. Closes #9
- Add getCurrentImagePath() getter on App for thread-safe encapsulated access; update handler to use it instead of directly grabbing the mutex - Add nil-check for App.handler in SaveImage to prevent potential panic - Add empty imageURL guard in App.tsx before setting img.src - Check f.Close() error after io.Copy to catch flush failures (disk full) - Remove unused saveReady channel (dead code in prepareSave/ImageHandler)
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughOpenImage は選択ファイルパスを mutex 保護で保存し、ExifResult に imageURL（t=timestamp）、mimeType、filePath を返す。ImageHandler ミドルウェアが GET /api/image とトークン化された POST /api/save を実装し、フロントエンドは imageURL 経由で画像を読み込み、canvas バイナリを /api/save?token=... に直接 POST するようになった。 ChangesImage handling refactor: Base64 to HTTP streaming
Sequence DiagramssequenceDiagram
participant Frontend
participant App.OpenImage
participant ImageHandler
participant Disk
Frontend->>App.OpenImage: OpenImage()
App.OpenImage->>App.OpenImage: store currentImagePath (mutex)
App.OpenImage->>Frontend: return imageURL (with t=timestamp) + mimeType
Frontend->>ImageHandler: GET /api/image?t=...
ImageHandler->>Disk: http.ServeFile(currentImagePath)
ImageHandler-->>Frontend: image bytes (HTTP response)
sequenceDiagram
participant Frontend
participant App.SaveImage
participant ImageHandler.prepareSave
participant ImageHandler.handleSave
participant Disk
Frontend->>App.SaveImage: SaveImage(isPng)
App.SaveImage->>ImageHandler.prepareSave: prepareSave(savePath, expectedMime)
ImageHandler.prepareSave-->>App.SaveImage: token
Frontend->>Frontend: canvas.toBlob() -> blob
Frontend->>ImageHandler.handleSave: POST /api/save?token=... (blob + Content-Type)
ImageHandler.handleSave->>Disk: write to temp file (stream)
ImageHandler.handleSave->>ImageHandler.handleSave: detect content type (first 512 bytes)
ImageHandler.handleSave->>Disk: os.Rename temp -> final
ImageHandler.handleSave-->>Frontend: 200 OK or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces the memory-intensive Base64 IPC transfer of image bytes between Go and the frontend with HTTP streaming via the Wails AssetServer. Opening an image now returns an /api/image?t=... URL (served directly from disk via http.ServeFile), and saving uses a two-step flow: the native save dialog is opened via IPC and stores a target path on the server, then the canvas Blob is POSTed to /api/save and streamed to disk. A new ImageHandler is registered as AssetServer Middleware so it intercepts /api/* in both dev (Vite) and production modes.
Changes:
- Add
ImageHandler(handler.go) exposing/api/image(GET, stream file) and/api/save(POST, stream binary to disk with size cap and MIME verification). - Refactor
App.OpenImage/App.SaveImage: drop Base64 encoding/decoding; returnimageURL+mimeType;SaveImage(isPng bool)only handles dialog and prepares server-side save state. - Update frontend (
App.tsx, generated bindings, vite config note) to load images by URL and POST canvas Blobs to/api/save.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| main.go | Instantiates ImageHandler and registers it as AssetServer Middleware. |
| handler.go | New file implementing /api/image (stream from disk) and /api/save (stream POST to disk with MIME validation). |
| app.go | Adds RWMutex-protected currentImagePath, switches OpenImage/SaveImage to URL/handler-based flow, removes Base64 logic. |
| frontend/src/App.tsx | Loads image from URL, tracks sourceMimeType, sends Blob via fetch('/api/save'). |
| frontend/wailsjs/go/models.ts | Updates generated ExifResult fields (imageURL, mimeType). |
| frontend/wailsjs/go/main/App.d.ts | Updates SaveImage signature to (arg1: boolean). |
| frontend/vite.config.ts | Documents that no server.proxy is needed due to Middleware. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+207
to
+221
| // Step 2: Convert canvas to binary Blob (no Base64 intermediate) | ||
| const blob = await new Promise<Blob>((resolve, reject) => { | ||
| canvasRef.current!.toBlob( | ||
| (b) => b ? resolve(b) : reject(new Error("toBlob returned null")), | ||
| targetMime, | ||
| 1.0 // For JPEG: highest quality. PNG ignores this. | ||
| ); | ||
| }); | ||
|
|
||
| // Step 3: Send binary directly to Go HTTP handler | ||
| const response = await fetch('/api/save', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': targetMime }, | ||
| body: blob, | ||
| }); |
Comment on lines
+78
to
+95
| func (h *ImageHandler) handleSave(w http.ResponseWriter, r *http.Request) { | ||
| if r.Method != http.MethodPost { | ||
| http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) | ||
| return | ||
| } | ||
|
|
||
| h.saveMu.Lock() | ||
| savePath := h.savePath | ||
| expectedMime := h.saveMime | ||
| // Clear the save state after reading | ||
| h.savePath = "" | ||
| h.saveMime = "" | ||
| h.saveMu.Unlock() | ||
|
|
||
| if savePath == "" { | ||
| http.Error(w, "No save path prepared. Call SaveImage first.", http.StatusBadRequest) | ||
| return | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 68-74: Current single-slot state in ImageHandler (prepareSave,
h.savePath, h.saveMime, saveMu) causes race and incorrect writes when multiple
SaveImage flows run; change to a per-request one-time save session: have
prepareSave generate and return a unique saveSessionID and store metadata in a
map (e.g. map[string]{path,mime} protected by a mutex) instead of overwriting
h.savePath/h.saveMime, update SaveImage and the /api/save handler to require the
saveSessionID, look up the path/mime from the map, atomically consume (delete)
the entry while holding the mutex, and remove the single-slot fields (or stop
using them) so each save is 1:1 bound to its token and cannot be clobbered by
concurrent requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b226c0fe-21cd-4839-9371-203e88e94997
📒 Files selected for processing (7)
app.gofrontend/src/App.tsxfrontend/vite.config.tsfrontend/wailsjs/go/main/App.d.tsfrontend/wailsjs/go/models.tshandler.gomain.go
Replace the single-slot savePath/saveMime state with a token-based
session map. Each SaveImage call now:
1. Generates a crypto-random token via prepareSave
2. Stores {path, mime, expiresAt} in a map keyed by that token
3. Returns the token to the frontend via SaveResult.saveToken
The frontend sends the token as a query parameter in POST /api/save.
The handler atomically looks up and deletes the session, ensuring:
- No race: concurrent saves get independent tokens, never clobber
- No stale state: if toBlob or fetch fails, the unused token expires
after 60s and is garbage-collected on the next prepareSave call
- 1:1 binding: each token can only be consumed once
Comment on lines
+202
to
+206
| func generateToken() string { | ||
| b := make([]byte, 16) | ||
| if _, err := rand.Read(b); err != nil { | ||
| // Fallback: use timestamp (less secure but functional) | ||
| return hex.EncodeToString([]byte(time.Now().String())) |
Comment on lines
+125
to
+154
| // Atomically look up and consume the session. | ||
| h.saveMu.Lock() | ||
| session, ok := h.saveSessions[token] | ||
| if ok { | ||
| delete(h.saveSessions, token) | ||
| } | ||
| h.saveMu.Unlock() | ||
|
|
||
| if !ok { | ||
| http.Error(w, "Invalid or expired save token", http.StatusBadRequest) | ||
| return | ||
| } | ||
| if time.Now().After(session.expiresAt) { | ||
| http.Error(w, "Save token expired", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| savePath := session.path | ||
| expectedMime := session.mime | ||
|
|
||
| // Cap incoming body to prevent memory exhaustion (100MB) | ||
| const maxBodySize = 100 * 1024 * 1024 | ||
| r.Body = http.MaxBytesReader(w, r.Body, maxBodySize) | ||
|
|
||
| // Validate Content-Type matches what was expected from the save dialog | ||
| contentType := r.Header.Get("Content-Type") | ||
| if expectedMime != "" && contentType != expectedMime { | ||
| http.Error(w, "Content-Type mismatch", http.StatusBadRequest) | ||
| return | ||
| } |
Comment on lines
+94
to
+100
| // Garbage-collect any expired sessions. | ||
| now := time.Now() | ||
| for k, s := range h.saveSessions { | ||
| if now.After(s.expiresAt) { | ||
| delete(h.saveSessions, k) | ||
| } | ||
| } |
If a user chose an existing file to overwrite, calling os.Create(savePath) immediately truncated the file to 0 bytes. If the subsequent io.Copy or MIME validation failed, the file was deleted entirely by os.Remove, resulting in data loss of the original file. To fix this: 1. Write the upload stream to os.CreateTemp in the same directory. 2. Perform MIME validation on the temporary file. 3. If everything succeeds, atomically os.Rename the temp file to the final savePath. 4. If any step fails, defer os.Remove cleans up only the temp file, leaving the original target file untouched.
Exact string matching against the Content-Type header is brittle and will fail if the client appends parameters (e.g. charset=utf-8) or boundaries. Use mime.ParseMediaType to extract the base media type for comparison against the expected MIME type.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 147-149: 現在の固定上限 const maxBodySize = 100 * 1024 * 1024 と r.Body =
http.MaxBytesReader(w, r.Body, maxBodySize)
は入力サイズをそのままアップロード上限に使っており、JPEG再圧縮や出力PNGで /api/save
側が拒否される問題を起こしています。対応としてはこのハードコードを緩和して入力上限と保存上限を別管理にする（例: 入力は安全な読み込み上限
inputMaxBytes、保存はより大きめの outputMaxBytes または設定可能な値に分離）か、ここでの MaxBytesReader
を削除して受信は緩めにし、保存処理側（/api/save の保存バリデーション）で outputMaxBytes
を検証して拒否するように修正してください。特に参照するシンボルは maxBodySize と http.MaxBytesReader と /api/save
の保存バリデーション処理です。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f27d9507-9f96-41b6-a337-ce02058a1bcb
📒 Files selected for processing (4)
app.gofrontend/src/App.tsxfrontend/wailsjs/go/models.tshandler.go
The 100MB limit via http.MaxBytesReader was originally introduced when the app used Base64 IPC to transfer images, where unbounded sizes could cause memory exhaustion (OOM) and crash the app. Since the app now streams the binary body directly to disk via io.Copy in the HTTP handler, the memory footprint is minimal regardless of file size. Removing this arbitrary limit allows users to export very large high-resolution lossless PNGs without encountering false rejections.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 211-218: The fallback in generateToken is insecure; change
generateToken to return (string, error) and remove the time-based fallback so
that a failed crypto/rand.Read returns an error; update prepareSave to accept
and propagate that (string, error) result (make prepareSave return (string,
error)), and update App.SaveImage (and any callers that used
generateToken/prepareSave) to handle the error and surface it to the caller
instead of creating a weak token; ensure all call sites are updated to check and
return errors so token-generation failures abort session creation and do not
produce predictable tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 56f4e3db-2bce-430b-ad5a-79de4f6b50c0
📒 Files selected for processing (1)
handler.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.