From c8795ddfacd6060142ccc2f1ac4e79446541c23c Mon Sep 17 00:00:00 2001 From: fabi Date: Sat, 22 Aug 2026 16:05:22 +0000 Subject: [PATCH] fix(upload): verify the copied bytes, so a purge mid-read cannot store a short file v0.18.5 copies a picked file's bytes into a Blob the origin owns, because WebKit stores a `File` as a reference to an OS file that iOS later deletes. For a photo that is one `arrayBuffer()` and effectively atomic. For a video it is a 4 MB-at- a-time loop over as much as 500 MB, which takes many seconds -- and the purge that motivated the whole function can land in the MIDDLE of it. Once the OS file is gone the remaining slices read as nothing, `new Blob` builds a short blob out of what it got, and nothing notices. That is strictly worse than the bug it replaces: an empty body is refused with a 400, but a short body uploads, passes the server's checks, and leaves the guest with a truncated video that appears to have worked. Silent corruption beats loud failure only in the sense that nobody finds out until the album is the only copy left. So the copy is now checked against `file.size` and a short read is refused. It is raised at PICK time, while the guest still has the file in front of them and can select it again -- not at send time, minutes later, when the moment has passed. `addToQueue` returns 'unreadable' rather than throwing, so the composer's per-file loop survives it: one bad photo out of five must not abandon the other four. The composer names the file in the toast instead of counting it the way it counts 'full', because the guest has to locate and re-pick that specific one. Scope: only files above the 4 MB chunk threshold can hit the partial case, so this does not touch the photo path that v0.18.5 fixed. It closes the hole that fix opened for videos. --- frontend/src/lib/upload-queue.ts | 39 ++++++++++++++++++++----- frontend/src/routes/upload/+page.svelte | 10 +++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index 866e088..5e7706b 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -889,7 +889,7 @@ export async function releaseResolvedParks(state: { /** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT * actually queued (deduped, or the queue is full of un-evictable in-flight items). */ -export type EnqueueResult = 'queued' | 'duplicate' | 'full'; +export type EnqueueResult = 'queued' | 'duplicate' | 'full' | 'unreadable'; /** Chunk size for `materialise`. Bounds peak JS heap, not total copy size. */ const MATERIALISE_CHUNK_BYTES = 4 * 1024 * 1024; @@ -907,15 +907,30 @@ const MATERIALISE_CHUNK_BYTES = 4 * 1024 * 1024; * can spill them to disk, which is exactly where a half-gigabyte video should live. */ async function materialise(file: File): Promise { + let out: Blob; if (file.size <= MATERIALISE_CHUNK_BYTES) { - return new Blob([await file.arrayBuffer()], { type: file.type }); + out = new Blob([await file.arrayBuffer()], { type: file.type }); + } else { + const parts: Blob[] = []; + for (let offset = 0; offset < file.size; offset += MATERIALISE_CHUNK_BYTES) { + const slice = file.slice(offset, offset + MATERIALISE_CHUNK_BYTES); + parts.push(new Blob([await slice.arrayBuffer()])); + } + out = new Blob(parts, { type: file.type }); } - const parts: Blob[] = []; - for (let offset = 0; offset < file.size; offset += MATERIALISE_CHUNK_BYTES) { - const slice = file.slice(offset, offset + MATERIALISE_CHUNK_BYTES); - parts.push(new Blob([await slice.arrayBuffer()])); + // Verify the copy. The purge this whole function exists to defeat can also land PARTWAY + // THROUGH the loop above: a 500 MB video is many seconds of reading, and once the OS file + // is gone the remaining slices read as nothing. `new Blob` is happy to build a short blob + // out of them, and short is far worse than absent — it uploads, the server stores it, and + // the guest gets a truncated video that looks like it worked. A read that returns fewer + // bytes than the file claims is never legitimate, so refuse it here, while the guest is + // still holding the phone and can pick the file again. + if (out.size !== file.size) { + throw new UnreadableBlobError( + 'Diese Datei konnte nicht vollständig gelesen werden — bitte wähle sie noch einmal aus.' + ); } - return new Blob(parts, { type: file.type }); + return out; } export async function addToQueue( @@ -980,7 +995,15 @@ export async function addToQueue( // Reading the file here makes IndexedDB own real bytes that no OS purge can reach. It // costs one full read at pick time, which is also the moment the file is guaranteed still // readable — the picker has only just handed it over. - const blob = await materialise(file); + // A file the browser cannot fully read is not a queueable item. Returning a result rather + // than throwing keeps the composer's per-file loop intact: one bad photo out of five must + // not abandon the other four, which is what an exception here would do. + let blob: Blob; + try { + blob = await materialise(file); + } catch { + return 'unreadable'; + } const entry: QueueEntry = { id, userId, diff --git a/frontend/src/routes/upload/+page.svelte b/frontend/src/routes/upload/+page.svelte index 04d50aa..4863e6a 100644 --- a/frontend/src/routes/upload/+page.svelte +++ b/frontend/src/routes/upload/+page.svelte @@ -163,6 +163,16 @@ } const result = await addToQueue(sf.file, caption, hashtagsString); if (result === 'full') full++; + // The browser could not read this file's bytes (iOS purges the OS file behind a + // picked photo). Named per file rather than counted like `full`: the guest has to + // find and re-pick this specific one, so a bare number would not be actionable. + if (result === 'unreadable') { + toast( + `„${sf.file.name}“ konnte nicht gelesen werden. Bitte wähle das Foto noch einmal aus.`, + 'error', + 8000 + ); + } } // Don't let a full queue silently swallow photos the user thinks were queued. if (full > 0) {