diff --git a/eslint-suppressions.json b/eslint-suppressions.json index ef6e75625fd..edfd52c3539 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1077,7 +1077,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..6df319ebf4f 100644 --- a/src/utils/create-stream-promise.ts +++ b/src/utils/create-stream-promise.ts @@ -6,6 +6,20 @@ const SEC_TO_MILLISEC = 1e3 // 6 MiB 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' + } +} + const createStreamPromise = function ( stream: Readable, timeoutSeconds: number, @@ -19,7 +33,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 +45,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 a6bdee4e458..1ec38d3a3fd 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, @@ -50,7 +51,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' @@ -732,6 +733,19 @@ const initializeProxy = async function ({ return handlers } +const rejectRequestBody = (req: Request, res: ServerResponse, error: unknown) => { + if (!(error instanceof StreamBodyTooBigError || error instanceof StreamTimeoutError)) { + res.destroy() + return + } + + 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]) +} + const onRequest = async ( { api, @@ -754,8 +768,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) @@ -1032,7 +1053,7 @@ export const startProxy = async function ({ projectDir, }) - const onRequestWithOptions = onRequest.bind(undefined, { + const requestOptions = { proxy, rewriter, serverHandler, @@ -1044,7 +1065,19 @@ export const startProxy = async function ({ siteInfo, env, api, - }) + } + const onRequestWithOptions = (req: Request, res: ServerResponse) => { + onRequest(requestOptions, req, res).catch((error: unknown) => { + console.error(NETLIFYDEVERR, 'Failed to handle request to', req.url, 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) @@ -1102,4 +1135,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 f7a87122eed..f67044655c9 100644 --- a/tests/integration/commands/dev/dev.test.ts +++ b/tests/integration/commands/dev/dev.test.ts @@ -428,6 +428,55 @@ 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 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', + 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') + t.expect(server.error).toContain('Failed to handle request to /%E0%A4%A') + t.expect(server.error).toContain('URIError: URI malformed') + }) + }) + }) + test('should not crash dev server when the framework server resets the connection mid-response', async (t) => { await withSiteBuilder(t, async (builder) => { const targetPort = await getPort()