From f8d7d72a148513b28b4c49909b07a0c99c9894bc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:42:26 +0000 Subject: [PATCH 1/5] Bun.serve: keep a split request head alive when the handler closes its socket A request head that arrives split over two reads is parsed out of the HTTP parser's per-socket fallback buffer, and the uWS HttpRequest the dispatch holds views into that buffer. server.stop(true) inside the handler closes the request's own socket right there, and HttpContext::onClose destructs the parser with its buffer. Everything that materialises the headers afterwards read freed memory: req.headers inside the handler, and the url/header snapshot the server takes when an async handler ends the dispatch. onClose now hands the buffer to the parse frame in onData, which owns it until it returns, after the last dispatch that can view it. --- packages/bun-uws/src/HttpContext.h | 21 ++++ packages/bun-uws/src/HttpContextData.h | 7 ++ packages/bun-uws/src/HttpParser.h | 12 +++ .../serve-pending-promise-abort-leak.test.ts | 101 ++++++++++++++++++ 4 files changed, 141 insertions(+) diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 22b93c4e64b3..67235805847e 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -248,6 +248,19 @@ struct HttpContext { /* Call filter */ HttpContextData *httpContextData = getSocketContextDataS(s); + /* This socket is closing from inside its own parse frame: a request + * handler called server.stop(true), or a response completed and the + * connection close gate fired. The HttpRequest that frame is + * dispatching holds string_views into the parser's fallback buffer when + * its head arrived split over two reads, and the handler can still + * materialise the headers from it (lazily, or through the post-handler + * snapshot). Give those bytes to the parse frame, which outlives every + * dispatch that can view them, instead of freeing them with the parser + * below. */ + if (httpContextData->parsingSocket == s && httpContextData->parsedFallbackHolder) { + *httpContextData->parsedFallbackHolder = httpResponseData->takeFallbackBuffer(); + } + bool nodeHttpTunnelAfterBody = false; if constexpr (IsNodeHttp) nodeHttpTunnelAfterBody = (httpResponseData->state & HttpResponseData::HTTP_NODE_TUNNEL_AFTER_BODY) != 0; if(httpResponseData->isConnectRequest || nodeHttpTunnelAfterBody) { @@ -392,6 +405,13 @@ struct HttpContext { httpContextData->parsingSocket = s; httpResponseData->isIdle = false; + /* Owns the parser's fallback buffer once a close below hands it over + * (see onClose). It is freed when this frame returns, after the last + * dispatch that can hold string_views into it. */ + std::string parsedFallback; + std::string *prevParsedFallbackHolder = httpContextData->parsedFallbackHolder; + httpContextData->parsedFallbackHolder = &parsedFallback; + /* node:http compat: maintain the headers/request timeout window (see * the requestHandler/dataHandler hooks and the post-parse check). */ const bool trackNodeHttpTimings = IsNodeHttp && !httpResponseData->isConnectRequest; @@ -651,6 +671,7 @@ struct HttpContext { /* Mark that we are no longer parsing Http */ httpContextData->flags.isParsingHttp = false; httpContextData->parsingSocket = prevParsingSocket; + httpContextData->parsedFallbackHolder = prevParsedFallbackHolder; /* If we got fullptr that means the parser wants us to close the socket from error (same as calling the errorHandler) */ if (httpErrorStatusCode) { /* node:http compat: parse errors surface as the server's 'clientError' diff --git a/packages/bun-uws/src/HttpContextData.h b/packages/bun-uws/src/HttpContextData.h index 5239d62a1ebd..810fca444b55 100644 --- a/packages/bun-uws/src/HttpContextData.h +++ b/packages/bun-uws/src/HttpContextData.h @@ -77,6 +77,13 @@ struct alignas(16) HttpContextData { * checks the parsed socket. */ struct us_socket_t *parsingSocket = nullptr; + /* Where onClose parks the fallback buffer of parsingSocket's parser, owned + * by that parse frame in onData from then on. A close inside the dispatch + * (server.stop(true) in a request handler) destructs the parser while the + * HttpRequest being dispatched still holds string_views into the buffer. + * nullptr outside a parse. */ + std::string *parsedFallbackHolder = nullptr; + /* This is the default router for default SNI or non-SSL */ HttpRouter router; void *upgradedWebSocket = nullptr; diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index 7925b43cdf92..98ba91bf3d0f 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -484,6 +484,18 @@ struct HttpResponseData; return remainingStreamingBytes != 0; } + /* Hands the fallback buffer to the caller, which becomes its owner. A + * request head that arrived split over two reads is parsed out of this + * buffer, so the HttpRequest the parse loop is dispatching right now + * holds string_views into it. HttpContext::onClose calls this before it + * destructs the parser under that dispatch. The move keeps the heap + * block, so the views stay valid: consumePostPadded reserves at least + * MINIMUM_HTTP_POST_PADDING bytes before it parses out of the buffer, + * which is past every short string optimization threshold. */ + std::string takeFallbackBuffer() { + return std::move(fallback); + } + /* Maximum number of trailer fields surfaced to JS (the section size cap * already bounds memory; this matches the regular-header count cap). */ static constexpr unsigned MAX_TRAILER_FIELDS = UWS_HTTP_MAX_HEADERS_COUNT - 1; diff --git a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts index 846ebadedc49..a1f48dbd7dd1 100644 --- a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts +++ b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts @@ -735,6 +735,107 @@ test.each(stoppedRequests)("server.stop(true) inside the handler of %s aborts it await stopped!; }); +// A request head that arrives split over two reads is parsed out of the HTTP +// parser's per-socket fallback buffer, and the uWS request the dispatch holds +// views into that buffer. server.stop(true) inside the handler closes the +// request's own socket right there, and the close destructed the parser with +// its buffer. Everything that materialises the headers after that read freed +// memory: `req.headers` inside the handler, and the snapshot the server takes +// itself when an async handler ends the dispatch. Under a sanitizer it is a +// heap-use-after-free; without one the headers come back as the bytes of +// whatever allocation took the block over. +test.each(["lazy", "async"])( + "a request head split over two reads survives server.stop(true) in the handler (%s headers)", + async mode => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { connect } from "node:net"; + import { existsSync } from "node:fs"; + + const { promise: printed, resolve: finish } = Promise.withResolvers(); + + function report(req) { + const got = {}; + for (const [key, value] of req.headers) got[key] = value; + console.log([got["x-mark"], String(got["x-pad"]?.length), Object.keys(got).sort().join(",")].join("|")); + finish(); + } + + // Native allocations of many sizes. On a build with no sanitizer they + // take over the freed block, so a view into it reads their bytes. + function churn() { + for (let n = 64; n < 1200; n += 8) { + const name = Buffer.alloc(n, 0x5a).toString(); + try { Bun.resolveSync("./" + name, "/tmp"); } catch {} + try { existsSync("/tmp/" + name); } catch {} + } + } + + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + fetch(req, srv) { + if (req.url.endsWith("/barrier")) return new Response("ok"); + srv.stop(true); + churn(); + if (process.env.SPLIT_HEAD_MODE === "lazy") { + report(req); + return new Response("x"); + } + return (async () => { + // Only to make the handler return a pending promise, so that + // the server snapshots the headers itself as the dispatch ends. + await Bun.sleep(1); + report(req); + return new Response("x"); + })(); + }, + }); + + const head = + "GET /a HTTP/1.1\\r\\nHost: x\\r\\nX-Pad: " + Buffer.alloc(300, 0x70).toString() + + "\\r\\nX-Mark: " + Buffer.alloc(40, 0x4d).toString() + "\\r\\n\\r\\n"; + const socket = connect(server.port, "127.0.0.1"); + socket.on("error", () => {}); + await new Promise(resolve => socket.once("connect", resolve)); + socket.setNoDelay(true); + // One read that holds a whole request plus the first 22 bytes of the + // next head: the server answers the first and parks the rest in the + // parser's fallback buffer. + const answered = new Promise(resolve => socket.once("data", resolve)); + socket.write("GET /barrier HTTP/1.1\\r\\nHost: x\\r\\n\\r\\n" + head.slice(0, 22)); + await answered; + // The rest of the head. This request is parsed out of the buffer. + socket.write(head.slice(22)); + await printed; + socket.destroy(); + `, + ], + env: { + ...bunEnv, + SPLIT_HEAD_MODE: mode, + // symbolize=0 so an unfixed build's ASAN abort exits promptly instead + // of spending seconds in llvm-symbolizer. + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"), + }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout: stdout.trim(), stderr: stderr.trim() }).toEqual({ + stdout: `${Buffer.alloc(40, 0x4d).toString()}|300|host,x-mark,x-pad`, + stderr: "", + }); + expect(exitCode).toBe(0); + }, +); + // A Response the server will never render still owns a body stream that // somebody produces into. The server has to cancel it, like a client abort // after the stream was attached does, or the producer waits for a pull that From 0686ed21d681daaa6b8ccaa6591e238a050863aa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 15 Sep 2026 10:50:25 +0000 Subject: [PATCH 2/5] test: keep the split-head test's allocation churn below the macOS path limit The helper that churns native allocations passed names of up to 1192 bytes to Bun.resolveSync. macOS caps a path at 1024 bytes, and a resolve whose path plus the probed extension passes that cap panics in the resolver, so the child process crashed on both darwin lanes. Names now stop at 760 bytes, which still covers the size of the freed block. The two cases also run concurrently. --- .../bun/http/serve-pending-promise-abort-leak.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts index a1f48dbd7dd1..42fe9757b49c 100644 --- a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts +++ b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts @@ -744,7 +744,7 @@ test.each(stoppedRequests)("server.stop(true) inside the handler of %s aborts it // itself when an async handler ends the dispatch. Under a sanitizer it is a // heap-use-after-free; without one the headers come back as the bytes of // whatever allocation took the block over. -test.each(["lazy", "async"])( +test.concurrent.each(["lazy", "async"])( "a request head split over two reads survives server.stop(true) in the handler (%s headers)", async mode => { await using proc = Bun.spawn({ @@ -764,10 +764,12 @@ test.each(["lazy", "async"])( finish(); } - // Native allocations of many sizes. On a build with no sanitizer they - // take over the freed block, so a view into it reads their bytes. + // Native allocations of many sizes around the size of the freed block + // (about 430 bytes). On a build with no sanitizer they take it over, + // so a view into it reads their bytes. The names stay far below the + // shortest platform path limit (1024 bytes on macOS). function churn() { - for (let n = 64; n < 1200; n += 8) { + for (let n = 64; n < 768; n += 8) { const name = Buffer.alloc(n, 0x5a).toString(); try { Bun.resolveSync("./" + name, "/tmp"); } catch {} try { existsSync("/tmp/" + name); } catch {} From 1a42be7934c9ee212e57512e1550aa21de1231b1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 03:24:02 +0000 Subject: [PATCH 3/5] Own the split head in the parse frame instead of hooking onClose A request head that arrives split over two reads is parsed in place out of HttpParser::fallback, and the dispatched uWS HttpRequest holds views into it. consumePostPadded now moves that buffer into a frame-local string for the dispatch and puts it back on the short-read path, so the frame that creates the views owns the bytes. This covers every site that destructs the parser mid-dispatch (onClose, HttpResponse::upgrade, the HTTP/2 handoff), not only the close, and it adds no work to a read that carries a whole head. The onClose hook, the HttpContextData holder and takeFallbackBuffer() are gone. The new test case needs no server API call: a split head plus a declared body that never arrives plus Connection: close closes the socket from the response completion inside the dispatch. --- packages/bun-uws/src/HttpContext.h | 21 ------ packages/bun-uws/src/HttpContextData.h | 7 -- packages/bun-uws/src/HttpParser.h | 31 ++++---- .../serve-pending-promise-abort-leak.test.ts | 73 +++++++++++++++++++ 4 files changed, 87 insertions(+), 45 deletions(-) diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 67235805847e..22b93c4e64b3 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -248,19 +248,6 @@ struct HttpContext { /* Call filter */ HttpContextData *httpContextData = getSocketContextDataS(s); - /* This socket is closing from inside its own parse frame: a request - * handler called server.stop(true), or a response completed and the - * connection close gate fired. The HttpRequest that frame is - * dispatching holds string_views into the parser's fallback buffer when - * its head arrived split over two reads, and the handler can still - * materialise the headers from it (lazily, or through the post-handler - * snapshot). Give those bytes to the parse frame, which outlives every - * dispatch that can view them, instead of freeing them with the parser - * below. */ - if (httpContextData->parsingSocket == s && httpContextData->parsedFallbackHolder) { - *httpContextData->parsedFallbackHolder = httpResponseData->takeFallbackBuffer(); - } - bool nodeHttpTunnelAfterBody = false; if constexpr (IsNodeHttp) nodeHttpTunnelAfterBody = (httpResponseData->state & HttpResponseData::HTTP_NODE_TUNNEL_AFTER_BODY) != 0; if(httpResponseData->isConnectRequest || nodeHttpTunnelAfterBody) { @@ -405,13 +392,6 @@ struct HttpContext { httpContextData->parsingSocket = s; httpResponseData->isIdle = false; - /* Owns the parser's fallback buffer once a close below hands it over - * (see onClose). It is freed when this frame returns, after the last - * dispatch that can hold string_views into it. */ - std::string parsedFallback; - std::string *prevParsedFallbackHolder = httpContextData->parsedFallbackHolder; - httpContextData->parsedFallbackHolder = &parsedFallback; - /* node:http compat: maintain the headers/request timeout window (see * the requestHandler/dataHandler hooks and the post-parse check). */ const bool trackNodeHttpTimings = IsNodeHttp && !httpResponseData->isConnectRequest; @@ -671,7 +651,6 @@ struct HttpContext { /* Mark that we are no longer parsing Http */ httpContextData->flags.isParsingHttp = false; httpContextData->parsingSocket = prevParsingSocket; - httpContextData->parsedFallbackHolder = prevParsedFallbackHolder; /* If we got fullptr that means the parser wants us to close the socket from error (same as calling the errorHandler) */ if (httpErrorStatusCode) { /* node:http compat: parse errors surface as the server's 'clientError' diff --git a/packages/bun-uws/src/HttpContextData.h b/packages/bun-uws/src/HttpContextData.h index 810fca444b55..5239d62a1ebd 100644 --- a/packages/bun-uws/src/HttpContextData.h +++ b/packages/bun-uws/src/HttpContextData.h @@ -77,13 +77,6 @@ struct alignas(16) HttpContextData { * checks the parsed socket. */ struct us_socket_t *parsingSocket = nullptr; - /* Where onClose parks the fallback buffer of parsingSocket's parser, owned - * by that parse frame in onData from then on. A close inside the dispatch - * (server.stop(true) in a request handler) destructs the parser while the - * HttpRequest being dispatched still holds string_views into the buffer. - * nullptr outside a parse. */ - std::string *parsedFallbackHolder = nullptr; - /* This is the default router for default SNI or non-SSL */ HttpRouter router; void *upgradedWebSocket = nullptr; diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index 98ba91bf3d0f..db46e5b16437 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -484,18 +484,6 @@ struct HttpResponseData; return remainingStreamingBytes != 0; } - /* Hands the fallback buffer to the caller, which becomes its owner. A - * request head that arrived split over two reads is parsed out of this - * buffer, so the HttpRequest the parse loop is dispatching right now - * holds string_views into it. HttpContext::onClose calls this before it - * destructs the parser under that dispatch. The move keeps the heap - * block, so the views stay valid: consumePostPadded reserves at least - * MINIMUM_HTTP_POST_PADDING bytes before it parses out of the buffer, - * which is past every short string optimization threshold. */ - std::string takeFallbackBuffer() { - return std::move(fallback); - } - /* Maximum number of trailer fields surfaced to JS (the section size cap * already bounds memory; this matches the regular-header count cap). */ static constexpr unsigned MAX_TRAILER_FIELDS = UWS_HTTP_MAX_HEADERS_COUNT - 1; @@ -1499,13 +1487,21 @@ struct HttpResponseData; size_t maxCopyDistance = std::min(maxFallbackSize - fallback.length(), (size_t) length); + /* This frame owns the buffer while the head is parsed out of it. The + * dispatch below can destruct this parser (the socket closes or is + * upgraded inside the handler) while the HttpRequest it was given + * still holds string_views into these bytes. */ + std::string head = std::move(fallback); + fallback.clear(); + /* We don't want fallback to be short string optimized, since we want to move it */ - fallback.reserve(fallback.length() + maxCopyDistance + std::max(MINIMUM_HTTP_POST_PADDING, sizeof(std::string))); - fallback.append(data, maxCopyDistance); + head.reserve(head.length() + maxCopyDistance + std::max(MINIMUM_HTTP_POST_PADDING, sizeof(std::string))); + head.append(data, maxCopyDistance); // break here on break - HttpParserResult consumed = fenceAndConsumePostPadded(maxHeaderSize, isConnectRequest, requireHostHeader, useStrictMethodValidation, useInsecureHTTPParser, useLenientTransferEncoding, nodeHttpRequestTrailers, chunkedExtensionsByteCount, fallback.data(), (unsigned int) fallback.length(), user, &req, requestHandler, dataHandler); - /* Return data will be different than user if we are upgraded to WebSocket or have an error */ + HttpParserResult consumed = fenceAndConsumePostPadded(maxHeaderSize, isConnectRequest, requireHostHeader, useStrictMethodValidation, useInsecureHTTPParser, useLenientTransferEncoding, nodeHttpRequestTrailers, chunkedExtensionsByteCount, head.data(), (unsigned int) head.length(), user, &req, requestHandler, dataHandler); + /* Return data will be different than user if we are upgraded to WebSocket or have an error. + * The parser can be gone by now: do not touch a member before this return. */ if (consumed.returnedData != user) { return consumed; } @@ -1516,7 +1512,6 @@ struct HttpResponseData; /* This logic assumes that we consumed everything in fallback buffer. * This is critically important, as we will get an integer overflow in case * of "had" being larger than what we consumed, and that we would drop data */ - fallback.clear(); data += consumedBytes - had; length -= consumedBytes - had; @@ -1579,6 +1574,8 @@ struct HttpResponseData; } } else { + /* Short read: nothing was dispatched, keep accumulating. */ + fallback = std::move(head); if (fallback.length() == maxFallbackSize) { return HttpParserResult::error(HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE, HTTP_PARSER_ERROR_REQUEST_HEADER_FIELDS_TOO_LARGE); } diff --git a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts index 42fe9757b49c..73003b7b2aca 100644 --- a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts +++ b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts @@ -838,6 +838,79 @@ test.concurrent.each(["lazy", "async"])( }, ); +// The same buffer, reached by the client alone: no server API call and no +// nested event loop. The head is split, the request declares a body it never +// sends, and `Connection: close` makes the completed response close the socket +// inside the dispatch. The pending `req.text()` then rejects, and its handler +// reads a url and headers that the close already freed. +test.concurrent("a split request head survives a Connection: close response on an unfinished body", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { connect } from "node:net"; + + const { promise: rejected, resolve: finish } = Promise.withResolvers(); + + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + fetch(req) { + if (req.method === "GET") return new Response("ok"); + req.text().then( + () => { console.log("resolved, expected a reject"); finish(); }, + error => { + const got = {}; + for (const [key, value] of req.headers) got[key] = value; + console.log([error.name, JSON.stringify(req.url), String(got["x-pad"]?.length), Object.keys(got).sort().join(",")].join("|")); + finish(); + }, + ); + // Larger than the cork buffer, so the response completes and the + // close gate fires while this dispatch is still on the stack. + return new Response(Buffer.alloc(20 * 1024, 0x61).toString()); + }, + }); + + const head = + "POST /a HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\nContent-Length: 10\\r\\nX-Pad: " + + Buffer.alloc(300, 0x70).toString() + "\\r\\n\\r\\n"; + const socket = connect(server.port, "127.0.0.1"); + socket.on("error", () => {}); + await new Promise(resolve => socket.once("connect", resolve)); + socket.setNoDelay(true); + const answered = new Promise(resolve => socket.once("data", resolve)); + // One read: a whole GET plus the first 22 bytes of the POST head, which + // park in the parser's fallback buffer. + socket.write("GET /barrier HTTP/1.1\\r\\nHost: x\\r\\n\\r\\n" + head.slice(0, 22)); + await answered; + socket.on("data", () => {}); + // The rest of the head, but never the 10 body bytes it declares. + socket.write(head.slice(22)); + await rejected; + socket.destroy(); + server.stop(true); + `, + ], + env: { + ...bunEnv, + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"), + }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout: stdout.trim(), stderr: stderr.trim() }).toEqual({ + stdout: `AbortError|"http://x/a"|300|connection,content-length,host,x-pad`, + stderr: "", + }); + expect(exitCode).toBe(0); +}); + // A Response the server will never render still owns a body stream that // somebody produces into. The server has to cancel it, like a client abort // after the stream was attached does, or the producer waits for a pull that From 4247cf02c380bcbbf5db38ebf1be6bebd4be0066 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 20:49:12 +0000 Subject: [PATCH 4/5] test: run the split-head cases from fixture files with bunRun The three cases spawned bun with an inline -e script. They now live in serve-split-head-stop-fixture.ts and serve-split-head-close-fixture.ts, and the test runs them with bunRun and asserts with toSpawn. --- .../serve-pending-promise-abort-leak.test.ts | 168 ++---------------- .../http/serve-split-head-close-fixture.ts | 56 ++++++ .../bun/http/serve-split-head-stop-fixture.ts | 77 ++++++++ 3 files changed, 148 insertions(+), 153 deletions(-) create mode 100644 test/js/bun/http/serve-split-head-close-fixture.ts create mode 100644 test/js/bun/http/serve-split-head-stop-fixture.ts diff --git a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts index 4dfa34f1cfe3..9e6810a663ae 100644 --- a/test/js/bun/http/serve-pending-promise-abort-leak.test.ts +++ b/test/js/bun/http/serve-pending-promise-abort-leak.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, bunRun } from "harness"; import { connect } from "node:net"; import { join } from "node:path"; @@ -12,6 +12,10 @@ async function stopAndAssertDrained(server: ReturnType) { expect(server.pendingRequests).toBe(0); } +// symbolize=0 so an unfixed build's ASAN abort exits promptly instead of +// spending seconds in llvm-symbolizer. +const asanOptionsWithoutSymbolizer = [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"); + test.each([false, true])( "RequestContext is freed when client aborts before Promise settles (http2: %p)", async http2 => { @@ -749,94 +753,12 @@ test.each(stoppedRequests)("server.stop(true) inside the handler of %s aborts it test.concurrent.each(["lazy", "async"])( "a request head split over two reads survives server.stop(true) in the handler (%s headers)", async mode => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` - import { connect } from "node:net"; - import { existsSync } from "node:fs"; - - const { promise: printed, resolve: finish } = Promise.withResolvers(); - - function report(req) { - const got = {}; - for (const [key, value] of req.headers) got[key] = value; - console.log([got["x-mark"], String(got["x-pad"]?.length), Object.keys(got).sort().join(",")].join("|")); - finish(); - } - - // Native allocations of many sizes around the size of the freed block - // (about 430 bytes). On a build with no sanitizer they take it over, - // so a view into it reads their bytes. The names stay far below the - // shortest platform path limit (1024 bytes on macOS). - function churn() { - for (let n = 64; n < 768; n += 8) { - const name = Buffer.alloc(n, 0x5a).toString(); - try { Bun.resolveSync("./" + name, "/tmp"); } catch {} - try { existsSync("/tmp/" + name); } catch {} - } - } - - const server = Bun.serve({ - port: 0, - hostname: "127.0.0.1", - idleTimeout: 0, - fetch(req, srv) { - if (req.url.endsWith("/barrier")) return new Response("ok"); - srv.stop(true); - churn(); - if (process.env.SPLIT_HEAD_MODE === "lazy") { - report(req); - return new Response("x"); - } - return (async () => { - // Only to make the handler return a pending promise, so that - // the server snapshots the headers itself as the dispatch ends. - await Bun.sleep(1); - report(req); - return new Response("x"); - })(); - }, - }); - - const head = - "GET /a HTTP/1.1\\r\\nHost: x\\r\\nX-Pad: " + Buffer.alloc(300, 0x70).toString() + - "\\r\\nX-Mark: " + Buffer.alloc(40, 0x4d).toString() + "\\r\\n\\r\\n"; - const socket = connect(server.port, "127.0.0.1"); - socket.on("error", () => {}); - await new Promise(resolve => socket.once("connect", resolve)); - socket.setNoDelay(true); - // One read that holds a whole request plus the first 22 bytes of the - // next head: the server answers the first and parks the rest in the - // parser's fallback buffer. - const answered = new Promise(resolve => socket.once("data", resolve)); - socket.write("GET /barrier HTTP/1.1\\r\\nHost: x\\r\\n\\r\\n" + head.slice(0, 22)); - await answered; - // The rest of the head. This request is parsed out of the buffer. - socket.write(head.slice(22)); - await printed; - socket.destroy(); - `, - ], - env: { - ...bunEnv, + expect( + await bunRun(join(import.meta.dir, "serve-split-head-stop-fixture.ts"), { SPLIT_HEAD_MODE: mode, - // symbolize=0 so an unfixed build's ASAN abort exits promptly instead - // of spending seconds in llvm-symbolizer. - ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"), - }, - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - - expect({ stdout: stdout.trim(), stderr: stderr.trim() }).toEqual({ - stdout: `${Buffer.alloc(40, 0x4d).toString()}|300|host,x-mark,x-pad`, - stderr: "", - }); - expect(exitCode).toBe(0); + ASAN_OPTIONS: asanOptionsWithoutSymbolizer, + }), + ).toSpawn(`${Buffer.alloc(40, 0x4d).toString()}|300|host,x-mark,x-pad`); }, ); @@ -846,71 +768,11 @@ test.concurrent.each(["lazy", "async"])( // inside the dispatch. The pending `req.text()` then rejects, and its handler // reads a url and headers that the close already freed. test.concurrent("a split request head survives a Connection: close response on an unfinished body", async () => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` - import { connect } from "node:net"; - - const { promise: rejected, resolve: finish } = Promise.withResolvers(); - - const server = Bun.serve({ - port: 0, - hostname: "127.0.0.1", - idleTimeout: 0, - fetch(req) { - if (req.method === "GET") return new Response("ok"); - req.text().then( - () => { console.log("resolved, expected a reject"); finish(); }, - error => { - const got = {}; - for (const [key, value] of req.headers) got[key] = value; - console.log([error.name, JSON.stringify(req.url), String(got["x-pad"]?.length), Object.keys(got).sort().join(",")].join("|")); - finish(); - }, - ); - // Larger than the cork buffer, so the response completes and the - // close gate fires while this dispatch is still on the stack. - return new Response(Buffer.alloc(20 * 1024, 0x61).toString()); - }, - }); - - const head = - "POST /a HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\nContent-Length: 10\\r\\nX-Pad: " + - Buffer.alloc(300, 0x70).toString() + "\\r\\n\\r\\n"; - const socket = connect(server.port, "127.0.0.1"); - socket.on("error", () => {}); - await new Promise(resolve => socket.once("connect", resolve)); - socket.setNoDelay(true); - const answered = new Promise(resolve => socket.once("data", resolve)); - // One read: a whole GET plus the first 22 bytes of the POST head, which - // park in the parser's fallback buffer. - socket.write("GET /barrier HTTP/1.1\\r\\nHost: x\\r\\n\\r\\n" + head.slice(0, 22)); - await answered; - socket.on("data", () => {}); - // The rest of the head, but never the 10 body bytes it declares. - socket.write(head.slice(22)); - await rejected; - socket.destroy(); - server.stop(true); - `, - ], - env: { - ...bunEnv, - ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"), - }, - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - - expect({ stdout: stdout.trim(), stderr: stderr.trim() }).toEqual({ - stdout: `AbortError|"http://x/a"|300|connection,content-length,host,x-pad`, - stderr: "", - }); - expect(exitCode).toBe(0); + expect( + await bunRun(join(import.meta.dir, "serve-split-head-close-fixture.ts"), { + ASAN_OPTIONS: asanOptionsWithoutSymbolizer, + }), + ).toSpawn(`AbortError|"http://x/a"|300|connection,content-length,host,x-pad`); }); // A Response the server will never render still owns a body stream that diff --git a/test/js/bun/http/serve-split-head-close-fixture.ts b/test/js/bun/http/serve-split-head-close-fixture.ts new file mode 100644 index 000000000000..eedb70b78a7e --- /dev/null +++ b/test/js/bun/http/serve-split-head-close-fixture.ts @@ -0,0 +1,56 @@ +// A request whose head arrives split over two reads, reached by the client +// alone: it declares a body it never sends, and `Connection: close` makes the +// completed response close the socket inside the dispatch. Prints the reject +// reason of the pending `req.text()`, then the url, the x-pad length and the +// sorted header names that its handler reads. +import { connect } from "node:net"; + +const { promise: rejected, resolve: finish } = Promise.withResolvers(); + +const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + fetch(req) { + if (req.method === "GET") return new Response("ok"); + req.text().then( + () => { + console.log("resolved, expected a reject"); + finish(); + }, + error => { + const got: Record = {}; + for (const [key, value] of req.headers) got[key] = value; + console.log( + [error.name, JSON.stringify(req.url), String(got["x-pad"]?.length), Object.keys(got).sort().join(",")].join( + "|", + ), + ); + finish(); + }, + ); + // Larger than the cork buffer, so the response completes and the close + // gate fires while this dispatch is still on the stack. + return new Response(Buffer.alloc(20 * 1024, 0x61).toString()); + }, +}); + +const head = + "POST /a HTTP/1.1\r\nHost: x\r\nConnection: close\r\nContent-Length: 10\r\nX-Pad: " + + Buffer.alloc(300, 0x70).toString() + + "\r\n\r\n"; +const socket = connect(server.port, "127.0.0.1"); +socket.on("error", () => {}); +await new Promise(resolve => socket.once("connect", resolve)); +socket.setNoDelay(true); +const answered = new Promise(resolve => socket.once("data", resolve)); +// One read: a whole GET plus the first 22 bytes of the POST head, which park +// in the parser's fallback buffer. +socket.write("GET /barrier HTTP/1.1\r\nHost: x\r\n\r\n" + head.slice(0, 22)); +await answered; +socket.on("data", () => {}); +// The rest of the head, but never the 10 body bytes it declares. +socket.write(head.slice(22)); +await rejected; +socket.destroy(); +server.stop(true); diff --git a/test/js/bun/http/serve-split-head-stop-fixture.ts b/test/js/bun/http/serve-split-head-stop-fixture.ts new file mode 100644 index 000000000000..d6886f4391bb --- /dev/null +++ b/test/js/bun/http/serve-split-head-stop-fixture.ts @@ -0,0 +1,77 @@ +// A request whose head arrives split over two reads, and a handler that calls +// server.stop(true). Prints the x-mark value, the x-pad length and the sorted +// header names that the handler reads afterwards. +// +// SPLIT_HEAD_MODE=lazy reads `req.headers` in the synchronous part of the +// handler. Any other value reads them after an await, which returns the +// snapshot the server takes itself when the dispatch ends. +import { existsSync } from "node:fs"; +import { connect } from "node:net"; + +const { promise: printed, resolve: finish } = Promise.withResolvers(); + +function report(req: Request) { + const got: Record = {}; + for (const [key, value] of req.headers) got[key] = value; + console.log([got["x-mark"], String(got["x-pad"]?.length), Object.keys(got).sort().join(",")].join("|")); + finish(); +} + +// Native allocations of many sizes around the size of the freed block (about +// 430 bytes). On a build with no sanitizer they take it over, so a view into +// it reads their bytes. The names stay far below the shortest platform path +// limit (1024 bytes on macOS). +function churn() { + for (let n = 64; n < 768; n += 8) { + const name = Buffer.alloc(n, 0x5a).toString(); + try { + Bun.resolveSync("./" + name, "/tmp"); + } catch {} + try { + existsSync("/tmp/" + name); + } catch {} + } +} + +const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 0, + fetch(req, srv) { + if (req.url.endsWith("/barrier")) return new Response("ok"); + srv.stop(true); + churn(); + if (process.env.SPLIT_HEAD_MODE === "lazy") { + report(req); + return new Response("x"); + } + return (async () => { + // Only to make the handler return a pending promise, so that the server + // snapshots the headers itself as the dispatch ends. + await Bun.sleep(1); + report(req); + return new Response("x"); + })(); + }, +}); + +const head = + "GET /a HTTP/1.1\r\nHost: x\r\nX-Pad: " + + Buffer.alloc(300, 0x70).toString() + + "\r\nX-Mark: " + + Buffer.alloc(40, 0x4d).toString() + + "\r\n\r\n"; +const socket = connect(server.port, "127.0.0.1"); +socket.on("error", () => {}); +await new Promise(resolve => socket.once("connect", resolve)); +socket.setNoDelay(true); +// One read that holds a whole request plus the first 22 bytes of the next +// head: the server answers the first and parks the rest in the parser's +// fallback buffer. +const answered = new Promise(resolve => socket.once("data", resolve)); +socket.write("GET /barrier HTTP/1.1\r\nHost: x\r\n\r\n" + head.slice(0, 22)); +await answered; +// The rest of the head. This request is parsed out of the buffer. +socket.write(head.slice(22)); +await printed; +socket.destroy(); From e09f98388e5c4eebea1cb56daeef66a94c33f420 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:32:11 +0000 Subject: [PATCH 5/5] ci: retrigger