Two halves of the same complaint: an oversized photo was accepted with a 201 and then silently soft-deleted minutes later, after the worker had burned six seconds of backoff re-reaching a conclusion it could not change. Admission. The compression budget now runs at upload time, against the header only, so a guest is told immediately and told why: "Bild hat zu viele Bildpunkte (ca. 99 Megapixel) und kann nicht verarbeitet werden. Bitte verkleinere es und lade es erneut hoch." instead of watching the photo vanish behind a vague "could not be processed" — which arrived only if they happened to still be on the feed with that card loaded. Nothing is stored, so there is no row to soft-delete and no orphan for the sweep to reclaim. Admission and the worker share ONE function (`decoder_within_budget`), so they cannot drift apart and start disagreeing about what is acceptable — a photo accepted at the door and rejected by the worker would be worse than either behaviour alone. The worker keeps its own check: the backfill decodes files that predate this check, and defence in depth is the whole reason the budget exists. Retries. The loop retried every failure, including ones that are a property of the input. An image over the budget, a corrupt file, an unsupported format: each fails identically on all three attempts, so the only effect was 2s + 4s of sleep and three near-identical warnings before the same outcome. `is_permanent_image_error` classifies the `ImageError` variants that cannot change between attempts — Limits, Unsupported, Decoding — and the loop gives up on those at once. `IoError` is deliberately excluded: an ENOSPC while writing a derivative is exactly the transient case the retry exists for, and misclassifying it would turn a blip back into the data loss round 1 fixed. Measured: retry log lines went from 3 per oversized upload to 0. Tests: unit tests for both sides of the classifier (a Limits error is permanent, a missing file is not) and for admission agreeing with the decoder on accept AND reject. The e2e spec is rewritten for the new contract — 400 with an actionable message, nothing stored, backend alive after a burst of four — plus a mirror asserting an ordinary photo still uploads and processes, since a budget that rejected everything would satisfy the other two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
101 lines
4.7 KiB
TypeScript
101 lines
4.7 KiB
TypeScript
/**
|
|
* Regression guard — an image that would blow the decode budget must be refused at the
|
|
* door, with a reason the guest can act on, and must never allocate.
|
|
*
|
|
* Two defects met here.
|
|
*
|
|
* 1. The budget was inert. `max_alloc = 256 MiB` was set, but reading the EXIF orientation
|
|
* tag requires `ImageReader::into_decoder()`, which skips the
|
|
* `limits.reserve(decoder.total_bytes())` that `decode()` performs — and nothing else
|
|
* enforces it (the JPEG decoder's `set_limits` only checks support and dimensions). The
|
|
* only real bound was the 12000px per-axis cap, leaving two concurrent decodes at
|
|
* 824 MiB against a 1 GiB container. This suite could not have caught it either, because
|
|
* the e2e app container had NO memory limit while production is capped at 1 GiB; that cap
|
|
* is now mirrored in docker-compose.test.yml so these assertions mean something.
|
|
*
|
|
* 2. Even with the budget restored, the upload was ACCEPTED with a 201 and then silently
|
|
* soft-deleted minutes later when the worker gave up — the photo simply vanished, with at
|
|
* best a vague "could not be processed". Admission now runs the same budget check against
|
|
* the header, so the guest is told immediately and told why.
|
|
*
|
|
* Fixture: 11000x9000 = 99 MP, 568 KiB on disk. Deliberately UNDER the per-axis cap, so the
|
|
* axis check cannot be what rejects it — 283 MiB decoded against a 256 MiB budget. A fixture
|
|
* at 13000px would pass this test against a build with no budget at all.
|
|
*/
|
|
import { test, expect } from '../../fixtures/test';
|
|
import { uploadRaw } from '../../helpers/upload-client';
|
|
import { BASE } from '../../helpers/env';
|
|
import { readFileSync } from 'node:fs';
|
|
import { join } from 'node:path';
|
|
|
|
const HUGE = join(process.cwd(), 'fixtures', 'media', 'huge-99mp.jpg');
|
|
const SAMPLE = join(process.cwd(), 'fixtures', 'media', 'sample.jpg');
|
|
|
|
test.describe('Upload — an oversized image is refused, not allocated', () => {
|
|
test('a 99 MP upload is rejected at admission with a readable reason', async ({ guest, db }) => {
|
|
test.setTimeout(60_000);
|
|
const g = await guest('BombThrower');
|
|
const before = await db.countUploadsForUser(g.userId);
|
|
|
|
const res = await uploadRaw(g.jwt, readFileSync(HUGE), {
|
|
filename: 'huge.jpg',
|
|
contentType: 'image/jpeg',
|
|
caption: 'zu gross',
|
|
});
|
|
|
|
// 4xx, not 201-then-vanish. The queue classifies this as terminal, so the guest gets the
|
|
// message rather than watching the photo disappear.
|
|
expect(res.status, 'an undecodable image must be refused at the door').toBe(400);
|
|
const body = (await res.json()) as { message?: string };
|
|
expect(body.message ?? '', 'the reason must be actionable, not generic').toMatch(/bildpunkte/i);
|
|
expect(body.message ?? '', 'and should name the size so it is obvious why').toMatch(/99/);
|
|
|
|
// Nothing was stored — no row to soft-delete later, no orphaned file to sweep.
|
|
expect(await db.countUploadsForUser(g.userId)).toBe(before);
|
|
|
|
// The backend never allocated: it is still alive and still doing useful work.
|
|
expect((await fetch(`${BASE}/health`)).status).toBe(200);
|
|
const ok = await uploadRaw(g.jwt, readFileSync(SAMPLE), {
|
|
filename: 'after.jpg',
|
|
contentType: 'image/jpeg',
|
|
});
|
|
expect(ok.status).toBe(201);
|
|
const after = (await ok.json()) as { id: string };
|
|
await expect.poll(() => db.compressionStatus(after.id), { timeout: 30_000 }).toBe('done');
|
|
});
|
|
|
|
test('a burst of oversized uploads leaves the container alive', async ({ guest }) => {
|
|
// The concurrent case is the one that OOM'd: `compression_concurrency` is 2, so decodes
|
|
// overlapped. Four at once is comfortably past that, and must still cost only header
|
|
// reads.
|
|
test.setTimeout(60_000);
|
|
const g = await guest('BombThrower2');
|
|
const bytes = readFileSync(HUGE);
|
|
|
|
const results = await Promise.all(
|
|
Array.from({ length: 4 }, (_, i) =>
|
|
uploadRaw(g.jwt, bytes, { filename: `huge-${i}.jpg`, contentType: 'image/jpeg' })
|
|
)
|
|
);
|
|
expect(results.map((r) => r.status)).toEqual([400, 400, 400, 400]);
|
|
|
|
expect(
|
|
(await fetch(`${BASE}/health`)).status,
|
|
'concurrent oversized uploads must not kill the backend'
|
|
).toBe(200);
|
|
});
|
|
|
|
test('an ordinary photo is unaffected by the admission check', async ({ guest, db }) => {
|
|
// The mirror that keeps the check honest: a budget that rejected everything would pass
|
|
// both tests above.
|
|
const g = await guest('NormalShooter');
|
|
const res = await uploadRaw(g.jwt, readFileSync(SAMPLE), {
|
|
filename: 'normal.jpg',
|
|
contentType: 'image/jpeg',
|
|
});
|
|
expect(res.status).toBe(201);
|
|
const { id } = (await res.json()) as { id: string };
|
|
await expect.poll(() => db.compressionStatus(id), { timeout: 30_000 }).toBe('done');
|
|
});
|
|
});
|