Skip to content
Merged
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
10 changes: 4 additions & 6 deletions packages/bun-uws/src/HttpContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<SSL, true> *) httpResponseData;
nodeHttpRequestTrailers = &nodeHttpResponseData->nodeHttpRequestTrailers;
nodeHttpChunkedExtensionsByteCount = &nodeHttpResponseData->chunkedExtensionsByteCount;
}

auto result = httpResponseData->template consumePostPadded<IsNodeHttp>(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<IsNodeHttp>(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 */
Expand Down
26 changes: 12 additions & 14 deletions packages/bun-uws/src/HttpParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down Expand Up @@ -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. */
Comment thread
robobun marked this conversation as resolved.
static const uint64_t MAX_CHUNK_EXTENSION_SIZE = 16 * 1024;

/* Returns UINT64_MAX on error. Maximum 999999999 is allowed. */
Expand Down Expand Up @@ -1261,17 +1261,15 @@ 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) */
std::string_view dataToConsume(data, length);
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
Expand All @@ -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]] {
Expand Down Expand Up @@ -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
Expand All @@ -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)) {
Expand Down Expand Up @@ -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
Expand All @@ -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)) {
Expand Down
10 changes: 5 additions & 5 deletions packages/bun-uws/src/HttpResponseData.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ struct HttpResponseData : AsyncSocketData<SSL>, 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
Expand Down Expand Up @@ -234,11 +239,6 @@ struct HttpResponseData<SSL, true> : HttpResponseData<SSL, false> {
* 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
Expand Down
93 changes: 92 additions & 1 deletion test/js/bun/http/http-server-chunking.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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,
Expand Down Expand Up @@ -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) {
Expand Down
Loading