From aa08300244f4536301c93c18b6715b5b60fdbb7c Mon Sep 17 00:00:00 2001 From: Ming Xie Date: Tue, 13 Oct 2026 17:32:55 -0400 Subject: [PATCH 1/4] fix(dev): respond with 413/408 instead of crashing on oversized or slow request bodies --- eslint-suppressions.json | 2 +- src/utils/create-stream-promise.ts | 16 +++++- src/utils/proxy.ts | 57 +++++++++++++++++++--- tests/integration/commands/dev/dev.test.ts | 25 ++++++++++ 4 files changed, 91 insertions(+), 9 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index a6cf561216c..f257bc62881 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1080,7 +1080,7 @@ "count": 1 }, "@typescript-eslint/no-misused-promises": { - "count": 7 + "count": 4 }, "@typescript-eslint/no-unnecessary-condition": { "count": 7 diff --git a/src/utils/create-stream-promise.ts b/src/utils/create-stream-promise.ts index b4a58590ab6..ca41658e4c9 100644 --- a/src/utils/create-stream-promise.ts +++ b/src/utils/create-stream-promise.ts @@ -6,6 +6,18 @@ const SEC_TO_MILLISEC = 1e3 // 6 MiB const DEFAULT_BYTES_LIMIT = 6e6 +export class StreamBodyTooBigError extends Error { + constructor() { + super('Stream body too big') + } +} + +export class StreamTimeoutError extends Error { + constructor() { + super('Request timed out waiting for body') + } +} + const createStreamPromise = function ( stream: Readable, timeoutSeconds: number, @@ -19,7 +31,7 @@ const createStreamPromise = function ( if (timeoutSeconds != null && Number.isFinite(timeoutSeconds)) { timeoutId = setTimeout(() => { data = null - reject(new Error('Request timed out waiting for body')) + reject(new StreamTimeoutError()) }, timeoutSeconds * SEC_TO_MILLISEC) } @@ -31,7 +43,7 @@ const createStreamPromise = function ( dataLength += chunk.length if (dataLength > bytesLimit) { data = null - reject(new Error('Stream body too big')) + reject(new StreamBodyTooBigError()) } else { data.push(chunk) } diff --git a/src/utils/proxy.ts b/src/utils/proxy.ts index 0a066360858..8b25914f187 100644 --- a/src/utils/proxy.ts +++ b/src/utils/proxy.ts @@ -50,7 +50,7 @@ import { logError, warn, } from './command-helpers.js' -import createStreamPromise from './create-stream-promise.js' +import createStreamPromise, { StreamBodyTooBigError, StreamTimeoutError } from './create-stream-promise.js' import { NFFunctionName, NFFunctionRoute, NFRequestID, headersForPath, parseHeaders } from './headers.js' import { generateRequestID } from './request-id.js' import { createRewriter, onChanges } from './rules-proxy.js' @@ -723,6 +723,32 @@ const initializeProxy = async function ({ return handlers } +const getRequestBodyErrorStatus = (error: unknown) => { + if (error instanceof StreamBodyTooBigError) { + return 413 + } + + if (error instanceof StreamTimeoutError) { + return 408 + } +} + +const rejectRequestBody = (req: Request, res: ServerResponse, error: unknown) => { + const status = getRequestBodyErrorStatus(error) + + if (status === undefined) { + res.destroy() + return + } + + log( + NETLIFYDEVWARN, + `Responded to ${req.method ?? ''} ${req.url ?? ''} with ${status.toString()}: ${(error as Error).message}`, + ) + res.writeHead(status, { Connection: 'close' }) + res.end(http.STATUS_CODES[status]) +} + const onRequest = async ( { api, @@ -745,8 +771,15 @@ const onRequest = async ( req: Request, res: ServerResponse, ) => { - req.originalBody = - req.method && ['GET', 'OPTIONS', 'HEAD'].includes(req.method) ? null : await createStreamPromise(req, BYTES_LIMIT) + try { + req.originalBody = + req.method && ['GET', 'OPTIONS', 'HEAD'].includes(req.method) + ? null + : await createStreamPromise(req, REQUEST_BODY_TIMEOUT_SECONDS) + } catch (error) { + rejectRequestBody(req, res, error) + return + } if (isImageRequest(req)) { return imageProxy(req, res) @@ -1017,7 +1050,7 @@ export const startProxy = async function ({ projectDir, }) - const onRequestWithOptions = onRequest.bind(undefined, { + const requestOptions = { proxy, rewriter, serverHandler, @@ -1029,7 +1062,19 @@ export const startProxy = async function ({ siteInfo, env, api, - }) + } + const onRequestWithOptions = (req: Request, res: ServerResponse) => { + onRequest(requestOptions, req, res).catch((error: unknown) => { + logError(`Failed to handle request: ${error instanceof Error ? error.message : String(error)}`) + + if (res.headersSent) { + res.destroy() + } else { + res.writeHead(500) + res.end('Could not proxy request.') + } + }) + } const primaryServer = settings.https ? https.createServer({ cert: settings.https.cert, key: settings.https.key }, onRequestWithOptions) : http.createServer(onRequestWithOptions) @@ -1087,4 +1132,4 @@ export const startProxy = async function ({ return getProxyUrl(settings) } -const BYTES_LIMIT = 30 +const REQUEST_BODY_TIMEOUT_SECONDS = 30 diff --git a/tests/integration/commands/dev/dev.test.ts b/tests/integration/commands/dev/dev.test.ts index 1f1c9ddecaf..09114585ea9 100644 --- a/tests/integration/commands/dev/dev.test.ts +++ b/tests/integration/commands/dev/dev.test.ts @@ -426,6 +426,31 @@ describe.concurrent('command/dev', () => { }) }) + test('should respond with 413 to a request body over the size limit and not crash dev server', async (t) => { + await withSiteBuilder(t, async (builder) => { + builder.withContentFile({ + path: 'index.html', + content: 'index', + }) + + await builder.build() + + await withDevServer({ cwd: builder.directory }, async (server) => { + const response = await fetch(`${server.url}/upload`, { + method: 'POST', + body: Buffer.alloc(6_100_000), + }) + + t.expect(response.status).toBe(413) + + const indexResponse = await fetch(server.url) + + t.expect(indexResponse.status).toBe(200) + t.expect(await indexResponse.text()).toEqual('index') + }) + }) + }) + test('should return .html file when file and folder have the same name', async (t) => { await withSiteBuilder(t, async (builder) => { builder From 7c6ad72c4c8c8290fc588652f54a7b414a1bd54a Mon Sep 17 00:00:00 2001 From: Ming Xie Date: Tue, 13 Oct 2026 17:46:28 -0400 Subject: [PATCH 2/4] fix(dev): deliver 413/408 without resetting the connection and log request failures with stacks --- src/utils/create-stream-promise.ts | 2 ++ src/utils/proxy.ts | 26 ++++++---------------- tests/integration/commands/dev/dev.test.ts | 22 ++++++++++++++++++ 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/utils/create-stream-promise.ts b/src/utils/create-stream-promise.ts index ca41658e4c9..6df319ebf4f 100644 --- a/src/utils/create-stream-promise.ts +++ b/src/utils/create-stream-promise.ts @@ -9,12 +9,14 @@ const DEFAULT_BYTES_LIMIT = 6e6 export class StreamBodyTooBigError extends Error { constructor() { super('Stream body too big') + this.name = 'StreamBodyTooBigError' } } export class StreamTimeoutError extends Error { constructor() { super('Request timed out waiting for body') + this.name = 'StreamTimeoutError' } } diff --git a/src/utils/proxy.ts b/src/utils/proxy.ts index 8b25914f187..b5090af399f 100644 --- a/src/utils/proxy.ts +++ b/src/utils/proxy.ts @@ -42,6 +42,7 @@ import { DEFAULT_FUNCTION_URL_EXPRESSION } from '../lib/functions/registry.js' import { initializeProxy as initializeImageProxy, isImageRequest } from '../lib/images/proxy.js' import { + NETLIFYDEVERR, NETLIFYDEVLOG, NETLIFYDEVWARN, type NormalizedCachedConfigConfig, @@ -723,29 +724,16 @@ const initializeProxy = async function ({ return handlers } -const getRequestBodyErrorStatus = (error: unknown) => { - if (error instanceof StreamBodyTooBigError) { - return 413 - } - - if (error instanceof StreamTimeoutError) { - return 408 - } -} - const rejectRequestBody = (req: Request, res: ServerResponse, error: unknown) => { - const status = getRequestBodyErrorStatus(error) - - if (status === undefined) { + if (!(error instanceof StreamBodyTooBigError || error instanceof StreamTimeoutError)) { res.destroy() return } - log( - NETLIFYDEVWARN, - `Responded to ${req.method ?? ''} ${req.url ?? ''} with ${status.toString()}: ${(error as Error).message}`, - ) - res.writeHead(status, { Connection: 'close' }) + const status = error instanceof StreamBodyTooBigError ? 413 : 408 + + log(NETLIFYDEVWARN, `Responded to ${req.method ?? ''} ${req.url ?? ''} with ${status.toString()}: ${error.message}`) + res.writeHead(status) res.end(http.STATUS_CODES[status]) } @@ -1065,7 +1053,7 @@ export const startProxy = async function ({ } const onRequestWithOptions = (req: Request, res: ServerResponse) => { onRequest(requestOptions, req, res).catch((error: unknown) => { - logError(`Failed to handle request: ${error instanceof Error ? error.message : String(error)}`) + console.error(`${NETLIFYDEVERR} Failed to handle request to ${req.url ?? ''}`, error) if (res.headersSent) { res.destroy() diff --git a/tests/integration/commands/dev/dev.test.ts b/tests/integration/commands/dev/dev.test.ts index 09114585ea9..5d5f2a928c5 100644 --- a/tests/integration/commands/dev/dev.test.ts +++ b/tests/integration/commands/dev/dev.test.ts @@ -451,6 +451,28 @@ describe.concurrent('command/dev', () => { }) }) + test('should respond with 500 when handling a request fails and not crash dev server', async (t) => { + await withSiteBuilder(t, async (builder) => { + builder.withContentFile({ + path: 'index.html', + content: 'index', + }) + + await builder.build() + + await withDevServer({ cwd: builder.directory }, async (server) => { + const response = await fetch(`${server.url}/%E0%A4%A`) + + t.expect(response.status).toBe(500) + + const indexResponse = await fetch(server.url) + + t.expect(indexResponse.status).toBe(200) + t.expect(await indexResponse.text()).toEqual('index') + }) + }) + }) + test('should return .html file when file and folder have the same name', async (t) => { await withSiteBuilder(t, async (builder) => { builder From 775f05172e57e5c9d803cf78641526949e603205 Mon Sep 17 00:00:00 2001 From: Ming Xie Date: Tue, 13 Oct 2026 18:00:41 -0400 Subject: [PATCH 3/4] fix(dev): keep the request URL out of the error log's format string --- src/utils/proxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/proxy.ts b/src/utils/proxy.ts index b5090af399f..cd029f579ad 100644 --- a/src/utils/proxy.ts +++ b/src/utils/proxy.ts @@ -1053,7 +1053,7 @@ export const startProxy = async function ({ } const onRequestWithOptions = (req: Request, res: ServerResponse) => { onRequest(requestOptions, req, res).catch((error: unknown) => { - console.error(`${NETLIFYDEVERR} Failed to handle request to ${req.url ?? ''}`, error) + console.error(NETLIFYDEVERR, 'Failed to handle request to', req.url, error) if (res.headersSent) { res.destroy() From 3cf08d2239536bc4e84c5f5c5e1d8c0ee108e0e9 Mon Sep 17 00:00:00 2001 From: Ming Xie Date: Fri, 16 Oct 2026 17:30:53 -0400 Subject: [PATCH 4/4] test(dev): name the malformed URL trigger and assert the logged error --- tests/integration/commands/dev/dev.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/commands/dev/dev.test.ts b/tests/integration/commands/dev/dev.test.ts index 5d5f2a928c5..71278efe434 100644 --- a/tests/integration/commands/dev/dev.test.ts +++ b/tests/integration/commands/dev/dev.test.ts @@ -451,7 +451,7 @@ describe.concurrent('command/dev', () => { }) }) - test('should respond with 500 when handling a request fails and not crash dev server', async (t) => { + test('should respond with 500 and log the error instead of crashing when the request URL cannot be decoded', async (t) => { await withSiteBuilder(t, async (builder) => { builder.withContentFile({ path: 'index.html', @@ -469,6 +469,8 @@ describe.concurrent('command/dev', () => { t.expect(indexResponse.status).toBe(200) t.expect(await indexResponse.text()).toEqual('index') + t.expect(server.error).toContain('Failed to handle request to /%E0%A4%A') + t.expect(server.error).toContain('URIError: URI malformed') }) }) })