Skip to content

Commit 7dc5ffb

Browse files
mcollinaclaude
andauthored
fix: prevent uncaught exception from malformed multipart requests (#595)
* fix: prevent uncaught exception from malformed multipart requests When req.file() is called after any async operation, malformed multipart requests could cause uncaught exceptions that crash the Node.js process. This occurred because the file stream error was emitted before user code could attach error listeners. This fix adds an immediate error listener to file streams that catches "terminated early" errors from malformed multipart data and propagates them through the normal error handling flow, preventing process crashes while preserving normal stream error handling behavior. Fixes #594 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * style: fix linting errors in test file - Prefix unused error parameter with underscore - Remove unused FormData import 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: remove unused MultipartHandler type definition Removed the unused MultipartHandler type from TypeScript definitions to fix linting error. This type was not exported or used anywhere. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: convert multipart-uncaught-error tests from tap to node:test Replace tap test framework with node:test and node:assert for the uncaught exception tests. Changes include: - Replace tap imports with node:test and node:assert - Remove t.plan() calls (not needed in node:test) - Replace t.ok() with assert.ok() - Replace t.pass() with boolean flags and assertions All tests pass with node:test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent f61b43c commit 7dc5ffb

File tree

2 files changed

+184
-0
lines changed

2 files changed

+184
-0
lines changed

index.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,20 @@ function fastifyMultipart (fastify, options, done) {
396396
}
397397
}
398398

399+
// Attach error listener immediately to prevent uncaught exceptions
400+
// This is critical when there's an async operation before req.file() is called,
401+
// allowing the file stream to emit errors before user code can attach listeners
402+
// However, we only capture and propagate errors from malformed multipart data
403+
// (e.g., "Part terminated early"), not errors from normal stream consumption
404+
file.on('error', function (err) {
405+
// Only propagate errors that indicate malformed multipart data
406+
// These are the errors that would cause uncaught exceptions
407+
if (err.message && err.message.includes('terminated early')) {
408+
onError(err)
409+
}
410+
// Other errors are expected to be handled by the consumer of the stream
411+
})
412+
399413
if (throwFileSizeLimit) {
400414
file.on('limit', function () {
401415
const err = new RequestFileTooLargeError()
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
'use strict'
2+
3+
const { test } = require('node:test')
4+
const assert = require('node:assert')
5+
const Fastify = require('fastify')
6+
const multipart = require('..')
7+
8+
test('malformed request should not cause uncaught exception when await before req.file()', async function (t) {
9+
const fastify = Fastify()
10+
11+
await fastify.register(multipart)
12+
13+
let errorListenerCalled = false
14+
let errorCaught = false
15+
16+
fastify.post('/', async function (req, reply) {
17+
// Simulate any async operation before req.file()
18+
// This allows request parsing to start before we get the file
19+
await new Promise(resolve => setImmediate(resolve))
20+
21+
try {
22+
const data = await req.file()
23+
24+
if (data) {
25+
// Attach error listener
26+
data.file.on('error', (_err) => {
27+
errorListenerCalled = true
28+
})
29+
30+
// Try to consume the file
31+
await data.toBuffer()
32+
}
33+
34+
reply.code(200).send({ ok: true })
35+
} catch (err) {
36+
errorCaught = true
37+
reply.code(400).send({ error: err.message })
38+
}
39+
})
40+
41+
await fastify.listen({ port: 0 })
42+
43+
// Send malformed multipart request (missing closing boundary)
44+
// Manually construct malformed multipart data
45+
const malformedData = '------MyBoundary\r\n' +
46+
'Content-Disposition: form-data; name="file"; filename="test.txt"\r\n' +
47+
'Content-Type: text/plain\r\n' +
48+
'\r\n' +
49+
'ABC'
50+
// Note: Missing closing boundary (------MyBoundary--)
51+
52+
try {
53+
const response = await fastify.inject({
54+
method: 'POST',
55+
url: '/',
56+
headers: {
57+
'content-type': 'multipart/form-data; boundary=----MyBoundary'
58+
},
59+
payload: malformedData
60+
})
61+
62+
// The request should complete without crashing
63+
assert.ok(response, 'request completed without uncaught exception')
64+
} catch (err) {
65+
// Even if there's an error, it should be catchable
66+
assert.ok(err, 'error was catchable')
67+
}
68+
69+
assert.ok(errorListenerCalled || errorCaught, 'error was handled either by listener or catch block')
70+
71+
await fastify.close()
72+
})
73+
74+
test('malformed request with req.files() should not cause uncaught exception', async function (t) {
75+
const fastify = Fastify()
76+
77+
await fastify.register(multipart)
78+
79+
fastify.post('/', async function (req, reply) {
80+
await new Promise(resolve => setImmediate(resolve))
81+
82+
try {
83+
const files = req.files()
84+
85+
for await (const file of files) {
86+
file.file.on('error', () => {})
87+
await file.toBuffer().catch(() => {})
88+
}
89+
90+
reply.code(200).send({ ok: true })
91+
} catch (err) {
92+
reply.code(400).send({ error: err.message })
93+
}
94+
})
95+
96+
await fastify.listen({ port: 0 })
97+
98+
const malformedData = '------MyBoundary\r\n' +
99+
'Content-Disposition: form-data; name="file"; filename="test.txt"\r\n' +
100+
'Content-Type: text/plain\r\n' +
101+
'\r\n' +
102+
'ABC'
103+
104+
try {
105+
const response = await fastify.inject({
106+
method: 'POST',
107+
url: '/',
108+
headers: {
109+
'content-type': 'multipart/form-data; boundary=----MyBoundary'
110+
},
111+
payload: malformedData
112+
})
113+
114+
assert.ok(response, 'request completed without uncaught exception')
115+
} catch (err) {
116+
assert.ok(err, 'error was catchable')
117+
}
118+
119+
await fastify.close()
120+
})
121+
122+
test('malformed request with req.parts() should not cause uncaught exception', async function (t) {
123+
const fastify = Fastify()
124+
125+
await fastify.register(multipart)
126+
127+
fastify.post('/', async function (req, reply) {
128+
await new Promise(resolve => setImmediate(resolve))
129+
130+
try {
131+
const parts = req.parts()
132+
133+
for await (const part of parts) {
134+
if (part.file) {
135+
part.file.on('error', () => {})
136+
await part.toBuffer().catch(() => {})
137+
}
138+
}
139+
140+
reply.code(200).send({ ok: true })
141+
} catch (err) {
142+
reply.code(400).send({ error: err.message })
143+
}
144+
})
145+
146+
await fastify.listen({ port: 0 })
147+
148+
const malformedData = '------MyBoundary\r\n' +
149+
'Content-Disposition: form-data; name="file"; filename="test.txt"\r\n' +
150+
'Content-Type: text/plain\r\n' +
151+
'\r\n' +
152+
'ABC'
153+
154+
try {
155+
const response = await fastify.inject({
156+
method: 'POST',
157+
url: '/',
158+
headers: {
159+
'content-type': 'multipart/form-data; boundary=----MyBoundary'
160+
},
161+
payload: malformedData
162+
})
163+
164+
assert.ok(response, 'request completed without uncaught exception')
165+
} catch (err) {
166+
assert.ok(err, 'error was catchable')
167+
}
168+
169+
await fastify.close()
170+
})

0 commit comments

Comments
 (0)