An upload over a few MB came back as a 400 with an empty body, no message anywhere, and nothing logged by Immich, the sidecar or officer. It was a race, not a size limit. Immich judges an asset from its first few KB and rejects immediately, then closes; both our hops were still writing the body; Node treats the leftover bytes as a protocol violation and replaces the application's answer with a bodyless `400 Bad Request` + `Connection: close`. The real message never reached the wire. Measured before the change: streamed lost the message 1/4 at 8 MB and 4/4 at 32 MB — probability rising with size, which is why small photos usually worked and a phone's video never did. Both hops needed it. Fixing only the sidecar took 32 MB from 4/4 failing to 2/4, because the platform proxy was losing it one hop up. Bounded at 512 MB, above which the body streams exactly as before. That ceiling is not a refusal and is deliberately not a 413: a file Immich ACCEPTS is read to the end and never races, so a 4 GB video is unaffected. All that is given up above the cap is the error message on a file that was going to be rejected anyway. A first attempt refused over-cap uploads outright and would have broken the working 4 GB case to improve diagnosis of the doomed one. `bufferRequestBody` is opt-in and off by default: the vault and wallet proxies must keep streaming so a passphrase or macaroon never lands in the platform's heap. Also adds the proxy error logging that made this findable at all — status and two byte counts from headers, never the bodies. `responseBytes: "unknown"` is what exposed the stripped response. Verified live at 32/256 MB (buffered) and 640 MB (streamed, passes through). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
233 lines
9.2 KiB
TypeScript
233 lines
9.2 KiB
TypeScript
import { describe, expect, it, afterAll, beforeAll } from 'bun:test';
|
|
import { Hono } from 'hono';
|
|
import { bodyParser } from '../_middlewares/body-parser';
|
|
import { createRouter } from '../create-router';
|
|
import { handleSidecarMessage } from '../sidecar-registry';
|
|
import { createSidecarProxy } from './create-proxy';
|
|
|
|
// Regression cover for COMMS/MULTIPART_PROXY_BUG.md: every multipart upload through a sidecar proxy
|
|
// arrived corrupted, because bodyParser parsed a body the proxy only meant to forward. Hono cached the
|
|
// resulting FormData and re-serialised it with a NEW boundary when the proxy asked for the bytes, while
|
|
// the proxy still forwarded the ORIGINAL content-type header. The far side then failed with
|
|
// "Multipart: Unexpected end of form".
|
|
//
|
|
// The assertion that matters is the one the bug report names: the boundary in the content-type header and
|
|
// the boundary in the body must be the same one the client sent.
|
|
|
|
const BOUNDARY = '6600BE83-EFF4-498D-9739-B6E978654467';
|
|
|
|
// A JPEG-shaped payload with embedded NULs and high bytes — the things a naive text round-trip mangles.
|
|
// Deliberately not tiny: Bun 1.3.10's multipart parser truncates an 8-byte part at its first NUL, which is
|
|
// a fixture artefact (64 bytes and up are fine) but cost time to chase, so it is written down here.
|
|
const FILE_BYTES = (() => {
|
|
const bytes = new Uint8Array(512);
|
|
bytes.set([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, 0x49, 0x46, 0x00, 0x01]);
|
|
for (let i = 12; i < bytes.length; i++) bytes[i] = (i * 7) % 256;
|
|
bytes.set([0xff, 0xd9], bytes.length - 2);
|
|
return bytes;
|
|
})();
|
|
|
|
/** The phone's exact wire format: a UUID boundary, fields in a fixed order, a binary file part. */
|
|
function phoneMultipart(): { body: Uint8Array<ArrayBuffer>; contentType: string } {
|
|
const fields = [
|
|
['deviceAssetId', 'probe-1'],
|
|
['deviceId', 'probe-device'],
|
|
['fileCreatedAt', '2026-08-07T00:00:00.000Z'],
|
|
['fileModifiedAt', '2026-08-07T00:00:00.000Z'],
|
|
['isFavorite', 'false'],
|
|
];
|
|
const head = fields
|
|
.map(([k, v]) => `--${BOUNDARY}\r\nContent-Disposition: form-data; name="${k}"\r\n\r\n${v}\r\n`)
|
|
.join('');
|
|
const filePrologue =
|
|
`--${BOUNDARY}\r\n` +
|
|
`Content-Disposition: form-data; name="assetData"; filename="t.jpg"\r\n` +
|
|
`Content-Type: image/jpeg\r\n\r\n`;
|
|
const epilogue = `\r\n--${BOUNDARY}--\r\n`;
|
|
|
|
const enc = new TextEncoder();
|
|
const parts = [enc.encode(head), enc.encode(filePrologue), FILE_BYTES, enc.encode(epilogue)];
|
|
const body = new Uint8Array(new ArrayBuffer(parts.reduce((n, p) => n + p.length, 0)));
|
|
let at = 0;
|
|
for (const p of parts) {
|
|
body.set(p, at);
|
|
at += p.length;
|
|
}
|
|
return { body, contentType: `multipart/form-data; boundary=${BOUNDARY}` };
|
|
}
|
|
|
|
/** Stands in for a sidecar. Reports what actually arrived on the wire. */
|
|
type Seen = {
|
|
contentType: string;
|
|
bodyBoundary: string | null;
|
|
fields: string[];
|
|
fileBytes: number[] | null;
|
|
raw: Uint8Array;
|
|
};
|
|
|
|
let sidecar: ReturnType<typeof Bun.serve>;
|
|
let app: Hono;
|
|
let seen: Seen | null = null;
|
|
|
|
beforeAll(async () => {
|
|
sidecar = Bun.serve({
|
|
port: 0,
|
|
async fetch(req) {
|
|
// A rejection with no body at all — the shape from COMMS/UPLOAD_EMPTY_400.md that tells the client
|
|
// "no" and gives it nothing to act on.
|
|
if (new URL(req.url).pathname.endsWith('/empty400')) return new Response(null, { status: 400 });
|
|
|
|
const contentType = req.headers.get('content-type') ?? '';
|
|
const raw = new Uint8Array(await req.arrayBuffer());
|
|
const asText = new TextDecoder().decode(raw);
|
|
const bodyBoundary = asText.match(/^--([^\r\n-][^\r\n]*?)\r\n/)?.[1] ?? null;
|
|
|
|
// Re-parse from the bytes we actually received, exactly as Immich would.
|
|
let fields: string[] = [];
|
|
let fileBytes: number[] | null = null;
|
|
try {
|
|
const form = await new Request('http://x', {
|
|
method: 'POST',
|
|
headers: { 'content-type': contentType },
|
|
body: raw,
|
|
}).formData();
|
|
fields = [...form.keys()];
|
|
const f = form.get('assetData');
|
|
if (f instanceof File) fileBytes = [...new Uint8Array(await f.arrayBuffer())];
|
|
} catch (err) {
|
|
seen = { contentType, bodyBoundary, fields: [], fileBytes: null, raw };
|
|
return new Response(JSON.stringify({ message: String(err) }), { status: 400 });
|
|
}
|
|
|
|
seen = { contentType, bodyBoundary, fields, fileBytes, raw };
|
|
return new Response(JSON.stringify({ ok: true }), { status: 201 });
|
|
},
|
|
});
|
|
|
|
const proxy = createSidecarProxy({ name: 'testcar', prefix: '/api/testcar' });
|
|
// What the registration socket would deliver when the sidecar reports its port.
|
|
handleSidecarMessage('test', { type: 'testcar:server', port: sidecar.port } as never);
|
|
|
|
const protectedRouter = createRouter();
|
|
protectedRouter.use(bodyParser());
|
|
protectedRouter.use(async (ctx, next) => {
|
|
ctx.set('user', { id: 1 } as never);
|
|
return next();
|
|
});
|
|
protectedRouter.route('/testcar', proxy.router);
|
|
|
|
app = new Hono();
|
|
app.route('/api', protectedRouter);
|
|
});
|
|
|
|
afterAll(() => sidecar?.stop(true));
|
|
|
|
describe('sidecar proxy multipart forwarding', () => {
|
|
it('forwards the client boundary unchanged, in both the header and the body', async () => {
|
|
const { body, contentType } = phoneMultipart();
|
|
|
|
const res = await app.request('http://x/api/testcar/_officer/assets', {
|
|
method: 'POST',
|
|
headers: { 'content-type': contentType },
|
|
body: new Blob([body]),
|
|
});
|
|
|
|
expect(res.status).toBe(201);
|
|
expect(seen).not.toBeNull();
|
|
// The bug: header said one boundary, body used another.
|
|
expect(seen!.contentType).toContain(BOUNDARY);
|
|
expect(seen!.bodyBoundary).toBe(BOUNDARY);
|
|
// Bun's FormData re-serialiser announces itself; if this appears, the body was rebuilt.
|
|
expect(seen!.bodyBoundary).not.toContain('WebkitFormBoundary');
|
|
});
|
|
|
|
it('delivers the body byte-for-byte, fields in the order sent', async () => {
|
|
const { body, contentType } = phoneMultipart();
|
|
|
|
const res = await app.request('http://x/api/testcar/_officer/assets', {
|
|
method: 'POST',
|
|
headers: { 'content-type': contentType },
|
|
body: new Blob([body]),
|
|
});
|
|
|
|
expect(res.status).toBe(201);
|
|
// The strongest form of "forwarded, not rebuilt": the bytes on the far side are the bytes we sent.
|
|
expect([...seen!.raw]).toEqual([...body]);
|
|
expect(seen!.fields).toEqual([
|
|
'deviceAssetId',
|
|
'deviceId',
|
|
'fileCreatedAt',
|
|
'fileModifiedAt',
|
|
'isFavorite',
|
|
'assetData',
|
|
]);
|
|
expect(seen!.fileBytes).toEqual([...FILE_BYTES]);
|
|
});
|
|
|
|
it('reports a rejection instead of passing it on in silence', async () => {
|
|
// COMMS/UPLOAD_EMPTY_400.md: uploads failed with an empty-bodied 400 and nothing logged it anywhere,
|
|
// so localising the fault took an evening of curl bisection. The platform is the only hop that sees
|
|
// both sides. `requestBytes` is the field that would have cracked it — a size threshold is legible at
|
|
// a glance instead of by bisection.
|
|
const warnings: unknown[][] = [];
|
|
const realWarn = console.warn;
|
|
console.warn = (...args: unknown[]) => void warnings.push(args);
|
|
|
|
try {
|
|
const payload = JSON.stringify({ some: 'payload' });
|
|
const res = await app.request('http://x/api/testcar/_officer/empty400', {
|
|
method: 'POST',
|
|
// Set explicitly: `app.request` does not derive it, and this is the field the bug report needed.
|
|
headers: { 'content-type': 'application/json', 'content-length': String(payload.length) },
|
|
body: payload,
|
|
});
|
|
|
|
// Pass-through is unchanged: the proxy reports, it does not rewrite.
|
|
expect(res.status).toBe(400);
|
|
expect(await res.text()).toBe('');
|
|
|
|
expect(warnings.length).toBe(1);
|
|
const [message, detail] = warnings[0] as [string, Record<string, unknown>];
|
|
expect(message).toContain('[testcar]');
|
|
expect(message).toContain('POST /_officer/empty400');
|
|
expect(message).toContain('400');
|
|
// The empty body is named, not left to be inferred from a byte count of zero.
|
|
expect(detail.emptyBody).toBe(true);
|
|
expect(detail.requestBytes).toBe(String(payload.length));
|
|
expect(detail.responseBytes).toBe('0');
|
|
} finally {
|
|
console.warn = realWarn;
|
|
}
|
|
});
|
|
|
|
it('says nothing about a request that succeeded', async () => {
|
|
const { body, contentType } = phoneMultipart();
|
|
const warnings: unknown[][] = [];
|
|
const realWarn = console.warn;
|
|
console.warn = (...args: unknown[]) => void warnings.push(args);
|
|
|
|
try {
|
|
const res = await app.request('http://x/api/testcar/_officer/assets', {
|
|
method: 'POST',
|
|
headers: { 'content-type': contentType },
|
|
body: new Blob([body]),
|
|
});
|
|
expect(res.status).toBe(201);
|
|
expect(warnings).toEqual([]);
|
|
} finally {
|
|
console.warn = realWarn;
|
|
}
|
|
});
|
|
|
|
it('still forwards a JSON body untouched', async () => {
|
|
const res = await app.request('http://x/api/testcar/_officer/echo', {
|
|
method: 'POST',
|
|
headers: { 'content-type': 'application/json' },
|
|
body: JSON.stringify({ hello: 'world' }),
|
|
});
|
|
|
|
expect(res.status).toBe(400); // not multipart — the stub fails to parse it, which is fine
|
|
expect(seen!.contentType).toBe('application/json');
|
|
});
|
|
});
|