From e819d2ae29b29f433e3fa4c07af12468bbf3f4b0 Mon Sep 17 00:00:00 2001 From: Mickael Date: Wed, 23 Sep 2026 10:27:52 +0100 Subject: [PATCH 1/3] fix(files): HEAD-safe attachment getHandler Rebase Extracadabra #3362 onto current main so CRITICAL published-CLI contract picks up request_actor_email_adress. Keep HEAD responses body-less on R2/workerd attachment reads. --- supabase/functions/_backend/files/files.ts | 130 ++++++++- tests/files-head-read.unit.test.ts | 292 +++++++++++++++++++++ 2 files changed, 415 insertions(+), 7 deletions(-) create mode 100644 tests/files-head-read.unit.test.ts diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 91dbdcc7ed..98101628b4 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -266,6 +266,18 @@ function withFileReadCacheControl(cacheControl: string | null | undefined): stri return withNoTransformCacheControl(cacheControl) } +function isHeadRequest(c: Context): boolean { + return c.req.raw.method === 'HEAD' +} + +function toHeadersOnlyResponse(response: Response): Response { + return new Response(null, { + headers: response.headers, + status: response.status, + statusText: response.statusText, + }) +} + function ensureNoTransformResponse(response: Response): Response { const cacheControl = withFileReadCacheControl(response.headers.get('cache-control')) if (cacheControl === response.headers.get('cache-control')) { @@ -281,12 +293,12 @@ function ensureNoTransformResponse(response: Response): Response { }) } -function withAttachmentResponseHeaders(response: Response, fileId: string): Response { +function withAttachmentResponseHeaders(response: Response, fileId: string, includeBody = true): Response { const headers = new Headers(response.headers) headers.set('cache-control', withFileReadCacheControl(headers.get('cache-control'))) headers.set('content-disposition', `attachment; filename="${fileId}"`) - return new Response(response.body, { + return new Response(includeBody ? response.body : null, { headers, status: response.status, statusText: response.statusText, @@ -401,11 +413,12 @@ async function getSupabaseStorageResponse(c: Context, fileId: string): Promise { const fileId = c.get('fileId') + const isHead = isHeadRequest(c) // File reads stay off the primary DB. A deleted version may still be in the // edge cache after R2 trash; check the deleted marker or one indexed r2_path // lookup before serving or restoring that cache entry. @@ -437,7 +450,7 @@ async function getHandler(c: Context): Promise { const cachedResponse = ensureNoTransformResponse(response) response = cachedResponse cloudlog({ requestId: c.get('requestId'), message: 'getHandler files cache hit' }) - if (c.req.raw.method !== 'HEAD') { + if (!isHead) { await saveBandwidthUsage(c, getTransferredBytesFromResponse(cachedResponse)) } // Best-effort restore: if a live file is cached but missing in R2, write it back. @@ -468,7 +481,7 @@ async function getHandler(c: Context): Promise { cloudlog({ requestId: c.get('requestId'), message: 'Failed to restore cached file to R2', fileId, error: String(err) }) } }) - return cachedResponse + return isHead ? toHeadersOnlyResponse(cachedResponse) : cachedResponse } if (await isAttachmentVersionDeleted(c, fileId)) { @@ -477,7 +490,7 @@ async function getHandler(c: Context): Promise { } const rangeHeaderFromRequest = c.req.header('range') - if (rangeHeaderFromRequest) { + if (rangeHeaderFromRequest && !isHead) { cloudlog({ requestId: c.get('requestId'), message: 'getHandler files range request', range: rangeHeaderFromRequest }) try { const retryBucket = new RetryBucket(bucket, DEFAULT_RETRY_PARAMS) @@ -490,7 +503,7 @@ async function getHandler(c: Context): Promise { if (rangeStart >= fileSize) { const emptyHeaders = new Headers() emptyHeaders.set('Content-Range', `bytes */${fileSize}`) - return new Response(new Uint8Array(0), { status: 206, headers: emptyHeaders }) + return new Response(isHead ? null : new Uint8Array(0), { status: 206, headers: emptyHeaders }) } } } @@ -500,6 +513,41 @@ async function getHandler(c: Context): Promise { } } + if (isHead) { + // HEAD must use R2 metadata only (head()), never bucket.get() — streaming a + // multi-MB body and stripping it breaks HTTP/2 Content-Length on Workers. + let objectInfo: R2Object | null = null + try { + objectInfo = await headFirstExistingAttachmentCandidate(new RetryBucket(bucket, DEFAULT_RETRY_PARAMS), candidateKeys) + } + catch (error) { + cloudlogErr({ requestId: c.get('requestId'), message: 'getHandler files head failed', fileId, error }) + throw quickError(503, 'upstream_unavailable', 'File storage temporarily unavailable', { fileId }, error, { alert: false }) + } + + if (objectInfo == null) { + cloudlog({ requestId: c.get('requestId'), message: 'getHandler files object is null' }) + return c.json({ error: 'not_found', message: 'Not found' }, 404) + } + + const headers = objectHeaders(objectInfo) + headers.set('Content-Disposition', `attachment; filename="${objectInfo.key}"`) + + if (rangeHeaderFromRequest) { + const parsedRange = parseAttachmentByteRange(rangeHeaderFromRequest, objectInfo.size) + if (parsedRange.kind === 'invalid') { + return buildInvalidAttachmentRangeResponse(objectInfo.size, false) + } + + headers.set('content-length', parsedRange.bytesTransferred.toString()) + headers.set('content-range', `bytes ${parsedRange.start}-${parsedRange.end}/${objectInfo.size}`) + return new Response(null, { headers, status: 206 }) + } + + headers.set('content-length', objectInfo.size.toString()) + return new Response(null, { status: 200, headers }) + } + let object: R2ObjectBody | null = null try { for (const candidateKey of candidateKeys) { @@ -592,6 +640,74 @@ export function calculateBytesTransferred(objLen: number, r2Range: R2Range | und return isPositiveFiniteNumber(bytesTransferred) ? bytesTransferred : objLen } +type ParsedAttachmentByteRange = + | { kind: 'partial', start: number, end: number, bytesTransferred: number } + | { kind: 'invalid' } + +export function parseAttachmentByteRange(rangeHeader: string, fileSize: number): ParsedAttachmentByteRange { + if (!isPositiveFiniteNumber(fileSize)) { + return { kind: 'invalid' } + } + + const match = /^bytes=(\d*)-(\d*)$/i.exec(rangeHeader.trim()) + if (!match) { + return { kind: 'invalid' } + } + + const startRaw = match[1] + const endRaw = match[2] + + if (startRaw === '' && endRaw !== '') { + const suffixLength = Number.parseInt(endRaw, 10) + if (!Number.isFinite(suffixLength) || suffixLength <= 0) { + return { kind: 'invalid' } + } + + if (suffixLength >= fileSize) { + return { kind: 'partial', start: 0, end: fileSize - 1, bytesTransferred: fileSize } + } + + const start = fileSize - suffixLength + return { kind: 'partial', start, end: fileSize - 1, bytesTransferred: suffixLength } + } + + if (startRaw === '') { + return { kind: 'invalid' } + } + + const rangeStart = Number.parseInt(startRaw, 10) + if (!Number.isFinite(rangeStart) || rangeStart < 0) { + return { kind: 'invalid' } + } + + if (rangeStart >= fileSize) { + return { kind: 'invalid' } + } + + const rangeEnd = endRaw === '' ? fileSize - 1 : Number.parseInt(endRaw, 10) + if (!Number.isFinite(rangeEnd) || rangeEnd < 0) { + return { kind: 'invalid' } + } + + const boundedEnd = Math.min(rangeEnd, fileSize - 1) + if (boundedEnd < rangeStart) { + return { kind: 'invalid' } + } + + return { + kind: 'partial', + start: rangeStart, + end: boundedEnd, + bytesTransferred: boundedEnd - rangeStart + 1, + } +} + +function buildInvalidAttachmentRangeResponse(fileSize: number, includeBody: boolean): Response { + const headers = new Headers() + headers.set('Content-Range', `bytes */${fileSize}`) + return new Response(includeBody ? new Uint8Array(0) : null, { status: 416, headers }) +} + function optionsHandler(c: Context) { cloudlog({ requestId: c.get('requestId'), message: 'optionsHandler files optionsHandler' }) return c.newResponse(null, 204, { diff --git a/tests/files-head-read.unit.test.ts b/tests/files-head-read.unit.test.ts new file mode 100644 index 0000000000..4649ebb8c7 --- /dev/null +++ b/tests/files-head-read.unit.test.ts @@ -0,0 +1,292 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const retryGetMock = vi.fn() +const retryHeadMock = vi.fn() +const createStatsBandwidthMock = vi.fn() +const queryMock = vi.fn() +const closeClientMock = vi.fn() +const getPgClientMock = vi.fn(() => ({ query: queryMock })) + +vi.mock('hono/adapter', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getRuntimeKey: () => 'workerd', + } +}) + +vi.mock('../supabase/functions/_backend/utils/discord.ts', () => ({ + sendDiscordAlert500: () => Promise.resolve(), + sendDiscordAlert: () => Promise.resolve(), +})) + +vi.mock('../supabase/functions/_backend/utils/pg.ts', () => ({ + closeClient: closeClientMock, + getAppOwnerPostgres: vi.fn(), + getDatabaseURL: vi.fn(() => 'postgres://test'), + getDrizzleClient: vi.fn(() => ({})), + getPgClient: getPgClientMock, +})) + +vi.mock('../supabase/functions/_backend/files/retry.ts', () => ({ + DEFAULT_RETRY_PARAMS: {}, + RetryBucket: class RetryBucketMock { + constructor() { } + get() { + return retryGetMock() + } + + head() { + return retryHeadMock() + } + }, +})) + +vi.mock('../supabase/functions/_backend/utils/stats.ts', () => ({ + createStatsBandwidth: createStatsBandwidthMock, +})) + +function createR2Object(size: number, range?: R2Range): R2ObjectBody { + return { + body: new Response('bundle bytes'.repeat(Math.ceil(size / 12))).body, + checksums: {}, + customMetadata: {}, + etag: 'etag', + httpEtag: '"etag"', + httpMetadata: {}, + key: 'orgs/test-org/apps/com.test.app/bundle.zip', + range, + size, + uploaded: new Date(), + version: 'version', + writeHttpMetadata(headers: Headers) { + headers.set('content-type', 'application/zip') + }, + } as unknown as R2ObjectBody +} + +function createR2HeadObject(size: number): R2Object { + return { + checksums: {}, + customMetadata: {}, + etag: 'etag', + httpEtag: '"etag"', + httpMetadata: {}, + key: 'orgs/test-org/apps/com.test.app/bundle.zip', + size, + uploaded: new Date(), + version: 'version', + writeHttpMetadata(headers: Headers) { + headers.set('content-type', 'application/zip') + }, + } as unknown as R2Object +} + +async function createFilesApp(routePrefix = '/files') { + const { app: files } = await import('../supabase/functions/_backend/files/files.ts') + const { createAllCatch, createHono } = await import('../supabase/functions/_backend/utils/hono.ts') + const { version } = await import('../supabase/functions/_backend/utils/version.ts') + + const appGlobal = createHono('files', version) + appGlobal.route(routePrefix, files) + createAllCatch(appGlobal, 'files') + return appGlobal +} + +const filePath = 'orgs/test-org/apps/com.test.app/bundle.zip' +const readUrl = `http://localhost/files/read/attachments/${filePath}?device_id=device-1` +const objectSize = 1_000 + +async function fetchHead(appGlobal: Awaited>, range?: string) { + const headers = range ? { range } : undefined + return appGlobal.fetch( + new Request(readUrl, { method: 'HEAD', headers }), + { ATTACHMENT_BUCKET: {} }, + { waitUntil: () => { } } as any, + ) +} + +describe('files attachment HEAD reads on workerd/R2', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + queryMock.mockResolvedValue({ rows: [] }) + globalThis.caches = { + default: { + match: async (request: Request) => { + if (new URL(request.url).pathname.startsWith('/deleted/')) + return null + return null + }, + put: async () => { }, + }, + } as any + }) + + it('returns headers-only 200 with Content-Length on R2 HEAD miss', async () => { + retryHeadMock.mockResolvedValue(createR2HeadObject(3_478_395)) + retryGetMock.mockResolvedValue(null) + const appGlobal = await createFilesApp() + + const response = await fetchHead(appGlobal) + + expect(response.status).toBe(200) + expect(response.headers.get('content-length')).toBe('3478395') + expect(response.headers.get('content-type')).toBe('application/zip') + expect(response.headers.get('content-disposition')).toBe(`attachment; filename="${filePath}"`) + expect(await response.text()).toBe('') + expect((await response.arrayBuffer()).byteLength).toBe(0) + expect(retryGetMock).not.toHaveBeenCalled() + expect(createStatsBandwidthMock).not.toHaveBeenCalled() + }) + + it('returns 404 when R2 head finds no object', async () => { + retryHeadMock.mockResolvedValue(null) + const appGlobal = await createFilesApp() + + const response = await fetchHead(appGlobal) + + expect(response.status).toBe(404) + expect(retryGetMock).not.toHaveBeenCalled() + }) + + it('returns 503 when R2 head fails', async () => { + retryHeadMock.mockRejectedValue(new Error('r2 unavailable')) + const appGlobal = await createFilesApp() + + const response = await fetchHead(appGlobal) + + expect(response.status).toBe(503) + expect(retryGetMock).not.toHaveBeenCalled() + }) + + it('returns 206 for bounded, open-ended, and suffix HEAD ranges', async () => { + retryHeadMock.mockResolvedValue(createR2HeadObject(objectSize)) + const appGlobal = await createFilesApp() + + const bounded = await fetchHead(appGlobal, 'bytes=0-99') + expect(bounded.status).toBe(206) + expect(bounded.headers.get('content-length')).toBe('100') + expect(bounded.headers.get('content-range')).toBe(`bytes 0-99/${objectSize}`) + expect((await bounded.arrayBuffer()).byteLength).toBe(0) + + const openEnded = await fetchHead(appGlobal, 'bytes=100-') + expect(openEnded.status).toBe(206) + expect(openEnded.headers.get('content-length')).toBe('900') + expect(openEnded.headers.get('content-range')).toBe(`bytes 100-999/${objectSize}`) + + const suffix = await fetchHead(appGlobal, 'bytes=-500') + expect(suffix.status).toBe(206) + expect(suffix.headers.get('content-length')).toBe('500') + expect(suffix.headers.get('content-range')).toBe(`bytes 500-999/${objectSize}`) + }) + + it('returns 416 for reversed and unsatisfiable HEAD ranges', async () => { + retryHeadMock.mockResolvedValue(createR2HeadObject(objectSize)) + const appGlobal = await createFilesApp() + + const reversed = await fetchHead(appGlobal, 'bytes=10-5') + expect(reversed.status).toBe(416) + expect(reversed.headers.get('content-range')).toBe(`bytes */${objectSize}`) + expect((await reversed.arrayBuffer()).byteLength).toBe(0) + + const unsatisfiable = await fetchHead(appGlobal, 'bytes=1000-') + expect(unsatisfiable.status).toBe(416) + expect(unsatisfiable.headers.get('content-range')).toBe(`bytes */${objectSize}`) + }) + + it('parses attachment byte ranges for suffix and invalid inputs', async () => { + const { parseAttachmentByteRange } = await import('../supabase/functions/_backend/files/files.ts') + + expect(parseAttachmentByteRange('bytes=-500', objectSize)).toEqual({ + kind: 'partial', + start: 500, + end: 999, + bytesTransferred: 500, + }) + expect(parseAttachmentByteRange('bytes=10-5', objectSize)).toEqual({ kind: 'invalid' }) + expect(parseAttachmentByteRange('bytes=1000-', objectSize)).toEqual({ kind: 'invalid' }) + }) + + it('returns headers-only 200 with Content-Length on cache hit', async () => { + globalThis.caches = { + default: { + match: async (request: Request) => { + if (new URL(request.url).pathname.startsWith('/deleted/')) + return null + return new Response('cached zip bytes', { + headers: { + 'cache-control': 'public, max-age=3600', + 'content-length': '3478395', + 'content-type': 'application/zip', + 'content-disposition': `attachment; filename="${filePath}"`, + }, + }) + }, + put: async () => { }, + }, + } as any + retryHeadMock.mockResolvedValue(createR2HeadObject(3_478_395)) + const appGlobal = await createFilesApp() + + const response = await appGlobal.fetch( + new Request(readUrl, { method: 'HEAD' }), + { ATTACHMENT_BUCKET: {} }, + { waitUntil: () => { } } as any, + ) + + expect(response.status).toBe(200) + expect(response.headers.get('content-length')).toBe('3478395') + expect(await response.text()).toBe('') + expect((await response.arrayBuffer()).byteLength).toBe(0) + expect(createStatsBandwidthMock).not.toHaveBeenCalled() + }) + + it('still returns body bytes for GET on R2 miss', async () => { + retryHeadMock.mockResolvedValue(createR2HeadObject(12)) + retryGetMock.mockResolvedValue(createR2Object(12)) + const appGlobal = await createFilesApp() + + const response = await appGlobal.fetch( + new Request(readUrl), + { ATTACHMENT_BUCKET: {} }, + { waitUntil: () => { } } as any, + ) + + expect(response.status).toBe(200) + expect(response.headers.get('content-length')).toBe('12') + expect((await response.text()).length).toBeGreaterThan(0) + expect(createStatsBandwidthMock).toHaveBeenCalledWith( + expect.anything(), + 'device-1', + 'com.test.app', + 12, + ) + }) + + it('returns 404 for deleted versions on HEAD and GET', async () => { + queryMock.mockResolvedValue({ + rows: [{ deleted: true, deleted_at: '2026-08-16T00:00:00Z' }], + }) + retryHeadMock.mockResolvedValue(createR2HeadObject(100)) + retryGetMock.mockResolvedValue(createR2Object(100)) + const appGlobal = await createFilesApp() + + const headResponse = await appGlobal.fetch( + new Request(readUrl, { method: 'HEAD' }), + { ATTACHMENT_BUCKET: {} }, + { waitUntil: () => { } } as any, + ) + const getResponse = await appGlobal.fetch( + new Request(readUrl), + { ATTACHMENT_BUCKET: {} }, + { waitUntil: () => { } } as any, + ) + + expect(headResponse.status).toBe(404) + expect(getResponse.status).toBe(404) + expect(await getResponse.json()).toMatchObject({ error: 'not_found' }) + expect(retryGetMock).not.toHaveBeenCalled() + expect(retryHeadMock).not.toHaveBeenCalled() + }) +}) From 087e609076a24ae4052260bd17bbb536d5f3cd41 Mon Sep 17 00:00:00 2001 From: Mickael Date: Wed, 23 Sep 2026 10:39:49 +0100 Subject: [PATCH 2/3] fix(files): keep HEAD attachment 404s body-less CR: miss/deleted/bucket-null paths still used c.json on HEAD, which puts a body + Content-Length on the same HTTP/2 trap as 200 hits. Return null-body 404 for HEAD; keep JSON 404 for GET. --- supabase/functions/_backend/files/files.ts | 21 ++++++++++++++------- tests/files-head-read.unit.test.ts | 2 ++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 98101628b4..9d867f71ac 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -278,6 +278,13 @@ function toHeadersOnlyResponse(response: Response): Response { }) } +function notFoundAttachmentResponse(c: Context, isHead: boolean): Response { + // HEAD must stay body-less (same HTTP/2 Content-Length trap as 200 hits). + if (isHead) + return new Response(null, { status: 404 }) + return c.json({ error: 'not_found', message: 'Not found' }, 404) +} + function ensureNoTransformResponse(response: Response): Response { const cacheControl = withFileReadCacheControl(response.headers.get('cache-control')) if (cacheControl === response.headers.get('cache-control')) { @@ -370,7 +377,7 @@ async function getSupabaseStorageResponse(c: Context, fileId: string): Promise { if (bucket == null) { cloudlog({ requestId: c.get('requestId'), message: 'getHandler files bucket is null' }) - return c.json({ error: 'not_found', message: 'Not found' }, 404) + return notFoundAttachmentResponse(c, isHead) } const cache = await getFileReadCache() @@ -444,7 +451,7 @@ async function getHandler(c: Context): Promise { if (response != null) { if (await isAttachmentVersionDeleted(c, fileId)) { cloudlog({ requestId: c.get('requestId'), message: 'getHandler files cache hit for deleted version', fileId }) - return c.json({ error: 'not_found', message: 'Not found' }, 404) + return notFoundAttachmentResponse(c, isHead) } const cachedResponse = ensureNoTransformResponse(response) @@ -486,7 +493,7 @@ async function getHandler(c: Context): Promise { if (await isAttachmentVersionDeleted(c, fileId)) { cloudlog({ requestId: c.get('requestId'), message: 'getHandler files cache miss for deleted version', fileId }) - return c.json({ error: 'not_found', message: 'Not found' }, 404) + return notFoundAttachmentResponse(c, isHead) } const rangeHeaderFromRequest = c.req.header('range') @@ -527,7 +534,7 @@ async function getHandler(c: Context): Promise { if (objectInfo == null) { cloudlog({ requestId: c.get('requestId'), message: 'getHandler files object is null' }) - return c.json({ error: 'not_found', message: 'Not found' }, 404) + return notFoundAttachmentResponse(c, isHead) } const headers = objectHeaders(objectInfo) @@ -564,7 +571,7 @@ async function getHandler(c: Context): Promise { } if (object == null) { cloudlog({ requestId: c.get('requestId'), message: 'getHandler files object is null' }) - return c.json({ error: 'not_found', message: 'Not found' }, 404) + return notFoundAttachmentResponse(c, isHead) } const bytesTransferred = calculateBytesTransferred(object.size, object.range) await saveBandwidthUsage(c, bytesTransferred) diff --git a/tests/files-head-read.unit.test.ts b/tests/files-head-read.unit.test.ts index 4649ebb8c7..f1d3e9ae51 100644 --- a/tests/files-head-read.unit.test.ts +++ b/tests/files-head-read.unit.test.ts @@ -147,6 +147,7 @@ describe('files attachment HEAD reads on workerd/R2', () => { const response = await fetchHead(appGlobal) expect(response.status).toBe(404) + expect((await response.arrayBuffer()).byteLength).toBe(0) expect(retryGetMock).not.toHaveBeenCalled() }) @@ -284,6 +285,7 @@ describe('files attachment HEAD reads on workerd/R2', () => { ) expect(headResponse.status).toBe(404) + expect((await headResponse.arrayBuffer()).byteLength).toBe(0) expect(getResponse.status).toBe(404) expect(await getResponse.json()).toMatchObject({ error: 'not_found' }) expect(retryGetMock).not.toHaveBeenCalled() From 1d4b4669cf0d0a785e084782817d9c9595695d29 Mon Sep 17 00:00:00 2001 From: Mickael Date: Wed, 23 Sep 2026 10:46:56 +0100 Subject: [PATCH 3/3] fix(files): declare HEAD method before supabase 404 path typecheck: method was referenced in the signed-URL 404 branch before its const declaration. --- supabase/functions/_backend/files/files.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 9d867f71ac..b25967e333 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -367,6 +367,7 @@ async function saveBandwidthUsage(c: Context, fileSize: number | null | undefine } async function getSupabaseStorageResponse(c: Context, fileId: string): Promise { + const method = c.req.raw.method === 'HEAD' ? 'HEAD' : 'GET' const { data: signedUrlData, error: signedUrlError } = await supabaseAdmin(c).storage.from('capgo').createSignedUrl(fileId, 60) if (signedUrlError || !signedUrlData?.signedUrl) { @@ -384,7 +385,6 @@ async function getSupabaseStorageResponse(c: Context, fileId: string): Promise