Answer an oversized photo with the reason, not a 500
A file past the size limit makes the multipart stream itself throw, which escaped the per-file catch and reached the error handler: the phone that just sent a 35 MB photo got 'Something went wrong'. Catch the limit errors around the part loop, say which limit was hit, and report it once rather than twice. Also asserts that derivatives carry no EXIF while the Original keeps the uploaded bytes.
This commit is contained in:
1 parent
8af52a0f7d
commit
a635c5a214
7 files changed
+234
-30
No files matched your search
@@ -46,6 +46,7 @@ public/ Hand-written CSS, ~2KB of progressive-enhancement JS,
|
||||
config/ Per-Site config, one file per Site
|
||||
test/ node:test suites
|
||||
docs/plan.md The agreed plan
|
||||
docs/operating.md Publishing, fixing and rolling back, for whoever operates it
|
||||
docs/adr/ Decisions worth remembering
|
||||
deploy.sh Tests, build, push
|
||||
```
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
# Operating the menus
|
||||
|
||||
For whoever is on the other end of "the menu is wrong". The design is in
|
||||
`plan.md`, the vocabulary in `CONTEXT.md`, and the deployment in
|
||||
`ansible-playbooks/menu-admin/readme.md`.
|
||||
|
||||
## Where things are
|
||||
|
||||
| Thing | Where |
|
||||
| --- | --- |
|
||||
| The app | container `menu-app.<site>` on `monoco`, image `registry.tanshu.com/menu-admin:<tag>` |
|
||||
| Menu content | `/var/lib/menu-app.<site>/data` — `originals/`, `drafts/`, `published/` |
|
||||
| Admin | `https://<vhost>/admin` |
|
||||
| The password | `/var/lib/menu-app.<site>/site.env` (mode 600, never in git) |
|
||||
| Sections and branding | `config/<site>.site.json` in this repo, baked into the image |
|
||||
|
||||
## Everyday tasks
|
||||
|
||||
**Publish a new menu.** Sign in at `/admin`, find the Section, fill in "New Menu
|
||||
Set" (label, optional valid-until date), Create Draft, then drop the photos in.
|
||||
Drag to reorder — the order in the editor is the order customers see. Press
|
||||
Preview to check it, then Publish. Publishing archives whatever was current.
|
||||
|
||||
**Put an older menu back.** Open the Section's "older menus" fold in the admin,
|
||||
open the set you want and press "Make current". Its URL does not change.
|
||||
|
||||
**Change the admin password.** On the server:
|
||||
|
||||
```sh
|
||||
bash /tmp/provision-site-env.sh /var/lib/menu-app.hopsngrains.com --force
|
||||
docker restart menu-app.hopsngrains.com
|
||||
```
|
||||
|
||||
**Add, rename or reorder a Section.** Edit `config/<site>.site.json`, rebuild and
|
||||
push (`./deploy.sh <tag>`), then run the playbook with the new tag. Sections are
|
||||
deliberately not editable from the admin (ADR 0005).
|
||||
|
||||
**Fix a missing image.** If a Page's thumbnail or viewing image is lost while its
|
||||
Original survives, the app can rebuild them:
|
||||
|
||||
```sh
|
||||
docker run --rm --user 1000:1000 -e MENU_ROOT=/data \
|
||||
-e MENU_SITE_CONFIG=/app/config/hopsngrains.site.json \
|
||||
-v /var/lib/menu-app.hopsngrains.com:/data \
|
||||
--entrypoint node registry.tanshu.com/menu-admin:<tag> src/cli/repair.ts
|
||||
```
|
||||
|
||||
`src/cli/verify.ts` does the same inventory without changing anything, and is
|
||||
what the server refuses to boot on.
|
||||
|
||||
## When something looks wrong
|
||||
|
||||
| Symptom | Look here |
|
||||
| --- | --- |
|
||||
| Landing page says the menus are being updated | no Section has a Current Menu Set — publish one in the admin |
|
||||
| A Section 404s | that Section has no Current set, or the URL is misspelled |
|
||||
| Sign-in rejects the right password | `docker logs menu-app.<site>`, then check `site.env` exists and is owned by the app user |
|
||||
| Admin is missing entirely | `site.env` was unreadable at boot, so the admin is disabled: run the provision script and `docker restart` |
|
||||
| A photo uploads but looks wrong | the browser shrinks to 3000px before upload; that is expected, and the Original is kept |
|
||||
| The container will not start | `docker logs` — a fatal integrity problem makes it refuse to serve a data directory it cannot trust |
|
||||
| Caddy reload failed during a deploy | nothing is down: Caddy validates before applying. Read the error in the playbook output |
|
||||
| Everything is slow | check the tag is current; a Viewing image is ~1600px, so a Section page should not be megabytes |
|
||||
|
||||
**Logs.** `docker logs -f menu-app.<site>` — JSON lines, one per event, including
|
||||
every upload, publish, restore and failed sign-in.
|
||||
|
||||
**Checking from outside.** No sign-in, no writes, exits non-zero on failure, so
|
||||
it can gate a cutover:
|
||||
|
||||
```sh
|
||||
npm run check-site -- https://menu.hopsngrains.com
|
||||
```
|
||||
|
||||
## Backups
|
||||
|
||||
The app is the only writer. The irreplaceable part is
|
||||
`/var/lib/menu-app.<site>/data/originals` — the photos exactly as uploaded, from
|
||||
which everything else can be regenerated with `repair`. Back that directory up;
|
||||
`site.env` is worth keeping too (otherwise you re-provision the password).
|
||||
|
||||
## Rolling back
|
||||
|
||||
The old nginx Site is kept, not deleted. To go back:
|
||||
|
||||
```sh
|
||||
docker start nginx-menu.hopsngrains.com
|
||||
```
|
||||
|
||||
then point Caddy back at it — the cutover section of the playbook readme has the
|
||||
exact invocation, including the flag that stops the app's snippet from being
|
||||
re-added.
|
||||
+9
-4
@@ -75,10 +75,15 @@
|
||||
body: body,
|
||||
}).then(function (response) {
|
||||
return response.json().then(function (payload) {
|
||||
if (!response.ok) throw new Error(payload.error || 'upload failed');
|
||||
if (payload.failed && payload.failed.length > 0) {
|
||||
throw new Error(payload.failed.map(function (item) { return item.error; }).join('; '));
|
||||
}
|
||||
// The server reports per-file problems in `failed`, including on a 413,
|
||||
// so those messages win over a generic fallback.
|
||||
var detail = (payload.failed || [])
|
||||
.map(function (item) {
|
||||
return item.error;
|
||||
})
|
||||
.join('; ');
|
||||
if (!response.ok) throw new Error(detail || payload.error || 'upload failed');
|
||||
if (detail) throw new Error(detail);
|
||||
return payload;
|
||||
});
|
||||
});
|
||||
|
||||
+68
-25
@@ -25,12 +25,21 @@ import { sessionCookieAttributes, SESSION_TTL_MS } from '../server.ts';
|
||||
|
||||
const HTML = 'text/html; charset=utf-8';
|
||||
|
||||
/** Small limits are readable too, so the message is true for any Site config. */
|
||||
export function describeBytes(bytes: number): string {
|
||||
if (bytes >= 1024 * 1024) return `${Math.round(bytes / (1024 * 1024))} MB`;
|
||||
if (bytes >= 1024) return `${Math.round(bytes / 1024)} KB`;
|
||||
return `${bytes} bytes`;
|
||||
}
|
||||
|
||||
export interface AdminRouteDeps {
|
||||
config: SiteConfig;
|
||||
store: SetStore;
|
||||
sessionSecret: string;
|
||||
adminPasswordHash: string | null;
|
||||
secureCookies: boolean;
|
||||
/** Quoted back to the user when a file is refused for being too large. */
|
||||
maxUploadBytes: number;
|
||||
throttle?: LoginThrottle | undefined;
|
||||
}
|
||||
|
||||
@@ -303,33 +312,67 @@ export async function registerAdminRoutes(app: FastifyInstance, deps: AdminRoute
|
||||
let authorised = verifyCsrf(token, deps.sessionSecret, request.headers['x-csrf']);
|
||||
const added: string[] = [];
|
||||
const failed: { file: string; error: string }[] = [];
|
||||
for await (const part of request.parts()) {
|
||||
if (part.type === 'field') {
|
||||
if (part.fieldname === 'csrf') {
|
||||
authorised = authorised || verifyCsrf(token, deps.sessionSecret, part.value);
|
||||
try {
|
||||
for await (const part of request.parts()) {
|
||||
if (part.type === 'field') {
|
||||
if (part.fieldname === 'csrf') {
|
||||
authorised = authorised || verifyCsrf(token, deps.sessionSecret, part.value);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (!authorised) {
|
||||
await part.toBuffer().catch(() => undefined);
|
||||
continue;
|
||||
}
|
||||
const filename = part.filename || 'upload';
|
||||
try {
|
||||
const data = await part.toBuffer();
|
||||
const page = await store.addPage(set.id, { data, originalName: filename });
|
||||
added.push(page.id);
|
||||
} catch (error) {
|
||||
const code = (error as { code?: string }).code;
|
||||
// A limit error belongs to the whole upload, not to this file: let the
|
||||
// outer catch describe it once instead of twice.
|
||||
if (
|
||||
code === 'FST_REQ_FILE_TOO_LARGE' ||
|
||||
code === 'FST_FILES_LIMIT' ||
|
||||
code === 'FST_PARTS_LIMIT'
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
failed.push({
|
||||
file: filename,
|
||||
error:
|
||||
error instanceof UnsupportedImageError || error instanceof StoreError
|
||||
? error.message
|
||||
: `${filename} could not be uploaded`,
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (!authorised) {
|
||||
await part.toBuffer().catch(() => undefined);
|
||||
continue;
|
||||
}
|
||||
const filename = part.filename || 'upload';
|
||||
try {
|
||||
const data = await part.toBuffer();
|
||||
const page = await store.addPage(set.id, { data, originalName: filename });
|
||||
added.push(page.id);
|
||||
} catch (error) {
|
||||
const message =
|
||||
error instanceof UnsupportedImageError
|
||||
? error.message
|
||||
: error instanceof StoreError
|
||||
? error.message
|
||||
: error instanceof Error && 'code' in error && error.code === 'FST_REQ_FILE_TOO_LARGE'
|
||||
? `${filename} is larger than the upload limit`
|
||||
: `${filename} could not be uploaded`;
|
||||
failed.push({ file: filename, error: message });
|
||||
}
|
||||
} catch (error) {
|
||||
// A file past the size limit makes the multipart stream itself throw, so
|
||||
// this has to be caught around the loop rather than around toBuffer().
|
||||
// Without it the phone that just tried to send a 35 MB photo gets an
|
||||
// opaque "Something went wrong".
|
||||
if (!authorised) return reply.code(403).send('Rejected.');
|
||||
const code = (error as { code?: string }).code;
|
||||
const message =
|
||||
code === 'FST_REQ_FILE_TOO_LARGE'
|
||||
? `That file is larger than the ${describeBytes(deps.maxUploadBytes)} limit.`
|
||||
: code === 'FST_FILES_LIMIT'
|
||||
? 'That upload had more files than this Site accepts at once.'
|
||||
: code === 'FST_PARTS_LIMIT'
|
||||
? 'That upload had more parts than we accept.'
|
||||
: null;
|
||||
if (message === null) throw error;
|
||||
request.log.warn({ code }, 'upload rejected by the multipart limits');
|
||||
failed.push({ file: 'upload', error: message });
|
||||
if (wantsJson(request)) return reply.code(413).send({ added: added.length, failed });
|
||||
return back(
|
||||
request,
|
||||
reply,
|
||||
`${message} ${added.length} page${added.length === 1 ? '' : 's'} uploaded before that.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (!authorised) return reply.code(403).send('Rejected.');
|
||||
|
||||
@@ -98,6 +98,7 @@ export async function buildApp(options: AppOptions): Promise<FastifyInstance> {
|
||||
sessionSecret: options.sessionSecret,
|
||||
adminPasswordHash: options.adminPasswordHash,
|
||||
secureCookies: options.secureCookies,
|
||||
maxUploadBytes: options.config.limits.maxUploadBytes,
|
||||
throttle: options.throttle,
|
||||
});
|
||||
|
||||
|
||||
+47
-1
@@ -19,7 +19,10 @@ let csrf: string;
|
||||
|
||||
const CLOCK = testClock('2026-05-01T06:00:00.000Z');
|
||||
|
||||
async function build(throttle?: LoginThrottle): Promise<FastifyInstance> {
|
||||
async function build(
|
||||
throttle?: LoginThrottle,
|
||||
limits?: { maxUploadBytes?: number; maxPagesPerSet?: number },
|
||||
): Promise<FastifyInstance> {
|
||||
const built = await buildApp({
|
||||
config: parseSiteConfig({
|
||||
site: 'menu.example.test',
|
||||
@@ -28,6 +31,7 @@ async function build(throttle?: LoginThrottle): Promise<FastifyInstance> {
|
||||
{ slug: 'food', label: 'Food', order: 1 },
|
||||
{ slug: 'bar', label: 'Bar', order: 2 },
|
||||
],
|
||||
...(limits === undefined ? {} : { limits }),
|
||||
}),
|
||||
dataDir,
|
||||
sessionSecret: SECRET,
|
||||
@@ -319,6 +323,48 @@ describe('uploading Pages', () => {
|
||||
assert.equal(response.statusCode, 409);
|
||||
});
|
||||
|
||||
it('reports a file over the upload limit, stores nothing, and stays healthy', async () => {
|
||||
const limited = await build(undefined, { maxUploadBytes: 2048 });
|
||||
const created = await limited.inject({
|
||||
method: 'POST',
|
||||
url: '/admin/sets',
|
||||
cookies: cookies(),
|
||||
headers: { 'x-csrf': csrf },
|
||||
payload: { section: 'bar', label: 'Bar Menu' },
|
||||
});
|
||||
const id = String(created.headers['location']).replace('/admin/sets/', '');
|
||||
|
||||
const boundary = '----oversize';
|
||||
const payload = Buffer.concat([
|
||||
Buffer.from(
|
||||
`--${boundary}\r\nContent-Disposition: form-data; name="pages"; filename="huge.jpg"\r\nContent-Type: image/jpeg\r\n\r\n`,
|
||||
),
|
||||
Buffer.alloc(8 * 1024, 7),
|
||||
Buffer.from(`\r\n--${boundary}--\r\n`),
|
||||
]);
|
||||
|
||||
const response = await limited.inject({
|
||||
method: 'POST',
|
||||
url: `/admin/sets/${id}/pages`,
|
||||
cookies: cookies(),
|
||||
headers: {
|
||||
'x-csrf': csrf,
|
||||
accept: 'application/json',
|
||||
'content-type': `multipart/form-data; boundary=${boundary}`,
|
||||
},
|
||||
payload,
|
||||
});
|
||||
|
||||
const body = response.json() as { added: number; failed: { error: string }[] };
|
||||
assert.equal(response.statusCode, 413);
|
||||
assert.equal(body.added, 0);
|
||||
assert.equal(body.failed.length, 1, 'the reason is reported once, not twice');
|
||||
assert.match(body.failed.map((item) => item.error).join(' '), /larger than the 2 KB limit/);
|
||||
assert.deepEqual((await limited.menu.store.get(id)).manifest.pages, []);
|
||||
assert.equal((await limited.inject({ method: 'GET', url: '/healthz' })).statusCode, 200);
|
||||
await limited.close();
|
||||
});
|
||||
|
||||
it('removes a Page', async () => {
|
||||
const { id } = await newSet();
|
||||
await upload(id, 'one.jpg');
|
||||
|
||||
@@ -109,4 +109,21 @@ describe('createImagePipeline', () => {
|
||||
error instanceof UnsupportedImageError && error.message.includes('gif'),
|
||||
);
|
||||
});
|
||||
|
||||
it('strips metadata from the derivatives but keeps the Original as uploaded', async () => {
|
||||
const tagged = await sharp({
|
||||
create: { width: 1200, height: 900, channels: 3, background: { r: 10, g: 90, b: 120 } },
|
||||
})
|
||||
.withExif({ IFD0: { Copyright: 'Test Kitchen', Artist: 'Nobody' } })
|
||||
.jpeg()
|
||||
.toBuffer();
|
||||
|
||||
assert.notEqual((await sharp(tagged).metadata()).exif, undefined);
|
||||
|
||||
const derivatives = await pipeline.process(tagged, 'tagged.jpg');
|
||||
assert.equal((await sharp(derivatives.webp).metadata()).exif, undefined);
|
||||
assert.equal((await sharp(derivatives.jpeg).metadata()).exif, undefined);
|
||||
assert.equal((await sharp(derivatives.thumb).metadata()).exif, undefined);
|
||||
assert.equal(Buffer.compare(derivatives.original, tagged), 0);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user