From 182e712a0e396bc3d114362767521a780d61fe66 Mon Sep 17 00:00:00 2001 From: fabi Date: Wed, 12 Aug 2026 19:10:45 +0200 Subject: [PATCH] fix(export): a resumed download can no longer splice two archives together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `serve_file` emitted no validator — no ETag, no Last-Modified — and ignored If-Range entirely, while `resolve_export_file` re-reads `export_current` on EVERY request and a download ticket survives 20 redemptions over 6 hours. So: a guest's 500 MB Gallery.zip drops at 500 MB. The host takes a photo down — epoch bumps, the rebuild lands, the old generation is pruned. The client resumes with `Range: bytes=500000000-`. The ticket and session are both still valid, the handler resolves the NEW archive, seeks 500 MB into a different file of a different length, and streams. The client concatenates the halves into a structurally corrupt ZIP. Nothing logs an error anywhere; a 404 would have been the correct answer. Now every response carries an ETag over the generation-stamped filename plus the length, and a partial is served only against a matching If-Range. A Range with no validator — curl -C -, wget -c, the Android download manager, all of which resume blindly — gets the whole file instead. Restarting a download is a cost; a corrupt keepsake is not recoverable. Browsers send If-Range, so this is also the first release where their resume works at all: with no validator to send, they simply refused to try. --- backend/src/handlers/admin.rs | 41 ++++++++- .../download-resume-validator.spec.ts | 85 +++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 e2e/specs/06-export/download-resume-validator.spec.ts diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index ece9a59..b750807 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -511,6 +511,9 @@ pub async fn download_zip( headers .get(axum::http::header::RANGE) .and_then(|v| v.to_str().ok()), + headers + .get(axum::http::header::IF_RANGE) + .and_then(|v| v.to_str().ok()), ) .await } @@ -578,6 +581,9 @@ pub async fn download_html( headers .get(axum::http::header::RANGE) .and_then(|v| v.to_str().ok()), + headers + .get(axum::http::header::IF_RANGE) + .and_then(|v| v.to_str().ok()), ) .await } @@ -596,6 +602,7 @@ async fn serve_file( filename: &str, content_type: &str, range_header: Option<&str>, + if_range_header: Option<&str>, ) -> Result { use crate::handlers::upload::{RangeSpec, parse_range}; use axum::body::Body; @@ -613,6 +620,26 @@ async fn serve_file( .len(); let disposition = format!("attachment; filename=\"{filename}\""); + + // A validator that CHANGES when the archive does, so a resume cannot splice two generations. + // + // The on-disk name is `{prefix}.{event_id}.{epoch}.zip`, so it already identifies the exact + // generation; length distinguishes a rebuild at the same epoch. Together they are a strong + // validator. + // + // Why this matters: `resolve_export_file` re-reads `export_current` on EVERY request, and a + // download ticket outlives several redemptions. So a guest whose 500 MB download drops at + // 500 MB, while the host takes a photo down (epoch bumps, rebuild lands, the old generation is + // pruned), used to resume with `Range: bytes=500000000-` against a DIFFERENT FILE of a + // different length — and the server would happily seek 500 MB into it and stream. The client + // concatenated the two halves into a structurally corrupt ZIP, with nothing logged anywhere. + let etag = format!( + "\"{}-{len}\"", + path.file_name() + .and_then(|n| n.to_str()) + .unwrap_or(filename) + ); + let base = |status: StatusCode| { Response::builder() .status(status) @@ -621,9 +648,21 @@ async fn serve_file( // Advertised on EVERY response, including the 200. A client only knows it may resume // if the first (unranged) response says so. .header(header::ACCEPT_RANGES, "bytes") + .header(header::ETAG, etag.clone()) }; - match parse_range(range_header, len) { + // Serve a partial ONLY when the client proves it is resuming the same bytes. + // + // `If-Range` matching our ETag is that proof. A client that sends `Range` with no `If-Range` + // at all (curl -C -, wget -c, most download managers) cannot be given a partial safely — it + // has no way to notice the archive changed underneath it — so it gets a 200 and starts over. + // Restarting a download is a cost; a silently corrupt keepsake is not recoverable. Browsers + // send `If-Range`, so the ordinary resume path is unaffected, and this is the first release + // where their resume works at all: without a validator they simply refused to try. + let resume_is_safe = if_range_header.is_some_and(|v| v.trim() == etag); + let effective_range = if resume_is_safe { range_header } else { None }; + + match parse_range(effective_range, len) { RangeSpec::Full => base(StatusCode::OK) .header(header::CONTENT_LENGTH, len) .body(Body::from_stream(ReaderStream::new(file))) diff --git a/e2e/specs/06-export/download-resume-validator.spec.ts b/e2e/specs/06-export/download-resume-validator.spec.ts new file mode 100644 index 0000000..e5bb256 --- /dev/null +++ b/e2e/specs/06-export/download-resume-validator.spec.ts @@ -0,0 +1,85 @@ +/** + * A resumed keepsake download must never splice two different archives together. + * + * The download endpoint re-resolves `export_current` on EVERY request, and a download ticket + * outlives several redemptions. So the dangerous sequence was: + * + * guest's 500 MB download drops at 500 MB + * → host takes a photo down (epoch bumps, rebuild lands, old generation pruned) + * → client resumes with `Range: bytes=500000000-` + * → server seeks 500 MB into a DIFFERENT file of a different length and streams it + * → the client concatenates the two halves into a structurally corrupt ZIP + * + * Nothing anywhere logged an error. The archive is the one artifact the whole event exists to + * produce, so a partial is now served only against a matching `If-Range` validator. + */ +import { test, expect } from '../../fixtures/test'; +import { seedUpload } from '../../helpers/seed'; +import { BASE } from '../../helpers/env'; + +test.describe('Export — a resume cannot splice two archives', () => { + test('partial content requires a matching If-Range; a blind Range restarts instead', async ({ + host, + }) => { + test.setTimeout(60_000); + const bearer = { Authorization: `Bearer ${host.jwt}` }; + + await seedUpload(host.jwt, { caption: 'resumable' }); + + const rel = await fetch(`${BASE}/api/v1/host/gallery/release`, { + method: 'POST', + headers: bearer, + }); + expect(rel.status).toBe(204); + + await expect + .poll( + async () => { + const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer }); + return (await res.json()).zip?.status; + }, + { timeout: 45_000, intervals: [500] } + ) + .toBe('done'); + + const mint = async () => { + const r = await fetch(`${BASE}/api/v1/export/ticket?kind=zip`, { + method: 'POST', + headers: bearer, + }); + return (await r.json()).ticket as string; + }; + const url = async () => `${BASE}/api/v1/export/zip?ticket=${encodeURIComponent(await mint())}`; + + // 1. The full download advertises a validator. Without one a browser will not even attempt a + // resume, so this header is what makes the feature work at all — and it is what the + // partial below is checked against. + const full = await fetch(await url()); + expect(full.status).toBe(200); + const etag = full.headers.get('etag'); + expect(etag, 'the archive must carry an ETag or no client can resume safely').toBeTruthy(); + expect(full.headers.get('accept-ranges')).toBe('bytes'); + + // 2. A resume that PROVES continuity gets its partial. + const resumed = await fetch(await url(), { + headers: { Range: 'bytes=0-99', 'If-Range': etag! }, + }); + expect(resumed.status, 'a matching If-Range must still get 206').toBe(206); + expect(resumed.headers.get('content-range')).toMatch(/^bytes 0-99\/\d+$/); + + // 3. A resume that cannot prove it — `curl -C -`, `wget -c`, the Android download manager — + // gets the whole file instead of a silently spliced one. Restarting a download is a cost; + // a corrupt keepsake is not recoverable. + const blind = await fetch(await url(), { headers: { Range: 'bytes=0-99' } }); + expect(blind.status, 'a Range with no If-Range must NOT be served as a partial').toBe(200); + expect(blind.headers.get('content-range')).toBeNull(); + + // 4. And a stale validator — the exact case that used to splice — is refused a partial too. + const stale = await fetch(await url(), { + headers: { Range: 'bytes=0-99', 'If-Range': '"Gallery.some-other-event.99.zip-123"' }, + }); + expect(stale.status, 'an If-Range from a different generation must not get a partial').toBe( + 200 + ); + }); +});