From 63f7a14b6cf4edac5c0e048c917e3ea1ba4987c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 8 Aug 2026 15:45:01 +0000 Subject: [PATCH] stream request bodies through the photos sidecar both forwarders buffered the whole request body into an ArrayBuffer before re-sending it to Immich. with maxRequestBodySize at 4GB that put a phone's video upload in the sidecar's heap for a hop that never reads the bytes. callUpstream now sets duplex: 'half' so a stream is a legal body, matching what createSidecarProxy already does on the platform side. the four JSON callers are unaffected. the platform proxy forwards no content-length, so the body already reached us chunked; this extends that one hop to Immich. verified against the live instance (3.1.0): bulk-upload-check round-trips a streamed body and returns the right verdict. Co-Authored-By: Claude Opus 5 --- src/servers/sidecar/photos/locked.ts | 3 ++- src/servers/sidecar/photos/routes.ts | 3 ++- src/servers/sidecar/photos/upstream.ts | 18 ++++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/servers/sidecar/photos/locked.ts b/src/servers/sidecar/photos/locked.ts index 9f637ec6..a52b970a 100644 --- a/src/servers/sidecar/photos/locked.ts +++ b/src/servers/sidecar/photos/locked.ts @@ -268,7 +268,8 @@ async function forward(req: Request, url: URL, cfg: UpstreamConfig, userId: numb path: `/api/${rest}`, method: req.method, query: search ? `?${search}` : '', - body: hasBody ? await req.arrayBuffer() : null, + // Streamed, not buffered — same reason as the unlocked forwarder in routes.ts. + body: hasBody ? req.body : null, contentType: req.headers.get('content-type'), range: req.headers.get('range'), ifNoneMatch: req.headers.get('if-none-match'), diff --git a/src/servers/sidecar/photos/routes.ts b/src/servers/sidecar/photos/routes.ts index 604eaa17..f6950587 100644 --- a/src/servers/sidecar/photos/routes.ts +++ b/src/servers/sidecar/photos/routes.ts @@ -113,7 +113,8 @@ export async function handleOfficerRoute(cfg: UpstreamConfig, req: Request, url: path: `/api/${rest}`, method: req.method, query: search ? `?${search}` : '', - body: hasBody ? await req.arrayBuffer() : null, + // Streamed, not buffered: an asset upload can be gigabytes and this layer never reads the bytes. + body: hasBody ? req.body : null, contentType: req.headers.get('content-type'), range: req.headers.get('range'), ifNoneMatch: req.headers.get('if-none-match'), diff --git a/src/servers/sidecar/photos/upstream.ts b/src/servers/sidecar/photos/upstream.ts index 98e38dde..2a429735 100644 --- a/src/servers/sidecar/photos/upstream.ts +++ b/src/servers/sidecar/photos/upstream.ts @@ -164,6 +164,14 @@ type CallOptions = { method?: string; /** Raw search string including the leading `?`, or empty. */ query?: string; + /** + * A string for the JSON callers, or the caller's own request body as a STREAM for the two forwarders. + * + * Streamed rather than buffered because a phone's video upload is bounded by `maxRequestBodySize` + * (4 GB) and buffering put all of it in this process's heap for a hop that never looks at the bytes. + * Streaming to Immich means chunked transfer-encoding instead of Content-Length — which is already how + * the body reaches us, since the platform proxy forwards no content-length either. + */ body?: BodyInit | null; contentType?: string | null; /** Byte range for thumbnail/original/video reads, forwarded verbatim. */ @@ -192,14 +200,20 @@ export async function callUpstream(cfg: UpstreamTarget, opts: CallOptions): Prom if (opts.range) headers.Range = opts.range; if (opts.ifNoneMatch) headers['If-None-Match'] = opts.ifNoneMatch; - const res = await fetch(`${cfg.base}${opts.path}${opts.query ?? ''}`, { + // `duplex: 'half'` is required by the fetch spec whenever the body is a stream, and is absent from + // TypeScript's RequestInit — hence the widened type, same as createSidecarProxy does. Harmless for the + // string bodies. + const init: RequestInit & { duplex?: 'half' } = { method: opts.method ?? 'GET', headers, body: opts.body ?? undefined, + duplex: 'half', // A redirect from an API route means something has gone wrong with auth; surface it rather than // following it into an HTML page. redirect: 'manual', - }); + }; + + const res = await fetch(`${cfg.base}${opts.path}${opts.query ?? ''}`, init); if (res.status === 401 || res.status === 403) { // 403 is the interesting one: the key is valid but lacks the permission this route needs.