diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 86fff4f1bda2..1919787a6648 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -328,18 +328,16 @@ struct HttpContext { /* The return value is entirely up to us to interpret. The HttpParser cares only for whether the returned value is DIFFERENT from passed user */ - /* node:http compat: the parser's per-connection node state lives in the - * IsNodeHttp=true ext block; the Bun.serve instantiation passes nullptr - * (and its parser instantiation contains no use of them). */ + /* node:http compat: the trailer capture lives in the IsNodeHttp=true ext + * block; the Bun.serve instantiation passes nullptr (and its parser + * instantiation contains no use of it). */ std::string *nodeHttpRequestTrailers = nullptr; - uint64_t *nodeHttpChunkedExtensionsByteCount = nullptr; if constexpr (IsNodeHttp) { auto *nodeHttpResponseData = (HttpResponseData *) httpResponseData; nodeHttpRequestTrailers = &nodeHttpResponseData->nodeHttpRequestTrailers; - nodeHttpChunkedExtensionsByteCount = &nodeHttpResponseData->chunkedExtensionsByteCount; } - auto result = httpResponseData->template consumePostPadded(httpContextData->maxHeaderSize, httpResponseData->isConnectRequest, httpContextData->flags.requireHostHeader,httpContextData->flags.useStrictMethodValidation, httpContextData->flags.useInsecureHTTPParser, nodeHttpRequestTrailers, nodeHttpChunkedExtensionsByteCount, data, (unsigned int) length, s, proxyParser, [httpContextData](void *s, HttpRequest *httpRequest) -> void * { + auto result = httpResponseData->template consumePostPadded(httpContextData->maxHeaderSize, httpResponseData->isConnectRequest, httpContextData->flags.requireHostHeader,httpContextData->flags.useStrictMethodValidation, httpContextData->flags.useInsecureHTTPParser, nodeHttpRequestTrailers, &httpResponseData->chunkedExtensionsByteCount, data, (unsigned int) length, s, proxyParser, [httpContextData](void *s, HttpRequest *httpRequest) -> void * { /* For every request we reset the timeout and hang until user makes action */ diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index b584d5c2489e..a85f2539a9d5 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -86,8 +86,8 @@ struct HttpResponseData; /* A bare CR (not followed by LF) terminated a header value (llhttp's * HPE_LF_EXPECTED). */ HTTP_PARSER_ERROR_LF_EXPECTED = 11, - /* node:http compat only: the chunk extensions of a single chunk exceeded - * the 16 KiB limit enforced by Node (HPE_CHUNK_EXTENSIONS_OVERFLOW). */ + /* The chunk extensions of a single chunk exceeded the 16 KiB limit + * (Node/llhttp's HPE_CHUNK_EXTENSIONS_OVERFLOW). */ HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW = 12, /* An HTTP/2 client connection preface was received on an HTTP/1 server * (llhttp's HPE_PAUSED_H2_UPGRADE). */ @@ -573,9 +573,9 @@ struct HttpResponseData; const size_t MAX_FALLBACK_SIZE = BUN_DEFAULT_MAX_HTTP_HEADER_SIZE; - /* Maximum size of the chunk extensions of a single chunk, matching Node's - * kMaxChunkExtensionsSize in src/node_http_parser.cc (16 KiB). Enforced - * only for node:http compat servers. */ + /* Maximum chunk-extension bytes per chunk, matching Node/llhttp's + * kMaxChunkExtensionsSize (16 KiB). Enforced for every server + * personality so a client cannot stream unbounded extension bytes. */ static const uint64_t MAX_CHUNK_EXTENSION_SIZE = 16 * 1024; /* Returns UINT64_MAX on error. Maximum 999999999 is allowed. */ @@ -1261,9 +1261,7 @@ struct HttpResponseData; } else if (transferEncoding.has) { /* We already validated that chunked is last if present, before calling the handler */ remainingStreamingBytes = STATE_IS_CHUNKED; - if constexpr (IsNodeHttp) { - *chunkedExtensionsByteCount = 0; - } + *chunkedExtensionsByteCount = 0; /* If consume minimally, we do not want to consume anything but we want to mark this as being chunked */ if constexpr (!ConsumeMinimally) { /* Go ahead and parse it (todo: better heuristics for emitting FIN to the app level) */ @@ -1271,7 +1269,7 @@ struct HttpResponseData; for (auto chunk : uWS::ChunkIterator(&dataToConsume, &remainingStreamingBytes, false, chunkedExtensionsByteCount, nodeHttpRequestTrailers, maxBufferedHeaderSize)) { /* llhttp errors at the offending extension byte, before any body bytes from * that chunk reach the application; check before every dispatch. */ - if (IsNodeHttp && *chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { + if (*chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { return HttpParserResult::error(HTTP_ERROR_413_PAYLOAD_TOO_LARGE, HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW); } /* The fin dispatch completes the message: a malformed trailer field @@ -1286,7 +1284,7 @@ struct HttpResponseData; return HttpParserResult::success(consumedTotal, returnedUser); } } - if (IsNodeHttp && *chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { + if (*chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { return HttpParserResult::error(HTTP_ERROR_413_PAYLOAD_TOO_LARGE, HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW); } if (isParsingInvalidChunkedEncoding(remainingStreamingBytes)) [[unlikely]] { @@ -1352,7 +1350,7 @@ struct HttpResponseData; /* It's either chunked or with a content-length */ std::string_view dataToConsume(data, length); for (auto chunk : uWS::ChunkIterator(&dataToConsume, &remainingStreamingBytes, false, chunkedExtensionsByteCount, nodeHttpRequestTrailers, maxFallbackSize)) { - if (IsNodeHttp && *chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { + if (*chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { return HttpParserResult::error(HTTP_ERROR_413_PAYLOAD_TOO_LARGE, HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW); } /* The fin dispatch completes the message: a malformed trailer field @@ -1365,7 +1363,7 @@ struct HttpResponseData; return HttpParserResult::success(0, returnedUser); } } - if (IsNodeHttp && *chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { + if (*chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { return HttpParserResult::error(HTTP_ERROR_413_PAYLOAD_TOO_LARGE, HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW); } if (isParsingInvalidChunkedEncoding(remainingStreamingBytes)) { @@ -1435,7 +1433,7 @@ struct HttpResponseData; /* It's either chunked or with a content-length */ std::string_view dataToConsume(data, length); for (auto chunk : uWS::ChunkIterator(&dataToConsume, &remainingStreamingBytes, false, chunkedExtensionsByteCount, nodeHttpRequestTrailers, maxFallbackSize)) { - if (IsNodeHttp && *chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { + if (*chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { return HttpParserResult::error(HTTP_ERROR_413_PAYLOAD_TOO_LARGE, HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW); } /* The fin dispatch completes the message: a malformed trailer field @@ -1448,7 +1446,7 @@ struct HttpResponseData; return HttpParserResult::success(0, returnedUser); } } - if (IsNodeHttp && *chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { + if (*chunkedExtensionsByteCount > MAX_CHUNK_EXTENSION_SIZE) [[unlikely]] { return HttpParserResult::error(HTTP_ERROR_413_PAYLOAD_TOO_LARGE, HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW); } if (isParsingInvalidChunkedEncoding(remainingStreamingBytes)) { diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 2da4f8a5cb41..05b7cc180b7e 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -195,6 +195,11 @@ struct HttpResponseData : AsyncSocketData, HttpParser { * so it cannot live in `state`. */ bool isConnectRequest = false; + /* Chunk-extension bytes consumed on the current chunk-size line, reset per + * chunk (llhttp's on_chunk_header); capped at MAX_CHUNK_EXTENSION_SIZE for + * both Bun.serve and node:http servers. */ + uint64_t chunkedExtensionsByteCount = 0; + /* node:http server compat: number of pipelined responses dispatched to JS * that have not yet become this connection's current response. While * non-zero, newly parsed requests keep being queued (preserving response @@ -234,11 +239,6 @@ struct HttpResponseData : HttpResponseData { * Mirrors last_message_start_/headers_completed_ in Node's http parser * ConnectionsList, which back server.headersTimeout/requestTimeout. */ uint64_t lastMessageStartMs = 0; - /* Bytes of chunk extensions consumed on the current chunk-size line of the - * request body, matching llhttp/Node which resets the counter in - * on_chunk_header (per chunk, not per message). The parser gets it as a - * nullable pointer (see HttpParser::consumePostPadded). */ - uint64_t chunkedExtensionsByteCount = 0; /* Trailer fields set via response.addTrailers(), pre-rendered as * "name: value\r\n" lines. Written between the terminating 0 chunk and the * final CRLF of a chunked response (RFC 9112 7.1.2); non-empty also forces diff --git a/test/js/bun/http/http-server-chunking.test.ts b/test/js/bun/http/http-server-chunking.test.ts index 387635bc2312..f73cc302c33f 100644 --- a/test/js/bun/http/http-server-chunking.test.ts +++ b/test/js/bun/http/http-server-chunking.test.ts @@ -229,6 +229,97 @@ describe.if(isPosix)("HTTP server handles split chunk-size CRLF", () => { expect(exitCode).toBe(0); }); + test.concurrentIf(!isASAN)("rejects chunk extensions that exceed the 16 KiB per-chunk cap", async () => { + // A hostile client can hold a connection and unmetered inbound bandwidth by + // streaming arbitrarily large chunk-extension bytes, which maxRequestBodySize + // never sees. llhttp (Node) caps this at 16 KiB per chunk; Bun.serve must too. + const script = ` + const server = Bun.serve({ + port: 0, + maxRequestBodySize: 1024 * 1024, + async fetch(req) { + const n = (await req.arrayBuffer()).byteLength; + return new Response("n=" + n); + }, + }); + const { promise, resolve } = Promise.withResolvers(); + let received = ""; + const socket = await Bun.connect({ + hostname: "localhost", + port: server.port, + socket: { + data(s, d) { received += d.toString(); }, + open(s) { + // 20 KiB of extension bytes on one chunk-size line (cap is 16 KiB). + // Body is 2 bytes, well under maxRequestBodySize. + const ext = Buffer.alloc(20 * 1024, "e").toString(); + s.write("POST / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\nTransfer-Encoding: chunked\\r\\n\\r\\n2;" + ext + "\\r\\nhi\\r\\n0\\r\\n\\r\\n"); + s.flush(); + }, + error() {}, + close() { console.log(JSON.stringify({ received })); resolve(); }, + }, + }); + await promise; + server.stop(); + `; + + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", script], env: bunEnv, stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { received } = JSON.parse(stdout); + // Server must reject (413) and close the connection; it must not hand the + // request to fetch() (which would answer 200 "n=2"). + expect(received).not.toContain("200"); + expect(received).not.toContain("n=2"); + expect(received).toContain("413"); + expect(exitCode).toBe(0); + }); + + test.concurrentIf(!isASAN)( + "accepts small chunk extensions on every chunk (cap is per chunk, not per message)", + async () => { + // 10 chunks x 8 KiB extension each = 80 KiB total extension bytes, but each + // chunk-size line is under the 16 KiB cap so the request must succeed. + const script = ` + const server = Bun.serve({ + port: 0, + async fetch(req) { return new Response("Got: " + (await req.text())); }, + }); + const { promise, resolve } = Promise.withResolvers(); + let received = ""; + const socket = await Bun.connect({ + hostname: "localhost", + port: server.port, + socket: { + data(s, d) { received += d.toString(); }, + open(s) { + const ext = Buffer.alloc(8 * 1024, "e").toString(); + let wire = "POST / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\nTransfer-Encoding: chunked\\r\\n\\r\\n"; + for (let i = 0; i < 10; i++) wire += "1;" + ext + "\\r\\nA\\r\\n"; + wire += "0\\r\\n\\r\\n"; + s.write(wire); + s.flush(); + }, + error() {}, + close() { console.log(received); resolve(); }, + }, + }); + await promise; + server.stop(); + `; + + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", script], env: bunEnv, stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(stdout).toContain("200 OK"); + expect(stdout).toContain("Got: AAAAAAAAAA"); + expect(exitCode).toBe(0); + }, + ); + test.concurrentIf(!isASAN)("rejects bare LF in chunk-size position (invalid byte not stranded)", async () => { // A byte <=32 that isn't \r in chunk-size position must error immediately. // Previously this could strand the byte in HttpParser's fallback buffer, @@ -442,7 +533,7 @@ describe.if(isPosix)("HTTP server handles split chunk-size CRLF", () => { describe.if(isPosix)("HTTP server handles fragmented requests", () => { test.concurrentIf(!isASAN)("handles requests with tiny send buffer (regression test)", async () => { using server = Bun.serve({ - hostname: "localhost", + hostname: "127.0.0.1", port: 0, async fetch(req) {