diff --git a/functions/src/utils/stream-object.test.ts b/functions/src/utils/stream-object.test.ts new file mode 100644 index 000000000..8d500fa9f --- /dev/null +++ b/functions/src/utils/stream-object.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest'; +import { parseByteRange } from './stream-object'; + +describe('parseByteRange', () => { + it('returns undefined when there is nothing to honour', () => { + expect(parseByteRange(undefined, 100)).toBeUndefined(); + expect(parseByteRange('items=0-5', 100)).toBeUndefined(); + expect(parseByteRange('bytes=-', 100)).toBeUndefined(); + expect(parseByteRange('bytes=0-1,5-6', 100)).toBeUndefined(); + expect(parseByteRange('bytes=9-2', 100)).toBeUndefined(); + }); + + it('parses a closed range', () => { + expect(parseByteRange('bytes=0-9', 100)).toEqual({ start: 0, end: 9 }); + }); + + it('parses an open-ended range to the last byte', () => { + expect(parseByteRange('bytes=90-', 100)).toEqual({ start: 90, end: 99 }); + }); + + it('clamps an end past the object', () => { + expect(parseByteRange('bytes=90-500', 100)).toEqual({ start: 90, end: 99 }); + }); + + it('parses a suffix range, clamped to the object', () => { + expect(parseByteRange('bytes=-10', 100)).toEqual({ start: 90, end: 99 }); + expect(parseByteRange('bytes=-500', 100)).toEqual({ start: 0, end: 99 }); + }); + + it('reports a range wholly outside the object as unsatisfiable', () => { + expect(parseByteRange('bytes=100-', 100)).toBe('unsatisfiable'); + expect(parseByteRange('bytes=-0', 100)).toBe('unsatisfiable'); + }); +}); diff --git a/functions/src/utils/stream-object.ts b/functions/src/utils/stream-object.ts new file mode 100644 index 000000000..01b5aecf2 --- /dev/null +++ b/functions/src/utils/stream-object.ts @@ -0,0 +1,102 @@ +import type { Request, Response } from 'express'; +import { logger } from 'firebase-functions'; +import type { Readable } from 'stream'; + +/** + * Cloud Run rejects a response larger than 32 MiB with a bare `500` unless it is streamed with + * chunked transfer encoding — and a response carrying `Content-Length` is not streamed in that + * sense. Bodies at or under this size keep their `Content-Length` (so clients show progress); + * anything larger is sent without one, which is what makes Node fall back to chunked encoding. + * Kept a little under the hard limit to leave room for the headers. + */ +export const MAX_CONTENT_LENGTH_BYTES = 31 * 1024 * 1024; + +export type ByteRange = { start: number; end: number }; + +/** + * Parses a `Range` header against an object of `size` bytes. + * + * - `undefined`: no range to honour — serve the whole object. Covers a missing header, a unit + * other than `bytes`, malformed syntax and multi-range requests, all of which RFC 9110 lets a + * server answer with a plain `200`. + * - `'unsatisfiable'`: well-formed but entirely outside the object — answer `416`. + * - otherwise the inclusive byte range to send, clamped to the object. + * @param {string | undefined} header Raw `Range` request header + * @param {number} size Object size in bytes + * @return {ByteRange | 'unsatisfiable' | undefined} the range to serve, if any + */ +export function parseByteRange(header: string | undefined, size: number): ByteRange | 'unsatisfiable' | undefined { + if (!header) return undefined; + const match = /^bytes=(\d*)-(\d*)$/.exec(header.trim()); + if (!match) return undefined; + const [, first, last] = match; + if (first === '' && last === '') return undefined; + if (first === '') { + // Suffix range: the final `last` bytes. + const length = Number(last); + if (length === 0 || size === 0) return 'unsatisfiable'; + return { start: Math.max(0, size - length), end: size - 1 }; + } + const start = Number(first); + const end = last === '' ? size - 1 : Math.min(Number(last), size - 1); + if (last !== '' && Number(last) < start) return undefined; + if (start >= size) return 'unsatisfiable'; + return { start, end }; +} + +/** The slice of a Storage `File` this needs, so tests can hand in a fake. */ +export interface StreamableFile { + createReadStream(options?: { start?: number; end?: number; validation?: false }): Readable; +} + +/** + * Streams a stored object to the response, honouring a single `Range`. Headers other than + * length and range (type, disposition, cache) must already be set by the caller. + * + * Replaces `download()` to `/tmp` + `sendFile`: on Cloud Functions `/tmp` is RAM, so that held + * the whole object in memory, and `sendFile` always sets `Content-Length` — which is what made + * every object over 32 MiB fail. + * @param {Request} req Incoming request, read for `Range` and the method + * @param {Response} res Response to stream into + * @param {StreamableFile} file Storage object to read + * @param {number} size Object size in bytes, from its metadata + * @param {string} tag Log prefix of the calling route + */ +export function streamStoredObject(req: Request, res: Response, file: StreamableFile, size: number, tag: string): void { + res.header('Accept-Ranges', 'bytes'); + const range = parseByteRange(req.headers.range, size); + if (range === 'unsatisfiable') { + res.status(416).header('Content-Range', `bytes */${size}`).header('Cache-Control', 'no-cache').end(); + return; + } + const { start, end } = range ?? { start: 0, end: size - 1 }; + const length = size === 0 ? 0 : end - start + 1; + if (range) { + res.status(206).header('Content-Range', `bytes ${start}-${end}/${size}`); + } + if (length <= MAX_CONTENT_LENGTH_BYTES) { + res.header('Content-Length', String(length)); + } + if (req.method === 'HEAD' || length === 0) { + res.end(); + return; + } + // `validation: false` because a partial read cannot be checksummed, and a full one is + // already covered by the ETag the caller sent. + const stream = file.createReadStream(range ? { start, end, validation: false } : { validation: false }); + stream.on('error', error => { + logger.error(`${tag} stream failed`, error); + if (res.headersSent) { + // Mid-body: the status is gone, so the only honest signal left is a truncated response. + res.destroy(error); + } else { + res.status(500).removeHeader('Content-Length'); + res.removeHeader('Content-Range'); + res.removeHeader('Content-Disposition'); + res.header('Cache-Control', 'no-cache').end(); + } + }); + // A client that disconnects should stop the Storage read rather than let it run to the end. + res.on('close', () => stream.destroy()); + stream.pipe(res); +} diff --git a/functions/src/v1/cdn-assets.test.ts b/functions/src/v1/cdn-assets.test.ts index b3aec7c92..cc56e872e 100644 --- a/functions/src/v1/cdn-assets.test.ts +++ b/functions/src/v1/cdn-assets.test.ts @@ -1,6 +1,7 @@ import type { Express } from 'express'; import express from 'express'; import { writeFileSync } from 'fs'; +import { Readable } from 'stream'; import sharp from 'sharp'; import request from 'supertest'; import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -46,7 +47,9 @@ async function jpegFixture(quality = 95): Promise { raw[i + 2] = (x * y) % 255; } } - return sharp(raw, { raw: { width, height, channels: 3 } }).jpeg({ quality }).toBuffer(); + return sharp(raw, { raw: { width, height, channels: 3 } }) + .jpeg({ quality }) + .toBuffer(); } /** A genuinely multi-frame GIF; frames must differ or the encoder collapses them to one. */ @@ -81,9 +84,16 @@ interface StoredAsset { documentMissing?: boolean; } -function given(asset: StoredAsset): { download: ReturnType } { +function given(asset: StoredAsset): { download: ReturnType; createReadStream: ReturnType } { const file = { - getMetadata: asset.storageMissing ? vi.fn().mockRejectedValue(new Error('not found')) : vi.fn().mockResolvedValue([{ md5Hash: MD5 }]), + getMetadata: asset.storageMissing + ? vi.fn().mockRejectedValue(new Error('not found')) + : vi.fn().mockResolvedValue([{ md5Hash: MD5, size: String(asset.bytes.length) }]), + createReadStream: vi + .fn() + .mockImplementation((options?: { start?: number; end?: number }) => + Readable.from([asset.bytes.subarray(options?.start ?? 0, options?.end === undefined ? undefined : options.end + 1)]) + ), download: vi.fn().mockImplementation(async (options?: { destination: string }) => { if (options?.destination) { writeFileSync(options.destination, asset.bytes); @@ -111,6 +121,13 @@ function app(): Express { const URL = '/api/v1/spaces/s1/assets/a1'; +/** supertest only buffers known binary types; this keeps any body as raw bytes. */ +function binaryParser(res: NodeJS.ReadableStream, callback: (err: Error | null, body: Buffer) => void): void { + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => callback(null, Buffer.concat(chunks))); +} + beforeEach(() => { vi.restoreAllMocks(); }); @@ -138,6 +155,50 @@ describe('asset routes — the passthrough pair', () => { expect(res.body.length).toBe(bytes.length); }); + it('/download streams from Storage instead of staging the object in /tmp', async () => { + const bytes = Buffer.from('video bytes'); + const file = given({ bytes, type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`).buffer(true).parse(binaryParser); + + expect(res.status).toBe(200); + expect(res.headers['accept-ranges']).toBe('bytes'); + expect(res.headers['content-length']).toBe(String(bytes.length)); + expect(Buffer.compare(res.body, bytes)).toBe(0); + expect(file.download).not.toHaveBeenCalled(); + }); + + it('/download honours a Range request with 206', async () => { + const bytes = Buffer.from('0123456789'); + given({ bytes, type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`).set('Range', 'bytes=2-5').buffer(true).parse(binaryParser); + + expect(res.status).toBe(206); + expect(res.headers['content-range']).toBe('bytes 2-5/10'); + expect(res.body.toString()).toBe('2345'); + }); + + it('/download answers an out-of-bounds Range with 416', async () => { + given({ bytes: Buffer.from('0123456789'), type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`).set('Range', 'bytes=50-'); + + expect(res.status).toBe(416); + expect(res.headers['content-range']).toBe('bytes */10'); + }); + + it('a bare transform-route URL on a video streams the stored bytes', async () => { + const bytes = Buffer.from('video bytes'); + const file = given({ bytes, type: 'video/mp4', extension: '.mp4' }); + + const res = await request(app()).get(URL).buffer(true).parse(binaryParser); + + expect(res.status).toBe(200); + expect(Buffer.compare(res.body, bytes)).toBe(0); + expect(file.download).not.toHaveBeenCalled(); + }); + it.each([['/original'], ['/download']])('%s rejects a transform parameter rather than ignoring it', async route => { given({ bytes: await jpegFixture(), type: 'image/jpeg', extension: '.jpg' }); diff --git a/functions/src/v1/cdn.ts b/functions/src/v1/cdn.ts index ceac0c994..4815928f4 100644 --- a/functions/src/v1/cdn.ts +++ b/functions/src/v1/cdn.ts @@ -55,6 +55,7 @@ import { assetResponsePolicy } from '../utils/asset-headers'; import { buildContentDisposition } from '../utils/content-disposition'; import { redactQuery } from '../utils/log-redact'; import { resolveLocaleFilePath } from '../utils/locale-utils'; +import { streamStoredObject } from '../utils/stream-object'; import { RequestWithToken, requireContentPermissions, @@ -535,9 +536,10 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { // Set by the branches where sharp produces the response, so those never touch `/tmp`. // `/tmp` on Cloud Functions is tmpfs — RAM against the instance limit — so writing the // output there and reading it straight back with `sendFile` held every transformed image - // in memory twice. The passthrough branches below still use `tempFilePath`; converting - // those is the deferred video-streaming work. + // in memory twice. Only the SVG and video-thumbnail branches below still use `tempFilePath`; + // a plain passthrough streams from Storage. let output: Buffer | undefined; + let streamed = false; let filename = `${asset.name}${asset.extension}`; const formatMimeMap: Record = { webp: 'image/webp', jpeg: 'image/jpeg', png: 'image/png', avif: 'image/avif' }; const formatExtMap: Record = { webp: '.webp', jpeg: '.jpg', png: '.png', avif: '.avif' }; @@ -723,7 +725,9 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { overwriteType = format ? formatMimeMap[format] : 'image/webp'; filename = `${asset.name}-${suffix || `w${width}`}-thumbnail${format ? formatExtMap[format] : '.webp'}`; } else { - await assetFile.download({ destination: tempFilePath }); + // Video, audio, PDF and the rest pass through untouched, so they are streamed rather than + // staged in `/tmp` — see the `/download` route for why that matters past 32 MiB. + streamed = true; } const policy = assetResponsePolicy(overwriteType || asset.type, false); res @@ -733,6 +737,8 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { .contentType(policy.contentType); if (output) { res.send(output); + } else if (streamed) { + streamStoredObject(req, res, assetFile, Number(objectMetadata?.['size'] ?? 0), '[V1:AssetById]'); } else { res.sendFile(tempFilePath); } @@ -830,21 +836,15 @@ CDN.get(['/api/v1/spaces/:spaceId/assets/:assetId/original', '/api/v1/spaces/:sp } } - // A distinct path from the transform route's `assets-${assetId}`. That route writes - // *transformed* output there on the video-thumbnail branch, so sharing the path would let a - // concurrent request for the same asset serve the wrong bytes. - // - // `sendFile` rather than a stream because it implements Range requests, and a video here is - // exactly where a client resumes a partial transfer. Streaming is not the fix for the tmpfs - // cost — a redirect to Storage is, because GCS handles Range natively. - const tempFilePath = `${os.tmpdir()}/assets-stored-${assetId}`; - await assetFile.download({ destination: tempFilePath }); - + // Streamed straight from Storage rather than downloaded to `/tmp` and sent with `sendFile`: + // `/tmp` is RAM on Cloud Functions, and `sendFile` always sets `Content-Length`, which makes + // Cloud Run reject anything over 32 MiB — a 100 MB video failed with a bare `500`. The stream + // honours `Range` itself, so a client can still resume a partial transfer. const policy = assetResponsePolicy(asset.type, attachment); res .header('Cache-Control', `public, max-age=${CACHE_ASSET_MAX_AGE}, s-maxage=${CACHE_ASSET_MAX_AGE}`) .header('Content-Disposition', buildContentDisposition(`${asset.name}${asset.extension}`, policy.attachment)) .set(policy.headers) - .contentType(policy.contentType) - .sendFile(tempFilePath); + .contentType(policy.contentType); + streamStoredObject(req, res, assetFile, Number(objectMetadata?.['size'] ?? 0), tag); });