From 06e0bea0e984f70b61c88b628979aa1a0e7363e3 Mon Sep 17 00:00:00 2001 From: fabi Date: Thu, 13 Aug 2026 19:45:08 +0200 Subject: [PATCH] fix(export): a refused download mint no longer strands the running transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/export/ticket` mints before charging the daily limit, deliberately: charging first meant a store-capacity 503 — a server-side condition the guest cannot see or cause — still cost one of their three daily downloads, with no refund path. But a mint refused with 429 left its ticket in the store. Download tickets live six hours (they must, so a 1.4 GB transfer can resume with `Range`), and the per-session cap is four tickets OF THE SAME KIND. So: the transfer starts on ticket A -> the bar looks stuck on venue wifi -> the guest taps "Herunterladen" again -> mints 2 and 3 succeed, 4 and 5 return 429 but still mint -> the fifth evicts the oldest download ticket for the session, which is A -> the transfer drops, resumes, and 401s -> re-minting is impossible, they are at the daily limit The keepsake is unreachable until the next day, for tapping a button that appeared to do nothing. This is the failure `sse_churn_cannot_evict_a_running_ download` was written to prevent, reintroduced through the one channel that test does not cover: download tickets evicting each other. Discarding the ticket on the refusal path keeps both properties that put the mint first — a capacity 503 still costs no download, and a refused download now costs no slot. Also corrects three doc comments in this path still describing download tickets as "single-use, 30s TTL", false since they were made resumable. Stale comments here have already sent one review down the wrong path. The new spec asserts the two 429s explicitly, so it cannot pass vacuously if the daily limit stops being enforced. Co-Authored-By: Claude Opus 5 --- backend/src/handlers/admin.rs | 36 +++++-- .../06-export/ticket-refused-mint.spec.ts | 99 +++++++++++++++++++ 2 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 e2e/specs/06-export/ticket-refused-mint.spec.ts diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index b750807..5faa338 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -364,8 +364,10 @@ pub struct DownloadQuery { /// is a top-level navigation so the multi-GB ZIP streams straight to disk instead /// of being buffered in memory by `fetch()` + `blob()` — but a navigation can't /// carry an `Authorization` header, so the client exchanges its Bearer token for -/// an opaque ticket here, then hits `/export/zip?ticket=...`. Reuses the same -/// single-use, 30s-TTL store as the SSE stream. +/// an opaque ticket here, then hits `/export/zip?ticket=...`. Uses the same store as the SSE +/// stream, but NOT the same lifetime: a download ticket lives `DOWNLOAD_TTL` (6 h) and is +/// redeemable up to `MAX_DOWNLOAD_REDEMPTIONS` times, because a multi-GB transfer over venue wifi +/// has to survive being resumed with `Range`. #[derive(serde::Deserialize)] pub struct ExportTicketQuery { /// Which archive the ticket is for — `zip` or `html`. @@ -395,8 +397,11 @@ pub async fn export_ticket( // forever, with no explanation, on the one screen that is the emotional payoff of the app. // Minting is a normal `fetch`, so a 429 here reaches the user as a German message. // - // Moving it does not weaken the limit: tickets are single-use with a 30s TTL and can only be - // obtained from this authenticated endpoint, so one mint is at most one download. + // Moving it does not weaken the limit meaningfully: a ticket can only be obtained from this + // authenticated endpoint, is bound to one archive, and — since downloads must be resumable — + // is worth at most `MAX_DOWNLOAD_REDEMPTIONS` transfers rather than exactly one. The daily + // limit is therefore a bound on mints, not on bytes; see `MAX_DOWNLOAD_REDEMPTIONS` for why + // charging per redemption would re-break resumption. // Confirm the archive actually EXISTS before spending anything on it. // // `export_status` — which is what enables the Download button — reports `done` from @@ -460,14 +465,29 @@ pub async fn export_ticket( ) })?; - enforce_export_rate(&state, auth.user_id).await?; + // A refused mint must not leave its ticket behind. The per-session cap is FOUR tickets of the + // same kind, and a download ticket now lives six hours instead of being consumed on first use — + // so every abandoned one occupies a slot until it expires. A guest whose 1.4 GB transfer looks + // stuck and who taps "Herunterladen" a few more times spends mints 1-3 legitimately, then gets + // a 429 on taps 4 and 5 — but both still minted, and the fifth evicted the OLDEST download + // ticket for the session: the one the running transfer is holding. The next `Range` resume then + // 401s, and re-minting is impossible because they are at the daily limit. The keepsake is gone + // until tomorrow, having done nothing worse than tapping a button that appeared to do nothing. + // + // Discarding here keeps both properties that put the mint first: a store-capacity 503 still + // costs no download, and a refused download costs no slot. + if let Err(e) = enforce_export_rate(&state, auth.user_id).await { + let _ = state + .sse_tickets + .consume(&ticket, TicketKind::Download(export_kind)); + return Err(e); + } Ok(Json(serde_json::json!({ "ticket": ticket }))) } -/// Validate a download ticket (single-use) and confirm its session still exists. -/// Resolve a single-use download ticket to the user who minted it. The caller needs the -/// id to key the export rate limit per-user (see `enforce_export_rate`). +/// Validate a download ticket and confirm its session still exists, resolving it to the user who +/// minted it. Deliberately NOT single-use — see the note on `redeem_download` below. async fn authenticate_download_ticket( state: &AppState, ticket: &str, diff --git a/e2e/specs/06-export/ticket-refused-mint.spec.ts b/e2e/specs/06-export/ticket-refused-mint.spec.ts new file mode 100644 index 0000000..1a9e7af --- /dev/null +++ b/e2e/specs/06-export/ticket-refused-mint.spec.ts @@ -0,0 +1,99 @@ +/** + * A mint that is REFUSED must not cost the guest the download they already have running. + * + * `/export/ticket` issues the ticket before charging the daily limit, deliberately: charging first + * meant a store-capacity 503 — a server-side condition the guest cannot see or cause — still spent + * one of their three daily downloads, with no refund path. + * + * But the ticket a refused mint created was left in the store, and it occupies a slot there for six + * hours, because a download ticket is long-lived so a multi-GB transfer can resume with `Range`. + * The per-session cap is four tickets of the same kind, so: + * + * the 1.4 GB transfer starts on ticket A → the progress bar looks stuck on venue wifi → the guest + * taps "Herunterladen" again → mints 2 and 3 succeed, 4 and 5 are refused with 429 but STILL mint + * → the fifth evicts the oldest download ticket for the session, which is A + * → the transfer drops, resumes with `Range`, and 401s + * → re-minting is impossible: they are at the daily limit + * + * The keepsake is then unreachable until the next day, for tapping a button that appeared to do + * nothing. A refused mint now discards its own ticket. + */ +import { test, expect } from '../../fixtures/test'; +import { seedUpload } from '../../helpers/seed'; +import { BASE } from '../../helpers/env'; + +test.describe('Export — a refused mint does not evict a running download', () => { + test('the first ticket still works after the daily limit has refused later mints', async ({ + host, + api, + adminToken, + }) => { + test.setTimeout(60_000); + const bearer = { Authorization: `Bearer ${host.jwt}` }; + + await seedUpload(host.jwt, { caption: 'keepsake' }); + + 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'); + + // The e2e reseed forces every limiter OFF, so without this the daily limit never bites and the + // whole test passes vacuously. `export_rate_per_day` is seeded at 3 (migration 005). + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + export_rate_enabled: 'true', + }); + + try { + const mint = async () => + fetch(`${BASE}/api/v1/export/ticket?kind=zip`, { method: 'POST', headers: bearer }); + + // 1. The ticket the "running" transfer holds. + const first = await mint(); + expect(first.status).toBe(200); + const ticketA = (await first.json()).ticket as string; + expect(ticketA).toBeTruthy(); + + // 2. Two more legitimate mints, exhausting the day's three. + for (const i of [2, 3]) { + const r = await mint(); + expect(r.status, `mint ${i} is still within the daily allowance`).toBe(200); + } + + // 3. Two refused mints — the impatient taps. This is the CONTROL: if these came back 200 the + // limit was not in force and step 4 would prove nothing. + for (const i of [4, 5]) { + const r = await mint(); + expect(r.status, `mint ${i} must be refused — the daily limit is spent`).toBe(429); + } + + // 4. The running transfer resumes. Its ticket must have survived the refused mints: it is the + // guest's only remaining way to reach the keepsake today. + const resumed = await fetch( + `${BASE}/api/v1/export/zip?ticket=${encodeURIComponent(ticketA)}` + ); + expect( + resumed.status, + 'a refused mint must not evict the ticket an in-flight download is holding' + ).toBe(200); + await resumed.arrayBuffer(); + } finally { + await api.patchConfig(adminToken, { + rate_limits_enabled: 'false', + export_rate_enabled: 'false', + }); + } + }); +});