fix(shared): return a value that fails to percent-decode as-is

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',

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'
   }
 

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(

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)
   })
 })

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
   }
 }