Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion packages/fetch/src/body.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,41 @@ describe('toFetchBody', () => {
expect(headers).toEqual({
'content-disposition': 'inline; filename="__mocked__"',
'content-length': '0',
'content-type': '',
'content-type': 'application/octet-stream',
'x-custom-header': 'custom-value',
'standard-server': 'file',
})
})

it('file whose type the File constructor normalized to empty', async () => {
// a declared type with a byte outside 0x20-0x7E is dropped, like an untrusted upload's could be
const file = new File(['<script>alert(1)</script>'], 'a.png', { type: 'image/png\xFF' })
expect(file.type).toBe('')

generateContentDispositionSpy.mockReturnValue('inline; filename="__mocked__"')

const [body, headers] = toFetchBody(file, baseHeaders, {})

expect(body).toBe(file)
expect(headers).toEqual({
'content-disposition': 'inline; filename="__mocked__"',
'content-length': '25',
'content-type': 'application/octet-stream',
'x-custom-header': 'custom-value',
'standard-server': 'file',
})

const response = new Response(body, { headers: toFetchHeaders(headers) })
expect(response.headers.get('content-type')).toBe('application/octet-stream')
expect(await response.text()).toBe('<script>alert(1)</script>')
})

it('file without type keeps an explicit content-type', () => {
const file = new File(['foo'], 'foo.txt')

const [, headers] = toFetchBody(file, { ...baseHeaders, 'content-type': 'text/plain' }, {})

expect(headers['content-type']).toBe('text/plain')
})

