fix(documents): RFC 5987 Content-Disposition so NFD filenames stop crashing inline view (#964)
* fix(documents): RFC 5987 Content-Disposition so NFD filenames stop crashing inline view macOS/iOS uploads carry NFD-decomposed filenames (base letter + combining diaeresis U+0308, char code 776). undici Headers require ByteString values (every code unit <= 0xFF), so splicing the raw filename into the Content-Disposition header threw while building the response and the inline document route 500ed. 122 prod documents across 35 companies hit this; last crash 2026-07-09T16:17. Add lib/api/content-disposition.ts emitting the RFC 6266 dual form: an ASCII quoted fallback (NFC-normalize, then replace anything outside printable ASCII plus quote and backslash with _) and filename*=UTF-8''<percent-encoded> per RFC 5987 (encodeURIComponent on the NFC name, additionally escaping ! ' ( ) * which it leaves bare). Use it in the inline document route and in the two latent same-shape sites that embed raw employee names in payslip PDF headers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(api): sanitize lone surrogates before percent-encoding Content-Disposition (CodeRabbit) Unpaired UTF-16 surrogates survive normalize('NFC') and make encodeURIComponent throw a URIError, so replace them with U+FFFD via String.prototype.toWellFormed() before encoding so the helper always returns a valid header value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
5b505bb4a7
commit
b4a21b1029
@@ -0,0 +1,108 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import {
|
||||
parseJsonResponse,
|
||||
createMockRouteParams,
|
||||
createQueuedMockSupabase,
|
||||
} from '@/tests/helpers'
|
||||
|
||||
const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
|
||||
const requireAuthMock = vi.fn()
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
requireAuth: (...args: unknown[]) => requireAuthMock(...args),
|
||||
}))
|
||||
|
||||
const downloadMock = vi.fn()
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
createServiceClient: () => ({
|
||||
storage: {
|
||||
from: () => ({ download: downloadMock }),
|
||||
},
|
||||
}),
|
||||
}))
|
||||
|
||||
import { GET } from '../route'
|
||||
import { NextResponse } from 'next/server'
|
||||
|
||||
const mockUser = { id: 'user-1', email: 'test@test.se' }
|
||||
|
||||
// NFD filename as macOS/iOS uploads produce them: o + combining diaeresis
|
||||
// U+0308 (char code 776). This is the exact shape that made the raw header
|
||||
// build throw in prod (undici Headers require code units <= 0xFF).
|
||||
const NFD_FILE_NAME = 'kvitto fo\u0308rvaring.pdf'
|
||||
|
||||
function makeDoc(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: 'doc-1',
|
||||
company_id: 'company-1',
|
||||
file_name: NFD_FILE_NAME,
|
||||
mime_type: 'application/pdf',
|
||||
storage_path: 'documents/user-1/doc-1.pdf',
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function makeReq() {
|
||||
return new Request('http://localhost/api/documents/doc-1/inline')
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
requireAuthMock.mockResolvedValue({ user: mockUser, supabase: mockSupabase, error: null })
|
||||
downloadMock.mockResolvedValue({ data: new Blob(['%PDF-1.4']), error: null })
|
||||
})
|
||||
|
||||
describe('GET /api/documents/[id]/inline', () => {
|
||||
it('returns 401 when not authenticated', async () => {
|
||||
requireAuthMock.mockResolvedValue({
|
||||
user: null,
|
||||
supabase: mockSupabase,
|
||||
error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
|
||||
})
|
||||
const res = await GET(makeReq(), createMockRouteParams({ id: 'doc-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns 404 when the document is not found', async () => {
|
||||
enqueue({ data: null, error: { message: 'not found' } }) // doc lookup
|
||||
const res = await GET(makeReq(), createMockRouteParams({ id: 'doc-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: string }>(res)
|
||||
expect(status).toBe(404)
|
||||
expect(body.error).toBe('Document not found')
|
||||
})
|
||||
|
||||
it('returns 404 when the user is not a member of the document company', async () => {
|
||||
enqueue({ data: makeDoc(), error: null }) // doc lookup
|
||||
enqueue({ data: null, error: null }) // membership lookup
|
||||
const res = await GET(makeReq(), createMockRouteParams({ id: 'doc-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(404)
|
||||
})
|
||||
|
||||
it('returns 500 when the storage download fails', async () => {
|
||||
enqueue({ data: makeDoc(), error: null })
|
||||
enqueue({ data: { company_id: 'company-1' }, error: null })
|
||||
downloadMock.mockResolvedValue({ data: null, error: { message: 'boom' } })
|
||||
const res = await GET(makeReq(), createMockRouteParams({ id: 'doc-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(500)
|
||||
})
|
||||
|
||||
it('streams the file with an RFC 5987 Content-Disposition for an NFD filename', async () => {
|
||||
enqueue({ data: makeDoc(), error: null })
|
||||
enqueue({ data: { company_id: 'company-1' }, error: null })
|
||||
|
||||
const res = await GET(makeReq(), createMockRouteParams({ id: 'doc-1' }))
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
const disposition = res.headers.get('Content-Disposition') ?? ''
|
||||
expect(disposition).toContain('inline')
|
||||
// Extended form carries the NFC-composed UTF-8 percent-encoded name.
|
||||
expect(disposition).toContain(`filename*=UTF-8''kvitto%20f%C3%B6rvaring.pdf`)
|
||||
// ASCII fallback replaces the non-ASCII character.
|
||||
expect(disposition).toContain('filename="kvitto f_rvaring.pdf"')
|
||||
expect(res.headers.get('Content-Type')).toBe('application/pdf')
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,7 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { requireAuth } from '@/lib/auth/require-auth'
|
||||
import { createServiceClient } from '@/lib/supabase/server'
|
||||
import { contentDisposition } from '@/lib/api/content-disposition'
|
||||
|
||||
/**
|
||||
* GET /api/documents/:id/inline
|
||||
@@ -87,13 +88,14 @@ export async function GET(
|
||||
)
|
||||
}
|
||||
|
||||
const safeFileName = doc.file_name.replace(/[\r\n"]/g, '_')
|
||||
|
||||
return new NextResponse(blob, {
|
||||
status: 200,
|
||||
headers: {
|
||||
'Content-Type': resolveContentType(doc.file_name, doc.mime_type),
|
||||
'Content-Disposition': `inline; filename="${safeFileName}"`,
|
||||
// RFC 5987 dual form: NFD filenames from macOS/iOS uploads contain
|
||||
// combining marks (> 0xFF), which undici Headers reject as non-
|
||||
// ByteString values; splicing the raw name here 500ed the route.
|
||||
'Content-Disposition': contentDisposition('inline', doc.file_name),
|
||||
'Cache-Control': 'private, max-age=300',
|
||||
// Block MIME sniffing: Content-Type is derived from DB metadata
|
||||
// (with extension fallback for legacy rows), never from response
|
||||
|
||||
@@ -4,6 +4,7 @@ import { createServiceClientNoCookies } from '@/lib/auth/api-keys'
|
||||
import { resolvePayslipToken, isValidPayslipTokenFormat } from '@/lib/salary/payslips/links'
|
||||
import { buildPayslipData, payslipFileName } from '@/lib/salary/payslips/build-payslip-data'
|
||||
import { PayslipPDF } from '@/lib/salary/pdf/payslip-template'
|
||||
import { contentDisposition } from '@/lib/api/content-disposition'
|
||||
|
||||
// In-memory rate limiting per token (pattern from /api/calendar/feed).
|
||||
const rateLimitMap = new Map<string, { count: number; resetAt: number }>()
|
||||
@@ -118,7 +119,10 @@ export async function GET(
|
||||
return new Response(buffer as unknown as BodyInit, {
|
||||
headers: {
|
||||
'Content-Type': 'application/pdf',
|
||||
'Content-Disposition': `attachment; filename="${fileName}"`,
|
||||
// RFC 5987 dual form: employee names with non-Latin-1 characters
|
||||
// (e.g. NFD combining marks) would otherwise make undici reject
|
||||
// the header value and crash the response.
|
||||
'Content-Disposition': contentDisposition('attachment', fileName),
|
||||
'Cache-Control': 'no-store',
|
||||
},
|
||||
})
|
||||
|
||||
@@ -5,6 +5,7 @@ import { getCompanyDisplayName } from '@/lib/company/context'
|
||||
import { renderToBuffer } from '@react-pdf/renderer'
|
||||
import { PayslipPDF } from '@/lib/salary/pdf/payslip-template'
|
||||
import { buildPayslipData, payslipFileName } from '@/lib/salary/payslips/build-payslip-data'
|
||||
import { contentDisposition } from '@/lib/api/content-disposition'
|
||||
|
||||
ensureInitialized()
|
||||
|
||||
@@ -81,7 +82,10 @@ export const GET = withRouteContext<{ params: Promise<{ id: string; employeeId:
|
||||
return new Response(buffer as unknown as BodyInit, {
|
||||
headers: {
|
||||
'Content-Type': 'application/pdf',
|
||||
'Content-Disposition': `inline; filename="${fileName}"`,
|
||||
// RFC 5987 dual form: employee names with non-Latin-1 characters
|
||||
// (e.g. NFD combining marks) would otherwise make undici reject
|
||||
// the header value and crash the response.
|
||||
'Content-Disposition': contentDisposition('inline', fileName),
|
||||
},
|
||||
})
|
||||
},
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { contentDisposition } from '../content-disposition'
|
||||
|
||||
describe('contentDisposition', () => {
|
||||
it('passes a plain ASCII filename through unchanged in both forms', () => {
|
||||
expect(contentDisposition('inline', 'kvitto.pdf')).toBe(
|
||||
`inline; filename="kvitto.pdf"; filename*=UTF-8''kvitto.pdf`,
|
||||
)
|
||||
})
|
||||
|
||||
it('supports the attachment type', () => {
|
||||
expect(contentDisposition('attachment', 'report.pdf')).toBe(
|
||||
`attachment; filename="report.pdf"; filename*=UTF-8''report.pdf`,
|
||||
)
|
||||
})
|
||||
|
||||
it('produces a ByteString-safe value for an NFD Swedish filename', () => {
|
||||
// macOS/iOS NFD upload: base letter + combining diaeresis U+0308 (776),
|
||||
// the exact shape that crashed the inline route in prod.
|
||||
const nfd = 'kvitto fo\u0308rvaring.pdf'
|
||||
expect(nfd.charCodeAt(9)).toBe(776)
|
||||
|
||||
const header = contentDisposition('inline', nfd)
|
||||
|
||||
const maxCode = Math.max(...[...header].map((c) => c.charCodeAt(0)))
|
||||
expect(maxCode).toBeLessThanOrEqual(255)
|
||||
// The whole header is in fact printable ASCII.
|
||||
expect(/^[\x20-\x7e]*$/.test(header)).toBe(true)
|
||||
// filename* carries the NFC-composed UTF-8 percent-encoding of the name.
|
||||
expect(header).toContain(`filename*=UTF-8''kvitto%20f%C3%B6rvaring.pdf`)
|
||||
// The quoted fallback replaces the non-ASCII character with _.
|
||||
expect(header).toContain('filename="kvitto f_rvaring.pdf"')
|
||||
})
|
||||
|
||||
it('is stable across NFD and NFC inputs of the same name', () => {
|
||||
const nfd = 'lo\u0308n.pdf' // o + combining diaeresis
|
||||
const nfc = 'l\u00f6n.pdf' // precomposed \u00f6
|
||||
expect(contentDisposition('inline', nfd)).toBe(contentDisposition('inline', nfc))
|
||||
})
|
||||
|
||||
it('does not throw when used as an undici Headers value', () => {
|
||||
const nfd = 'lo\u0308nespec_a\u030agren.pdf'
|
||||
expect(
|
||||
() => new Headers({ 'Content-Disposition': contentDisposition('inline', nfd) }),
|
||||
).not.toThrow()
|
||||
})
|
||||
|
||||
it('neutralizes quote and CRLF header injection', () => {
|
||||
const header = contentDisposition('attachment', 'evil"\r\nSet-Cookie: x=y.pdf')
|
||||
expect(header).not.toContain('\r')
|
||||
expect(header).not.toContain('\n')
|
||||
expect(header).toContain('filename="evil___Set-Cookie: x=y.pdf"')
|
||||
// The extended form percent-encodes them instead of emitting them raw.
|
||||
expect(header).toContain('%0D%0A')
|
||||
})
|
||||
|
||||
it('replaces backslash in the quoted fallback', () => {
|
||||
expect(contentDisposition('inline', 'a\\b.pdf')).toContain('filename="a_b.pdf"')
|
||||
})
|
||||
|
||||
it(`percent-escapes ! ' ( ) * which encodeURIComponent leaves bare`, () => {
|
||||
const header = contentDisposition('inline', "a!'()*.pdf")
|
||||
expect(header).toContain(`filename*=UTF-8''a%21%27%28%29%2A.pdf`)
|
||||
})
|
||||
|
||||
it('sanitizes a lone high surrogate instead of throwing', () => {
|
||||
let header = ''
|
||||
expect(() => {
|
||||
header = contentDisposition('attachment', '\uD800')
|
||||
}).not.toThrow()
|
||||
// The lone surrogate becomes U+FFFD: _ in the fallback, percent-encoded
|
||||
// UTF-8 (%EF%BF%BD) in the extended form.
|
||||
expect(header).toBe(`attachment; filename="_"; filename*=UTF-8''%EF%BF%BD`)
|
||||
expect(() => new Headers({ 'Content-Disposition': header })).not.toThrow()
|
||||
})
|
||||
|
||||
it('sanitizes an embedded unpaired surrogate and keeps the rest of the name', () => {
|
||||
let header = ''
|
||||
expect(() => {
|
||||
header = contentDisposition('inline', 'a\uD800b')
|
||||
}).not.toThrow()
|
||||
expect(header).toBe(`inline; filename="a_b"; filename*=UTF-8''a%EF%BF%BDb`)
|
||||
expect(() => new Headers({ 'Content-Disposition': header })).not.toThrow()
|
||||
})
|
||||
|
||||
it('keeps a valid surrogate pair (emoji) working unchanged', () => {
|
||||
const header = contentDisposition('inline', 'r😀.pdf')
|
||||
expect(header).toBe(`inline; filename="r__.pdf"; filename*=UTF-8''r%F0%9F%98%80.pdf`)
|
||||
expect(() => new Headers({ 'Content-Disposition': header })).not.toThrow()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,43 @@
|
||||
/**
|
||||
* RFC 6266 Content-Disposition builder with RFC 5987 extended filename
|
||||
* encoding.
|
||||
*
|
||||
* undici (the fetch/Headers implementation in the Next.js runtime) requires
|
||||
* header values to be ByteStrings: every code unit <= 0xFF. Splicing a raw
|
||||
* filename into the header therefore throws for any non-Latin-1 character,
|
||||
* e.g. the NFD combining diaeresis (U+0308) that macOS/iOS uploads put in
|
||||
* Swedish filenames, turning the whole response into a 500.
|
||||
*
|
||||
* The dual form emitted here is:
|
||||
*
|
||||
* <type>; filename="<ascii fallback>"; filename*=UTF-8''<percent-encoded>
|
||||
*
|
||||
* Legacy clients read `filename`; modern browsers prefer `filename*`
|
||||
* (RFC 6266 section 4.3) and decode the original UTF-8 name.
|
||||
*/
|
||||
export function contentDisposition(
|
||||
type: 'inline' | 'attachment',
|
||||
filename: string,
|
||||
): string {
|
||||
// Lone/unpaired UTF-16 surrogates survive normalize('NFC') and make
|
||||
// encodeURIComponent below throw a URIError, which would turn the download
|
||||
// response into the very 500 this helper exists to prevent. Replace them
|
||||
// with U+FFFD first so the function always returns a valid header value.
|
||||
// Then normalize NFD (macOS/iOS) to NFC so precomposed characters encode
|
||||
// as themselves instead of base letter + combining mark.
|
||||
const normalized = filename.toWellFormed().normalize('NFC')
|
||||
|
||||
// ASCII fallback for the quoted-string form: anything outside printable
|
||||
// ASCII, plus the quoted-string specials " and \, becomes _. This also
|
||||
// neutralizes CR/LF header injection.
|
||||
const fallback = normalized.replace(/[^\x20-\x7e]|["\\]/g, '_')
|
||||
|
||||
// RFC 5987 value-chars: encodeURIComponent covers everything except
|
||||
// ! ' ( ) * which it leaves bare but RFC 5987 forbids unencoded.
|
||||
const encoded = encodeURIComponent(normalized).replace(
|
||||
/[!'()*]/g,
|
||||
(c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`,
|
||||
)
|
||||
|
||||
return `${type}; filename="${fallback}"; filename*=UTF-8''${encoded}`
|
||||
}
|
||||
Reference in New Issue
Block a user