diff --git a/packages/core/src/utils.test.ts b/packages/core/src/utils.test.ts index 2b43ae4f..1f74d465 100644 --- a/packages/core/src/utils.test.ts +++ b/packages/core/src/utils.test.ts @@ -14,6 +14,10 @@ beforeEach(() => { vi.clearAllMocks() }) +afterEach(() => { + vi.restoreAllMocks() +}) + describe('generateContentDisposition', () => { it('handle normal filename', () => { expect(generateContentDisposition('test.txt')).toEqual( @@ -197,11 +201,13 @@ it('getFilenameFromContentDisposition', () => { ) expect(getFilenameFromContentDisposition('inline; filename="a\\"')).toEqual(undefined) - // malformed escapes in filename* are kept as-is, the valid ones are still decoded + // a filename* that fails to decode is kept as-is expect( getFilenameFromContentDisposition("attachment; filename*=utf-8''%E4%B8%AD%ZZ.txt"), - ).toEqual('中%ZZ.txt') - expect(getFilenameFromContentDisposition('inline; filename*=%E2%82%AC%.txt')).toEqual('€%.txt') + ).toEqual('%E4%B8%AD%ZZ.txt') + expect(getFilenameFromContentDisposition('inline; filename*=%E2%82%AC%.txt')).toEqual( + '%E2%82%AC%.txt', + ) // quoted filename* is tolerated expect( @@ -405,6 +411,35 @@ describe('resolveStandardBodyHint', () => { ).toBe('none') }) + it('decodes the filename only when no other header decides the hint', () => { + const decodeSpy = vi.spyOn(globalThis, 'decodeURIComponent') + // just under the 16 KiB header limit of Node's HTTP server + const contentDisposition = `attachment; filename*=utf-8''${'%FFa'.repeat(3900)}` + + for (const [headers, hint] of [ + [{ 'content-type': 'application/json' }, 'json'], + [{ 'content-type': 'multipart/form-data; boundary=x' }, 'form-data'], + [{ 'content-type': 'application/x-www-form-urlencoded' }, 'url-search-params'], + [{ 'content-type': 'text/event-stream' }, 'event-stream'], + [{ 'standard-server': 'json' }, 'json'], + [{}, 'none'], + [{ 'content-length': '3' }, 'file'], + ] as const) { + expect( + resolveStandardBodyHint({ ...headers, 'content-disposition': contentDisposition }), + ).toBe(hint) + } + expect(decodeSpy).not.toHaveBeenCalled() + + expect( + resolveStandardBodyHint({ + 'content-type': 'application/pdf', + 'content-disposition': contentDisposition, + }), + ).toBe('file') + expect(decodeSpy).toHaveBeenCalled() + }) + it('octet-stream when content-length is absent', () => { expect(resolveStandardBodyHint({ 'content-type': 'application/octet-stream' })).toBe( 'octet-stream', diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 1e6252c8..b8fbc42b 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -119,11 +119,6 @@ export function resolveStandardBodyHint(headers: { ?.trim() .toLowerCase() const contentLength = flattenStandardHeader(headers['content-length']) - const contentDisposition = flattenStandardHeader(headers['content-disposition']) - const fileName = - contentDisposition !== undefined - ? getFilenameFromContentDisposition(contentDisposition) - : undefined if (mimeType === undefined && (contentLength === undefined || contentLength === '0')) { return 'none' @@ -145,7 +140,16 @@ export function resolveStandardBodyHint(headers: { return 'event-stream' } - if (fileName !== undefined || contentLength !== undefined) { + if (contentLength !== undefined) { + return 'file' + } + + const contentDisposition = flattenStandardHeader(headers['content-disposition']) + + if ( + contentDisposition !== undefined && + getFilenameFromContentDisposition(contentDisposition) !== undefined + ) { return 'file' } diff --git a/packages/peer/src/body.test.ts b/packages/peer/src/body.test.ts index 892b96aa..ee694676 100644 --- a/packages/peer/src/body.test.ts +++ b/packages/peer/src/body.test.ts @@ -6,6 +6,10 @@ import { encodeAtomicStandardBody, toStandardBody } from './body' import type { PeerRequestMessage } from './types' describe('toStandardBody', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + function makeMessage(options: { bodyHint?: StandardBodyHint contentType?: string @@ -158,6 +162,30 @@ describe('toStandardBody', () => { expect(cleanup).toHaveBeenCalledWith({ kind: 'success' }) }) + it('keeps a malformed filename as-is without retrying it once per escape', async () => { + const decodeSpy = vi.spyOn(globalThis, 'decodeURIComponent') + const filename = '%FFa'.repeat(262_144) // 1 MiB, peer messages have no header size limit + const cleanup = vi.fn() + const { resolveBody } = toStandardBody( + makeMessage({ + contentType: 'text/plain', + contentDisposition: `attachment; filename*=utf-8''${filename}`, + binary: new TextEncoder().encode('file content'), + }), + cleanup, + ) + + const body = (await resolveBody()) as File + + expect(body).toBeInstanceOf(File) + expect(body.name).toBe(filename) + expect(await body.text()).toBe('file content') + expect(decodeSpy).toHaveBeenCalledTimes(1) + + expect(cleanup).toHaveBeenCalledTimes(1) + expect(cleanup).toHaveBeenCalledWith({ kind: 'success' }) + }) + it('receives an empty file', async () => { const cleanup = vi.fn() const { resolveBody } = toStandardBody( diff --git a/packages/shared/src/uri.test.ts b/packages/shared/src/uri.test.ts index eb41ca05..a3cc4b5e 100644 --- a/packages/shared/src/uri.test.ts +++ b/packages/shared/src/uri.test.ts @@ -24,22 +24,35 @@ describe('safeEncodeURIComponent', () => { }) describe('safeDecodeURIComponent', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + it('decodes like decodeURIComponent for well-formed input', () => { expect(safeDecodeURIComponent('test')).toBe('test') expect(safeDecodeURIComponent('test%20value')).toBe('test value') expect(safeDecodeURIComponent('%CE%BB%E4%B8%96%E7%95%8C')).toBe('λ世界') + expect(safeDecodeURIComponent('%f0%9f%98%80')).toBe('😀') expect(safeDecodeURIComponent('')).toBe('') }) - it('keeps malformed escapes as-is and still decodes the valid ones instead of throwing', () => { - expect(safeDecodeURIComponent('invalid%20value%')).toBe('invalid value%') - expect(safeDecodeURIComponent('%E0%A4%A')).toBe('%E0%A4%A') // Invalid UTF-8 sequence + it('returns a value that fails to decode as-is instead of throwing', () => { + expect(safeDecodeURIComponent('invalid%20value%')).toBe('invalid%20value%') expect(safeDecodeURIComponent('%ZZ')).toBe('%ZZ') - expect(safeDecodeURIComponent('%ZZ%e4%b8%ad%')).toBe('%ZZ中%') - expect(safeDecodeURIComponent('%FF-%E2%82%AC')).toBe('%FF-€') - // a run of escapes that is not valid UTF-8 is kept as a whole - expect(safeDecodeURIComponent('%E4%B8%AD%FF%')).toBe('%E4%B8%AD%FF%') - // decoded output is not decoded again - expect(safeDecodeURIComponent('%2541%ZZ')).toBe('%41%ZZ') + // malformed UTF-8: truncated, overlong, surrogate, above U+10FFFF + expect(safeDecodeURIComponent('%E4%B8')).toBe('%E4%B8') + expect(safeDecodeURIComponent('%C0%AF')).toBe('%C0%AF') + expect(safeDecodeURIComponent('%ED%A0%80')).toBe('%ED%A0%80') + expect(safeDecodeURIComponent('%F4%90%80%80')).toBe('%F4%90%80%80') + expect(safeDecodeURIComponent('%E4%B8%AD%ZZ')).toBe('%E4%B8%AD%ZZ') + expect(safeDecodeURIComponent('%FF-%E2%82%AC')).toBe('%FF-%E2%82%AC') + }) + + it('tries decodeURIComponent once on hostile input', () => { + const decodeSpy = vi.spyOn(globalThis, 'decodeURIComponent') + const hostile = '%FFa'.repeat(250_000) + + expect(safeDecodeURIComponent(hostile)).toBe(hostile) + expect(decodeSpy).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/shared/src/uri.ts b/packages/shared/src/uri.ts index d16700d2..ba31f308 100644 --- a/packages/shared/src/uri.ts +++ b/packages/shared/src/uri.ts @@ -1,6 +1,5 @@ // with the u flag a well-formed surrogate pair is one code point, so only lone surrogates match const LONE_SURROGATE_REGEX = /[\uD800-\uDFFF]/gu -const PERCENT_ESCAPES_REGEX = /(?:%[0-9A-F]{2})+/gi /** * `encodeURIComponent` that never throws, lone surrogates become U+FFFD. @@ -16,7 +15,7 @@ export function safeEncodeURIComponent(value: string): string { } /** - * `decodeURIComponent` that never throws, runs of `%XX` escapes that fail to decode are kept as-is. + * `decodeURIComponent` that never throws, a value that fails to decode is returned as-is. */ export function safeDecodeURIComponent(value: string): string { if (!value.includes('%')) { @@ -27,13 +26,6 @@ export function safeDecodeURIComponent(value: string): string { // oxlint-disable-next-line no-restricted-globals return decodeURIComponent(value) } catch { - return value.replace(PERCENT_ESCAPES_REGEX, (escapes) => { - try { - // oxlint-disable-next-line no-restricted-globals - return decodeURIComponent(escapes) - } catch { - return escapes - } - }) + return value } }