it('file', () => {
Expand Down Expand Up @@ -417,6 +448,18 @@ describe('toFetchBody', () => {
expect(headers['content-type']).toEqual([])
})

it('file without type and with removed content-type header', () => {
const file = new File(['foo'], 'foo.bin')

const [body, headers] = toFetchBody(file, { ...baseHeaders, 'content-type': [] }, {})

expect(body).toBe(file)
expect(headers['content-type']).toEqual([])

const response = new Response(body, { headers: toFetchHeaders(headers) })
expect(response.headers.has('content-type')).toBe(false)
})

it('file with size=nan', () => {
// BunS3 is a File instance but has an unknown size (NaN), so to support it we should return a stream in this case.
const file = new File(['foo'], 'foo.pdf', { type: 'application/pdf' })
Expand Down Expand Up @@ -547,6 +590,17 @@ it.each([
expect(await file.text()).toEqual('foo')
},
},
{
// received with the type it was sent with, the same one FormData gives an untyped File
name: 'file-without-type',
createBody: () => new File(['foo'], 'foo.bin'),
assertBody: async (file: any) => {
expect(file).toBeInstanceOf(File)
expect(file.name).toEqual('foo.bin')
expect(file.type).toEqual('application/octet-stream')
expect(await file.text()).toEqual('foo')
},
},
{
name: 'event-stream',
createBody: async function* gen() {
Expand Down
3 changes: 2 additions & 1 deletion packages/fetch/src/body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ export function toFetchBody(
// and a transport can drop the empty ones (bun) or a proxy rewrite the content-length.
headers['standard-server'] ??= 'file' satisfies StandardBodyHint // A File is also a Blob

headers['content-type'] ??= body.type
// An empty content-type makes browsers sniff the body, which can serve an upload as HTML
headers['content-type'] ??= body.type || 'application/octet-stream'
// FIX: Bun returns `undefined` for an empty File name, despite the spec requiring a string
headers['content-disposition'] ??= generateContentDisposition(body instanceof File ? body.name ?? '' : 'blob')

Expand Down
47 changes: 46 additions & 1 deletion packages/node/src/body.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,10 +599,41 @@ describe('toNodeHttpBody', () => {
expect(headers).toEqual({
'content-disposition': 'inline; filename="__mocked__"',
'content-length': '0',
'content-type': '',
'content-type': 'application/octet-stream',
'x-custom-header': 'custom-value',
'standard-server': 'file',
})
})

it('file whose type the File constructor normalized to empty', async () => {
// a declared type with a byte outside 0x20-0x7E is dropped, like an untrusted upload's could be
const file = new File(['<script>alert(1)</script>'], 'a.png', { type: 'image/png\xFF' })
expect(file.type).toBe('')

generateContentDispositionSpy.mockReturnValue('inline; filename="__mocked__"')

const [body, headers] = toNodeHttpBody(file, baseHeaders, {})

expect(body).toBeInstanceOf(Readable)
expect(headers).toEqual({
'content-disposition': 'inline; filename="__mocked__"',
'content-length': '25',
'content-type': 'application/octet-stream',
'x-custom-header': 'custom-value',
'standard-server': 'file',
})

const response = new Response(body, { headers: toFetchHeaders(headers) })
expect(response.headers.get('content-type')).toBe('application/octet-stream')
expect(await response.text()).toBe('<script>alert(1)</script>')
})

it('file without type keeps an explicit content-type', async () => {
const file = new File(['foo'], 'foo.txt')

const [, headers] = toNodeHttpBody(file, { ...baseHeaders, 'content-type': 'text/plain' }, {})

expect(headers['content-type']).toBe('text/plain')
})

it('file with size=nan', async () => {
Expand Down Expand Up @@ -759,5 +790,19 @@ describe('toNodeHttpBody', () => {
const fetchHeaders = toFetchHeaders(headers)
expect(fetchHeaders.has('content-type')).toBe(false)
})

it('file without type: unset content-type', async () => {
const file = new File(['foo'], 'foo.bin')
const [body, headers] = toNodeHttpBody(file, {
...baseHeaders,
'content-type': [],
})

expect(body).toBeInstanceOf(Readable)
expect(headers['content-type']).toEqual([])

const fetchHeaders = toFetchHeaders(headers)
expect(fetchHeaders.has('content-type')).toBe(false)
})
})
})
3 changes: 2 additions & 1 deletion packages/node/src/body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ export function toNodeHttpBody(
// and a transport can drop the empty ones (bun) or a proxy rewrite the content-length.
headers['standard-server'] ??= 'file' satisfies StandardBodyHint // A File is also a Blob

headers['content-type'] ??= body.type
// An empty content-type makes browsers sniff the body, which can serve an upload as HTML
headers['content-type'] ??= body.type || 'application/octet-stream'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

body.type || 'application/octet-stream' only replaces the empty string, but File.type can also be non-empty and non-parsable ('/', ' ', 'text', ';'). The node and aws-lambda receivers build new File(…, { type: contentType }) straight from a raw upload header (packages/node/src/body.ts:71, packages/aws-lambda/src/body.ts:60), and a browser treats such a supplied type as undefined and sniffs it — so this arm does not close the sniffing case for those files. The same applies to packages/fetch/src/body.ts:110 and packages/peer/src/body.ts:127.

Technical details
# Untyped fallback only covers the empty string

## Affected sites
- `packages/node/src/body.ts:117` — `??= body.type || 'application/octet-stream'`
- `packages/fetch/src/body.ts:110` — same
- `packages/peer/src/body.ts:127` — same
- `packages/node/src/body.ts:71` — `_streamToFile(..., contentType ?? '')` sets `File.type` from the raw header
- `packages/aws-lambda/src/body.ts:60` — same

## Required outcome
- An echoed `Blob`/`File` whose `.type` is not a parsable MIME type should not reach the wire sniffable.

## Suggested approach (optional)
- `x-content-type-options: nosniff` (already on the maintainer list) covers this more generally than tightening the MIME check here.

## Open questions for the human (optional)
- Fold this into the deferred `nosniff` decision, or also validate `body.type` at the send sites?

// FIX: Bun returns `undefined` for an empty File name, despite the spec requiring a string
headers['content-disposition'] ??= generateContentDisposition(body instanceof File ? body.name ?? '' : 'blob')

Expand Down
46 changes: 46 additions & 0 deletions packages/peer/src/body.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,33 @@ describe('encodeAtomicStandardBody', () => {
expect(removedHeaders['content-type']).toEqual([])
})

it('encodes File body without type as application/octet-stream', async () => {
// a declared type with a byte outside 0x20-0x7E is dropped, like an untrusted upload's could be
const file = new File(['<script>alert(1)</script>'], 'a.png', { type: 'image/png\xFF' })
expect(file.type).toBe('')

const { jsonBody, headers, binary } = await encodeAtomicStandardBody(file, {})

expect(jsonBody).toBe(undefined)
expect(headers['content-type']).toBe('application/octet-stream')
expect(headers['content-disposition']).toBe(generateContentDisposition('a.png'))
expect(headers['content-length']).toBe('25')
expect(binary).toBe(file)

const { headers: emptyHeaders } = await encodeAtomicStandardBody(new Blob([]), {})
expect(emptyHeaders['content-type']).toBe('application/octet-stream')
})

it('encodes File body without type and preserves existing content-type header', async () => {
const file = new File(['foo'], 'foo.txt')

const { headers } = await encodeAtomicStandardBody(file, { 'content-type': 'text/plain' })
expect(headers['content-type']).toBe('text/plain')

const { headers: removedHeaders } = await encodeAtomicStandardBody(file, { 'content-type': [] })
expect(removedHeaders['content-type']).toEqual([])
})

it('encodes URLSearchParams body', async () => {
const params = new URLSearchParams('a=1&b=2')
const { jsonBody, headers, binary } = await encodeAtomicStandardBody(params, {})
Expand Down Expand Up @@ -440,4 +467,23 @@ describe('encodeAtomicStandardBody', () => {
expect(received.type).toBe('application/pdf')
expect(await received.text()).toBe('file content')
})

it('round-trips a File without type as application/octet-stream', async () => {
const file = new File(['file content'], 'data.bin')
const encoded = await encodeAtomicStandardBody(file, {})

const { resolveBody } = toStandardBody({
id: '1',
kind: 'request',
json: { url: '/upload', headers: encoded.headers, body: encoded.jsonBody },
binary: encoded.binary,
}, vi.fn())

const received = await resolveBody() as File
expect(received).toBeInstanceOf(File)
expect(received.name).toBe('data.bin')
// the same type FormData gives an untyped File
expect(received.type).toBe('application/octet-stream')
expect(await received.text()).toBe('file content')
})
})
3 changes: 2 additions & 1 deletion packages/peer/src/body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ export async function encodeAtomicStandardBody(
}

if (body instanceof Blob) {
headers['content-type'] ??= body.type
// An empty content-type makes browsers sniff the body, which can serve an upload as HTML
headers['content-type'] ??= body.type || 'application/octet-stream'
// FIX: Bun returns `undefined` for an empty File name, despite the spec requiring a string
headers['content-disposition'] ??= generateContentDisposition(
body instanceof File ? body.name ?? '' : 'blob',
Expand Down
5 changes: 3 additions & 2 deletions tests/bun/tests/data-transfer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,15 @@ for (const [adapter, createClientServer] of ADAPTERS) {
},
},
{
// Bun drops empty headers like content-type, so only the body hint identifies this one
// Bun drops empty headers, so an empty file must not rely on one to be identified
name: 'empty-file',
createBody: () => new File([], '', { type: '' }),
assertBody: async (body: any) => {
expect(body).toBeInstanceOf(File)
// Bun returns `undefined` instead of '' for an empty File name
expect(body.name ?? '').toEqual('')
expect(body.type).toEqual('')
// an untyped file is sent as application/octet-stream, the same type FormData gives it
expect(body.type).toEqual('application/octet-stream')
expect(body.size).toEqual(0)
expect(await body.text()).toEqual('')
},
Expand Down
6 changes: 4 additions & 2 deletions tests/data-transfer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe.each([
['message-port-fetch-streamed', () => createMessagePortClientServerTest({ fetchStreamed: true })],
['node-ws', () => createNodeWsClientServerTest()],
['node-ws-fetch-streamed', () => createNodeWsClientServerTest({ fetchStreamed: true })],
])('data transfer: $0', (_, createClientServer) => {
])('data transfer: $0', (transport, createClientServer) => {
const clientServer = createClientServer()

beforeEach(() => {
Expand Down Expand Up @@ -135,7 +135,9 @@ describe.each([
assertBody: async (body: any) => {
expect(body).toBeInstanceOf(File)
expect(body.name).toEqual('')
expect(body.type).toEqual('')
// an untyped file is sent as application/octet-stream, the same type FormData gives it,
// except in process, where the File is handed over untouched
expect(body.type).toEqual(transport === 'inprogress' ? '' : 'application/octet-stream')
expect(body.size).toEqual(0)
expect(await body.text()).toEqual('')
},
Expand Down
3 changes: 2 additions & 1 deletion tests/deno/tests/data-transfer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ for (const [adapter, createClientServer] of ADAPTERS) {
assertBody: async (body: any) => {
expect(body).toBeInstanceOf(File)
expect(body.name ?? '').toEqual('')
expect(body.type).toEqual('')
// an untyped file is sent as application/octet-stream, the same type FormData gives it
expect(body.type).toEqual('application/octet-stream')
expect(body.size).toEqual(0)
expect(await body.text()).toEqual('')
},
Expand Down
Loading