fix(export): a refused download mint no longer strands the running transfer
`/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 <noreply@anthropic.com>
This commit is contained in:
@@ -364,8 +364,10 @@ pub struct DownloadQuery {
|
|||||||
/// is a top-level navigation so the multi-GB ZIP streams straight to disk instead
|
/// 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
|
/// 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
|
/// carry an `Authorization` header, so the client exchanges its Bearer token for
|
||||||
/// an opaque ticket here, then hits `/export/zip?ticket=...`. Reuses the same
|
/// an opaque ticket here, then hits `/export/zip?ticket=...`. Uses the same store as the SSE
|
||||||
/// single-use, 30s-TTL store as the SSE stream.
|
/// 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)]
|
#[derive(serde::Deserialize)]
|
||||||
pub struct ExportTicketQuery {
|
pub struct ExportTicketQuery {
|
||||||
/// Which archive the ticket is for — `zip` or `html`.
|
/// 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.
|
// 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.
|
// 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
|
// Moving it does not weaken the limit meaningfully: a ticket can only be obtained from this
|
||||||
// obtained from this authenticated endpoint, so one mint is at most one download.
|
// 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.
|
// Confirm the archive actually EXISTS before spending anything on it.
|
||||||
//
|
//
|
||||||
// `export_status` — which is what enables the Download button — reports `done` from
|
// `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 })))
|
Ok(Json(serde_json::json!({ "ticket": ticket })))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Validate a download ticket (single-use) and confirm its session still exists.
|
/// Validate a download ticket and confirm its session still exists, resolving it to the user who
|
||||||
/// Resolve a single-use download ticket to the user who minted it. The caller needs the
|
/// minted it. Deliberately NOT single-use — see the note on `redeem_download` below.
|
||||||
/// id to key the export rate limit per-user (see `enforce_export_rate`).
|
|
||||||
async fn authenticate_download_ticket(
|
async fn authenticate_download_ticket(
|
||||||
state: &AppState,
|
state: &AppState,
|
||||||
ticket: &str,
|
ticket: &str,
|
||||||
|
|||||||
99
e2e/specs/06-export/ticket-refused-mint.spec.ts
Normal file
99
e2e/specs/06-export/ticket-refused-mint.spec.ts
Normal file
@@ -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',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user