fix(uploads): bound the HEIF fallback decode by declared pixels, not just bytes - #6456
Conversation
…just bytes The WebAssembly fallback allocates width * height * 4 up front, taking the size straight from the container and building the buffer before the codec is asked for anything — so a file that never decodes still costs the memory. Only a 20MB byte ceiling stood in front of it, and bytes do not bound a declared raster: a small container can name dimensions up to libheif's own default of ~1.07e9 pixels, about 4.3GB as RGBA. Read the declared dimensions with `heic-decode`'s `all()`, which parses the container and reports each image's size while leaving the decode for `decode()`, and refuse above 100MP. That caps the allocation near 400MB and clears every phone camera — a 48MP iPhone still is 8064x6048. `heic-decode` was already present as a transitive dependency of `heic-convert`; this promotes it to a direct one at the same version, since the code now imports it. Local types cover only the surface used, and a test pins that `all` really is a named ESM export — a CJS `module.exports = one` need not surface it, and if it stopped, the check would throw, get swallowed by the catch, and quietly stop guarding with mocked tests still green.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The guard uses Tests add a mocked pixel-ceiling suite plus pins that Reviewed by Cursor Bugbot for commit df71c97. Configure here. |
Greptile SummaryThe PR adds a declared-pixel ceiling before HEIF fallback decoding and promotes
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported dimensions issue was invalidated by the verified
|
| Filename | Overview |
|---|---|
| apps/sim/lib/uploads/server/heic.ts | Adds the pre-decode pixel guard and now reliably disposes the temporary decoder handles through a finally block. |
| apps/sim/lib/uploads/server/heic-pixel-guard.test.ts | Covers oversized, multi-image, boundary, ordinary-photo, no-decode, and decoder-handle cleanup behavior. |
| apps/sim/lib/uploads/server/heic.test.ts | Pins the real library mapping that exposes dimensions before decoding and verifies the named ESM export. |
| apps/sim/types/heic-decode.d.ts | Declares the directly used lazy-handle API and makes its required disposal operation explicit. |
| apps/sim/package.json | Promotes the newly imported heic-decode package to an exact direct dependency. |
| bun.lock | Records the direct dependency without introducing a duplicate heic-decode resolution. |
Sequence Diagram
sequenceDiagram
participant Upload
participant Guard as HEIF pixel guard
participant Decoder as heic-decode all()
participant Convert as heic-convert
Upload->>Guard: HEIF buffer
Guard->>Decoder: Parse declared image dimensions
Decoder-->>Guard: Handles with width, height, dispose
alt Any image exceeds 100MP
Guard->>Decoder: dispose()
Guard-->>Upload: null
else Dimensions are within ceiling
Guard->>Decoder: dispose()
Guard->>Convert: Convert primary image to JPEG
Convert-->>Upload: JPEG buffer
end
Reviews (3): Last reviewed commit: "test(uploads): pin that all() reports di..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
all() hands back live libheif handles and leaves freeing them to the caller — the default export does it in a finally, this path did not. Every HEIF preview would have leaked the decoder context on the WebAssembly heap. Copy the two numbers out, dispose in a finally so a throw mid-read still frees, and declare dispose as required on the returned array so a caller cannot omit it.
The pixel guard is only worth anything if the declared size is available up front. Were dimensions to move behind decode() — as they already are on the default export — width * height would silently become NaN and the check would stop rejecting anything, with every mocked test still green. Drives the real mapping with a stub libheif, so the contract is exercised without needing a HEVC encoder to build a valid container.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit df71c97. Configure here.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit df71c97. Configure here.
1 participant
Summary
heic-decode'sall(), which reports each image's size while leaving the decode fordecode()heic-decodefrom a transitive dependency ofheic-convertto a direct one at the same version, since the code now imports itType of Change
Testing
Added unit tests for the ceiling (including the multi-image case and that the handle is never asked to decode), plus one pinning that
allis a named ESM export — a mocked test alone could not catch that regressing. Verified the guard tests fail without the guard.lib/uploads(408),lib/copilotandapp/api/files(1658) suites pass; type-check, lint and api-validation clean.Checklist