From fe5c433f13fa761a6f1951f996f3b961b1872da9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:02:13 +0000 Subject: [PATCH 01/22] Bun.serve: hold a pipelined request until the response ahead of it completes A request head parsed while the connection's response was still pending (async handler, or a buffered body whose tryEnd tail was still draining through onWritable) made uWS close the connection: the in-flight response was cut off and the pipelined request was never answered. The parser now stops at the request boundary instead and parks the rest of the read verbatim (the mechanism node:http compat already used for flood prevention, shared and renamed). onData derives the park flag from the response state at entry and after each request's body fin, pauses reads while anything is parked, and markDone() arms one writable dispatch; HttpContext::onWritable replays the parked bytes through onData once the response is complete and drained, at which point nothing of that response is on the stack any more. A request behind one that will close the connection (Connection: close, HTTP/1.0) is parked too and goes down with the socket; previously it was dispatched and kept the connection open. us_socket_request_writable replaces the two identical copies of the arm-a-writable-dispatch helper (sendfile's us_socket_sendfile_needs_more and us_socket_mark_needs_more_not_ssl) and is what markDone() uses. --- packages/bun-usockets/src/libusockets.h | 8 +- packages/bun-usockets/src/socket.c | 9 + packages/bun-uws/src/HttpContext.h | 113 ++++- packages/bun-uws/src/HttpParser.h | 39 +- packages/bun-uws/src/HttpResponseData.h | 11 + .../bindings/node/JSNodeHTTPServerSocket.cpp | 22 +- src/uws_sys/Response.rs | 4 +- src/uws_sys/libuwsockets.cpp | 22 - src/uws_sys/socket.rs | 2 +- src/uws_sys/us_socket_t.rs | 6 +- test/js/bun/http/bun-serve-pipelining.test.ts | 447 ++++++++++++++++++ 11 files changed, 617 insertions(+), 66 deletions(-) create mode 100644 test/js/bun/http/bun-serve-pipelining.test.ts diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index d843768736bb..de133d8dadc0 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -700,7 +700,13 @@ void us_socket_local_address(us_socket_r s, char *nonnull_arg buf, int *nonnull_ struct us_socket_t *us_socket_detach(us_socket_r s) nonnull_fn_decl; int us_socket_ipc_write_fd(us_socket_r s, const char *data, int length, int fd) nonnull_fn_decl; -void us_socket_sendfile_needs_more(us_socket_r s) nonnull_fn_decl; +/* Have on_writable dispatched once the socket is writable, as if a + * us_socket_write() had just come up short. For progress that has to be made + * from on_writable but was not queued through us_socket_write(): a sendfile + * that hit EAGAIN, HTTP request bytes parked behind a response that just + * completed. Safe to call from inside on_writable itself. Leaves a paused + * socket's read side alone. */ +void us_socket_request_writable(us_socket_r s) nonnull_fn_decl; void *us_listen_socket_ext(struct us_listen_socket_t *ls) nonnull_fn_decl; LIBUS_SOCKET_DESCRIPTOR us_listen_socket_get_fd(struct us_listen_socket_t *ls) nonnull_fn_decl; int us_listen_socket_port(struct us_listen_socket_t *ls) nonnull_fn_decl; diff --git a/packages/bun-usockets/src/socket.c b/packages/bun-usockets/src/socket.c index 20f351f74b6c..c49d406784e4 100644 --- a/packages/bun-usockets/src/socket.c +++ b/packages/bun-usockets/src/socket.c @@ -408,6 +408,15 @@ static void us_internal_rearm_writable(struct us_socket_t *s) { LIBUS_SOCKET_WRITABLE | ((s->flags.is_paused || s->read_eof) ? 0 : LIBUS_SOCKET_READABLE)); } +/* See libusockets.h. last_write_failed is what keeps the loop polling writable + * past the end of the dispatch this may be called from (loop.c drops the + * interest again after an on_writable that left it clear). */ +void us_socket_request_writable(struct us_socket_t *s) { + if (us_socket_is_closed(s)) return; + s->flags.last_write_failed = 1; + us_internal_rearm_writable(s); +} + /* See libusockets.h: whether a zero-progress write on a writable event proves * the peer is gone. Only the libuv backend has to ask the kernel. */ int us_socket_stalled_write_means_peer_gone(struct us_socket_t *s) { diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index c3fbda0d72c7..245e3136047a 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -34,6 +34,7 @@ #include #include #include +#include extern "C" void Bun__NodeHTTP__onReadsResumable(int ssl, struct us_socket_t *s); @@ -117,11 +118,20 @@ struct HttpContext { static unsigned char socketKind() { return SSL ? US_SOCKET_KIND_UWS_HTTP_TLS : US_SOCKET_KIND_UWS_HTTP; } public: - /* node:http flood prevention: re-feed parked request bytes through the same - * parse path fresh socket data takes. The caller guarantees the buffer has - * LIBUS_RECV_BUFFER_PADDING of writable slack past `length`. */ - static us_socket_t *feedNodeHttpData(us_socket_t *s, char *data, int length) { - return onData(s, data, length); + /* Re-feed the bytes HttpParser parked (parkedRequestBytes) through the same + * parse path fresh socket data takes. Takes the parked bytes, so a dispatch + * during the replay that parks again starts a fresh batch behind them. The + * caller has already decided what to do about the paused read side. Returns + * what onData returns: the socket, closed, or the WebSocket it was upgraded + * into. */ + template + static us_socket_t *replayParkedRequestBytes(us_socket_t *s) { + auto *httpResponseData = reinterpret_cast *>(us_socket_ext(s)); + WTF::Vector parked = std::exchange(httpResponseData->parkedRequestBytes, {}); + size_t length = parked.size(); + /* The parser fences the buffer by writing past its logical end. */ + parked.grow(length + LIBUS_RECV_BUFFER_PADDING); + return onData(s, parked.mutableSpan().data(), static_cast(length)); } us_socket_group_t *getSocketGroup() { @@ -299,6 +309,19 @@ struct HttpContext { return us_socket_close(s, 0, nullptr); } + /* Bun.serve: whether the next request head on this connection must be parked + * instead of parsed (HttpParser::parkAtNextBoundary). Either the connection's + * one response slot is still taken, or the response in it is going to close + * the connection (Connection: close or HTTP/1.0 request, close requested by + * the app): nothing received behind such a request may be processed + * (RFC 9112 9.6), and parking it lets the close-after-drain gates discard it + * with the socket. Such bytes are never replayed: the only replay site runs + * behind onWritable's close gate, which fires on exactly the conditions the + * replay needs. */ + static bool cannotDispatchAnotherRequest(HttpResponseData *httpResponseData) { + return (httpResponseData->state & (HttpResponseData::HTTP_RESPONSE_PENDING | HttpResponseData::HTTP_CONNECTION_CLOSE)) != 0; + } + template static us_socket_t *onData(us_socket_t *s, char *data, int length) { // ref the socket to make sure we process it entirely before it is closed @@ -392,6 +415,16 @@ struct HttpContext { httpContextData->parsingSocket = s; httpResponseData->isIdle = false; + /* Bun.serve: requests pipelined behind a response that is still pending + * (async handler, or a body the socket is still draining) are parked at + * the next request boundary and replayed from onWritable once it has + * completed. Re-derived on every read: a keep-alive request arriving + * after the response completed takes the ordinary path. Body bytes of the + * pending request itself never reach a boundary, so they are unaffected. */ + if constexpr (!IsNodeHttp) { + httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); + } + /* 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; @@ -440,13 +473,17 @@ struct HttpContext { nodeHttpResponseData->headersCompleted = true; } - /* Are we not ready for another request yet? Terminate the connection. - * Important for denying async pipelining until, if ever, we want to support it. - * Otherwise requests can get mixed up on the same connection. We still support sync pipelining. */ + /* Is the previous response on this connection still in flight? */ bool hasQueuedPipelinedResponses = false; if constexpr (IsNodeHttp) hasQueuedPipelinedResponses = httpResponseData->nodeHttpQueuedPipelinedCount > 0; if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) || hasQueuedPipelinedResponses) { if constexpr (!IsNodeHttp) { + /* Bun.serve has one response slot per connection, so the parser + * parks a head that arrives while it is taken (parkAtNextBoundary, + * maintained by onData) instead of getting here. Dispatching onto + * the in-flight response would interleave the two responses on the + * wire; closing is the backstop. */ + ASSERT_NOT_REACHED(); us_socket_close((us_socket_t *) s, 0, nullptr); return nullptr; } else { @@ -470,7 +507,7 @@ struct HttpContext { httpResponseData->state |= HttpResponseData::HTTP_NODE_READS_PAUSED; /* Also stop the request loop over the buffer being parsed * right now — pausing the socket alone cannot bound it. */ - httpResponseData->nodeHttpParkAtNextBoundary = true; + httpResponseData->parkAtNextBoundary = true; ((HttpResponse *) s)->pause(); } } @@ -503,7 +540,7 @@ struct HttpContext { * and park already-received requests. No already-paused guard (replay clears the park flag only). */ if (((AsyncSocket *) s)->getBufferedAmount() > 0) { httpResponseData->state |= HttpResponseData::HTTP_NODE_READS_PAUSED; - httpResponseData->nodeHttpParkAtNextBoundary = true; + httpResponseData->parkAtNextBoundary = true; ((HttpResponse *) s)->pause(); } } @@ -629,6 +666,18 @@ struct HttpContext { httpResponseData->inStream = nullptr; } } + + /* Bun.serve: the request message is complete (a bodiless request gets an + * empty fin right after dispatch), so the next request boundary is what + * the parser reaches next. The handler may have completed the response + * anywhere up to here, synchronously or from inside this body callback, + * which is why the decision to parse or park what follows is taken now + * and not at dispatch. */ + if constexpr (!IsNodeHttp) { + if (fin) { + httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); + } + } return user; }); @@ -693,6 +742,23 @@ struct HttpContext { ((HttpResponse *) s)->resetTimeout(); } + /* Bun.serve: requests are parked on this connection. Invariant kept + * here and in markDone(): reads are paused while anything is parked + * (bounding it to one recv), and a replaying writable dispatch + * (onWritable) is armed as soon as the response ahead of them is + * complete, whichever of the two happened last. AsyncSocket::pause + * rather than HttpResponse::pause: the in-flight response's timeout + * must stay armed against a peer that pipelines and then stops + * reading. */ + if constexpr (!IsNodeHttp) { + if (!httpResponseData->parkedRequestBytes.isEmpty()) [[unlikely]] { + ((AsyncSocket *) s)->pause(); + if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { + us_socket_request_writable(s); + } + } + } + /* We need to check if we should close this socket here now */ if (httpResponseData->shouldCloseConnection()) { if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { @@ -856,9 +922,36 @@ struct HttpContext { /* Expect another writable event, or another request within the timeout */ reinterpret_cast *>(s)->resetTimeout(); + if constexpr (!IsNodeHttp) { + return replayParkedRequestsIfResponseComplete(s); + } return s; } + /* Bun.serve pipelining, replay half; the tail of every writable dispatch. + * Gets here either because the response the parked requests were waiting on + * completed inside callOnWritable above (a tryEnd tail draining), or via the + * dispatch markDone() / onData arm when it completed anywhere else. Nothing + * of the completed response is on the stack at this point, so the replayed + * request can take over the connection's response slot. Waits for the + * completed response's bytes to leave the buffer: the next dispatch is + * already owed while any are left. */ + static us_socket_t *replayParkedRequestsIfResponseComplete(us_socket_t *s) { + if (us_socket_is_closed(s) || us_socket_is_shut_down(s)) { + return s; + } + auto *httpResponseData = reinterpret_cast *>(us_socket_ext(s)); + if (httpResponseData->parkedRequestBytes.isEmpty() + || (httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) + || !reinterpret_cast *>(s)->hasFullyDrained()) { + return s; + } + /* Paused by onData when it parked them; it pauses again if the replayed + * dispatch leaves a response pending with more requests behind it. */ + reinterpret_cast *>(s)->resume(); + return replayParkedRequestBytes(s); + } + template static us_socket_t *onEnd(us_socket_t *s) { auto *asyncSocket = reinterpret_cast *>(s); diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index 7925b43cdf92..1092b38d5e14 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -594,12 +594,23 @@ struct HttpResponseData; private: std::string fallback; public: - /* node:http flood prevention. HTTP_NODE_READS_PAUSED (state bit) = the socket's raw reads are - * paused and stays set through spill replay; this flag = "the parse loop running now must stop - * at the next request boundary and park the rest", cleared for replay so it can make progress. */ - bool nodeHttpParkAtNextBoundary = false; + /* "The parse loop running now must stop at the next request boundary and park + * the rest of the buffer, unparsed, in parkedRequestBytes." The boundary check + * also holds while bytes are already parked, so later reads queue up behind + * them and replay (HttpContext::replayParkedRequestBytes) keeps wire order. + * + * Bun.serve: HttpContext::onData derives it (at entry and after each + * request's body fin) from cannotDispatchAnotherRequest: requests pipelined + * behind a response that is still being produced or drained wait for it, + * and ones behind a response that will close the connection go down with + * it. Reads are paused while bytes are parked, bounding them to one recv. + * + * node:http flood prevention: set on the pause edge alongside + * HTTP_NODE_READS_PAUSED (which stays set through the replay) and cleared for + * the replay so it can make progress. */ + bool parkAtNextBoundary = false; bool nodeHttpSpillReplayScheduled = false; - WTF::Vector nodeHttpPausedSpill; + WTF::Vector parkedRequestBytes; private: /* This guy really has only 30 bits since we reserve two highest bits to chunked encoding parsing state */ uint64_t remainingStreamingBytes = 0; @@ -1113,15 +1124,15 @@ struct HttpResponseData; consumedTotal += length; return HttpParserResult::success(consumedTotal, returnedUser); } - /* node:http flood prevention: a dispatch earlier in this buffer paused reads. - * Stop at this request boundary, park the rest, report it as consumed so the - * caller does not spill it into the size-capped header fallback buffer. */ - if constexpr (IsNodeHttp) { - if (nodeHttpParkAtNextBoundary) [[unlikely]] { - nodeHttpPausedSpill.append(std::span(data, length)); - consumedTotal += length; - return HttpParserResult::success(consumedTotal, user); - } + /* This connection cannot take another request right now (see + * parkAtNextBoundary). Stop at this request boundary, before getHeaders + * touches the next head, park the rest verbatim and report it as consumed + * so the caller does not spill it into the size-capped header fallback + * buffer. */ + if (parkAtNextBoundary || !parkedRequestBytes.isEmpty()) [[unlikely]] { + parkedRequestBytes.append(std::span(data, length)); + consumedTotal += length; + return HttpParserResult::success(consumedTotal, user); } /* RFC 9112 2.2: ignore empty lines (CRLF) received prior to the * request-line, like Node/llhttp - e.g. a stray "\r\n" sent on an diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 756007a69d28..fa983819ebd2 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -61,6 +61,17 @@ struct HttpResponseData : AsyncSocketData, HttpParser { /* A queued pipelined response (node:http) still owes output on this * connection, so it is not idle between the responses. */ httpResponseData->isIdle = httpResponseData->nodeHttpQueuedPipelinedCount == 0; + + /* Requests are parked behind this response (Bun.serve pipelining). They + * are replayed from HttpContext::onWritable rather than here: every caller + * (internalEnd, the uws_res_end* wrappers, the request context above them) + * still tears this response down after we return, and a dispatch now would + * land in the middle of that. node:http parks too (flood prevention); its + * onWritable hook tolerates the extra writable event and decides about + * replaying itself. */ + if (!this->parkedRequestBytes.isEmpty()) [[unlikely]] { + us_socket_request_writable((us_socket_t *) uwsRes); + } } /* Caller of onWritable. It is possible onWritable calls markDone so we need to borrow it. */ diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 92aba2c148f1..1cb9b974878a 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -402,7 +402,7 @@ void JSNodeHTTPServerSocket::appendPipelinedResponse(JSC::VM& vm, WebCore::JSNod m_pipelinedResponses.last().set(vm, this, response); } -/* node:http flood prevention, resume half. Parked pipelined requests (HttpParser::nodeHttpPausedSpill) +/* node:http flood prevention, resume half. Parked pipelined requests (HttpParser::parkedRequestBytes) * must replay before fresh reads (ordering) and not synchronously inside the resuming JS operation. * Deferred as an event-loop task rooting the JS socket; reads resume once the spill drains without re-pausing. */ template @@ -412,20 +412,16 @@ static void replayNodeHttpPausedSpill(us_socket_t* socket) httpResponseData->nodeHttpSpillReplayScheduled = false; /* Let the replay's own parse loop run; HTTP_NODE_READS_PAUSED stays set so * fresh socket bytes cannot race ahead of the spill. */ - httpResponseData->nodeHttpParkAtNextBoundary = false; - WTF::Vector spill = std::exchange(httpResponseData->nodeHttpPausedSpill, {}); - if (!spill.isEmpty()) { - /* The parser's post-padded fence writes two bytes past the logical end. */ - size_t spillLength = spill.size(); - spill.grow(spillLength + LIBUS_RECV_BUFFER_PADDING); - us_socket_t* returned = uWS::HttpContext::feedNodeHttpData(socket, spill.mutableSpan().data(), (int)spillLength); + httpResponseData->parkAtNextBoundary = false; + if (!httpResponseData->parkedRequestBytes.isEmpty()) { + us_socket_t* returned = uWS::HttpContext::template replayParkedRequestBytes(socket); if (!returned || us_socket_is_closed(returned)) { return; } socket = returned; httpResponseData = reinterpret_cast*>(us_socket_ext(socket)); } - if (httpResponseData->nodeHttpParkAtNextBoundary) { + if (httpResponseData->parkAtNextBoundary) { /* A dispatch during the replay hit backpressure again and re-parked * the rest; stay paused until the next resumable event. */ return; @@ -451,13 +447,13 @@ static void onNodeHttpReadsResumable(us_socket_t* socket) if (reinterpret_cast*>(socket)->getBufferedAmount() > 0) { return; } - if (httpResponseData->nodeHttpPausedSpill.isEmpty() + if (httpResponseData->parkedRequestBytes.isEmpty() && httpResponseData->nodeHttpQueuedPipelinedCount > 0) { return; } } - if (httpResponseData->nodeHttpPausedSpill.isEmpty()) { - httpResponseData->nodeHttpParkAtNextBoundary = false; + if (httpResponseData->parkedRequestBytes.isEmpty()) { + httpResponseData->parkAtNextBoundary = false; httpResponseData->state &= ~uWS::HttpResponseData::HTTP_NODE_READS_PAUSED; reinterpret_cast*>(socket)->resume(); return; @@ -500,7 +496,7 @@ template static void onNodeHttpReadsPaused(us_socket_t* socket) { auto* d = reinterpret_cast*>(us_socket_ext(socket)); - d->nodeHttpParkAtNextBoundary = true; + d->parkAtNextBoundary = true; d->state |= uWS::HttpResponseData::HTTP_NODE_READS_PAUSED; } diff --git a/src/uws_sys/Response.rs b/src/uws_sys/Response.rs index 1c9e2e9dd96e..8d6a18c8d8c9 100644 --- a/src/uws_sys/Response.rs +++ b/src/uws_sys/Response.rs @@ -428,7 +428,8 @@ impl Response { #[inline] pub(crate) fn mark_needs_more(&mut self) { if !SSL { - c::us_socket_mark_needs_more_not_ssl(self.as_raw()) + // S008: `us_socket_t` is an `opaque_ffi!` ZST, so the deref is safe. + us_socket_t::opaque_mut(self.downcast_socket()).request_writable(); } } @@ -1163,7 +1164,6 @@ pub mod c { pub(crate) safe fn uws_res_mark_wrote_content_length_header(ssl: i32, res: &mut uws_res); pub(crate) safe fn uws_res_mark_wrote_date_header(ssl: i32, res: &mut uws_res); pub(crate) safe fn uws_res_write_mark(ssl: i32, res: &mut uws_res); - pub(crate) safe fn us_socket_mark_needs_more_not_ssl(socket: &mut uws_res); pub(crate) safe fn uws_res_state(ssl: c_int, res: &uws_res) -> State; pub(crate) safe fn uws_res_is_connect_request(ssl: i32, res: &mut uws_res) -> bool; // Out-params are `&mut` (non-null, valid for write); the C shim only diff --git a/src/uws_sys/libuwsockets.cpp b/src/uws_sys/libuwsockets.cpp index 5c1e40665e62..877c74cd1925 100644 --- a/src/uws_sys/libuwsockets.cpp +++ b/src/uws_sys/libuwsockets.cpp @@ -1525,18 +1525,6 @@ size_t uws_req_get_header(uws_req_t *res, const char *lower_case_header, } } - void us_socket_mark_needs_more_not_ssl(uws_res_r res) - { - us_socket_r s = (us_socket_t *)res; - if(us_socket_is_closed(s)) return; - s->flags.last_write_failed = 1; - /* Same gate as us_internal_rearm_writable (socket.c): re-adding READABLE - * would undo a pause mid-backpressure and re-surface a consumed EOF on a - * half-open socket. */ - us_poll_change(&s->p, s->group->loop, - LIBUS_SOCKET_WRITABLE | ((s->flags.is_paused || s->read_eof) ? 0 : LIBUS_SOCKET_READABLE)); - } - __attribute__((callback (corker, ctx))) void uws_res_cork(int ssl, uws_res_r res, void *ctx, void (*corker)(void *ctx)) nonnull_fn_decl; @@ -1680,16 +1668,6 @@ __attribute__((callback (corker, ctx))) } } - void us_socket_sendfile_needs_more(us_socket_r s) { - if(us_socket_is_closed(s)) return; - s->flags.last_write_failed = 1; - /* Same gate as us_internal_rearm_writable (socket.c): re-adding READABLE - * would undo a pause mid-backpressure and re-surface a consumed EOF on a - * half-open socket. */ - us_poll_change(&s->p, s->group->loop, - LIBUS_SOCKET_WRITABLE | ((s->flags.is_paused || s->read_eof) ? 0 : LIBUS_SOCKET_READABLE)); - } - LIBUS_SOCKET_DESCRIPTOR us_socket_get_fd(us_socket_r s) { return us_poll_fd(&s->p); } diff --git a/src/uws_sys/socket.rs b/src/uws_sys/socket.rs index 6f5bb3e0484a..f41dafe94066 100644 --- a/src/uws_sys/socket.rs +++ b/src/uws_sys/socket.rs @@ -696,7 +696,7 @@ impl NewSocketHandler { pub fn mark_needs_more_for_sendfile(&self) { const { assert!(!IS_SSL, "SSL sockets do not support sendfile yet") }; if let InternalSocket::Connected(s) = self.socket { - sock(s).send_file_needs_more(); + sock(s).request_writable(); } } diff --git a/src/uws_sys/us_socket_t.rs b/src/uws_sys/us_socket_t.rs index 9ff7a7f0ac0f..86ca176baf7b 100644 --- a/src/uws_sys/us_socket_t.rs +++ b/src/uws_sys/us_socket_t.rs @@ -438,8 +438,8 @@ impl us_socket_t { c::us_socket_flush(self); } - pub(crate) fn send_file_needs_more(&mut self) { - c::us_socket_sendfile_needs_more(self); + pub(crate) fn request_writable(&mut self) { + c::us_socket_request_writable(self); } pub fn get_fd(&self) -> Fd { @@ -575,7 +575,7 @@ mod c { ) -> i32; pub(super) safe fn us_socket_shutdown_read(s: &mut us_socket_t); pub(super) safe fn us_socket_is_shut_down(s: &us_socket_t) -> i32; - pub(super) safe fn us_socket_sendfile_needs_more(socket: &mut us_socket_t); + pub(super) safe fn us_socket_request_writable(s: &mut us_socket_t); pub(super) safe fn us_socket_get_fd(s: &us_socket_t) -> LIBUS_SOCKET_DESCRIPTOR; pub(super) safe fn us_socket_verify_error(s: &us_socket_t) -> us_bun_verify_error_t; pub(super) safe fn us_socket_get_error(s: &us_socket_t) -> c_int; diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts new file mode 100644 index 000000000000..f0623e1b0684 --- /dev/null +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -0,0 +1,447 @@ +import { describe, expect, it } from "bun:test"; +import { isPosix, tempDir, tls } from "harness"; +import { join } from "node:path"; + +// A request pipelined behind a response that was still in flight (the handler had +// not returned yet, or it had and uWS was still draining a body that did not fit +// in the socket buffer) used to make uWS close the connection the moment it +// parsed the second request head: the in-flight response was truncated and the +// second request never answered. Such a request is now held until the response +// ahead of it completes and is dispatched then, so responses stay in request +// order (RFC 9112 9.3.2); one held behind a Connection: close request is dropped +// with the connection (RFC 9112 9.6). + +type RawResponse = { statusLine: string; headers: Record; body: string }; + +// Splits the byte stream into Content-Length framed responses (a 101 has no +// body). Bodies are accumulated as chunks so a multi-megabyte body does not get +// re-concatenated on every read. +class ResponseReader { + responses: RawResponse[] = []; + // Bytes after the last complete response that do not form a head yet (after + // a 101 these are WebSocket frames). + unparsed: Buffer = Buffer.alloc(0); + #head: { statusLine: string; headers: Record } | undefined; + #bodyChunks: Buffer[] = []; + #bodyHave = 0; + #bodyNeed = 0; + + push(chunk: Buffer) { + while (chunk.length > 0) { + if (!this.#head) { + this.unparsed = Buffer.concat([this.unparsed, chunk]); + const headEnd = this.unparsed.indexOf("\r\n\r\n"); + if (headEnd === -1) return; + const [statusLine, ...lines] = this.unparsed.subarray(0, headEnd).toString("latin1").split("\r\n"); + const headers: Record = {}; + for (const line of lines) { + const colon = line.indexOf(":"); + headers[line.slice(0, colon).toLowerCase()] = line.slice(colon + 1).trim(); + } + this.#head = { statusLine, headers }; + this.#bodyNeed = Number(headers["content-length"] ?? 0); + chunk = this.unparsed.subarray(headEnd + 4); + this.unparsed = Buffer.alloc(0); + } + const take = chunk.subarray(0, this.#bodyNeed - this.#bodyHave); + this.#bodyChunks.push(take); + this.#bodyHave += take.length; + chunk = chunk.subarray(take.length); + if (this.#bodyHave < this.#bodyNeed) return; + this.responses.push({ ...this.#head, body: Buffer.concat(this.#bodyChunks).toString("latin1") }); + this.#head = undefined; + this.#bodyChunks = []; + this.#bodyHave = 0; + } + } +} + +type Target = ({ port: number; hostname: string } | { unix: string }) & { tls?: { ca: string } }; + +class RawClient extends ResponseReader { + closed = false; + #socket!: Awaited>; + #waiters: { condition: (client: RawClient) => boolean; resolve: () => void; reject: (error: Error) => void }[] = []; + + static async connect(target: Target): Promise { + const client = new RawClient(); + const handshake = Promise.withResolvers(); + client.#socket = await Bun.connect({ + ...target, + socket: { + handshake: (_socket, success, error) => (success ? handshake.resolve() : handshake.reject(error)), + data: (_socket, chunk) => { + client.push(chunk); + client.#settle(); + }, + close: () => { + client.closed = true; + client.#settle(); + }, + error: (_socket, error) => client.#fail(error), + connectError: (_socket, error) => client.#fail(error), + }, + }); + if (target.tls) await handshake.promise; + return client; + } + + write(data: string | Uint8Array) { + const length = typeof data === "string" ? Buffer.byteLength(data, "latin1") : data.byteLength; + expect(this.#socket.write(data)).toBe(length); + } + + // Resolves once `condition` holds, or as soon as the server closes the + // connection, so that the assertions after it report what actually arrived. + until(condition: (client: RawClient) => boolean): Promise { + const { promise, resolve, reject } = Promise.withResolvers(); + this.#waiters.push({ condition, resolve, reject }); + this.#settle(); + return promise; + } + + #settle() { + this.#waiters = this.#waiters.filter(waiter => { + if (!this.closed && !waiter.condition(this)) return true; + waiter.resolve(); + return false; + }); + } + + #fail(error: Error) { + const waiters = this.#waiters; + this.#waiters = []; + for (const waiter of waiters) waiter.reject(error); + } + + [Symbol.dispose]() { + this.#socket.end(); + } +} + +// The parking/replay code is instantiated once per uWS socket flavor (plain and +// TLS); a unix listener is the transport whose send buffer is smallest. +type Transport = { + name: "tcp" | "tls" | "unix"; + supported: boolean; + listen(dir: string): object; + target(server: Bun.Server, dir: string): Target; + probe(server: Bun.Server, dir: string): Promise; +}; +const tcp = { port: 0, hostname: "127.0.0.1" }; +const transports: Transport[] = [ + { + name: "tcp", + supported: true, + listen: () => tcp, + target: server => ({ port: server.port!, hostname: "127.0.0.1" }), + probe: server => fetch(`${server.url}probe`), + }, + { + name: "tls", + supported: true, + listen: () => ({ ...tcp, tls }), + target: server => ({ port: server.port!, hostname: "127.0.0.1", tls: { ca: tls.cert } }), + probe: server => fetch(`${server.url}probe`, { tls: { ca: tls.cert } }), + }, + { + name: "unix", + supported: isPosix, + listen: dir => ({ unix: join(dir, "pipeline.sock") }), + target: (_server, dir) => ({ unix: join(dir, "pipeline.sock") }), + probe: (_server, dir) => fetch("http://localhost/probe", { unix: join(dir, "pipeline.sock") }), + }, +]; +const tcpOnly = transports[0]; + +const request = (path: string, extraHeaders = "") => `GET ${path} HTTP/1.1\r\nHost: x\r\n${extraHeaders}\r\n`; +const ok = (body: string) => ({ statusLine: "HTTP/1.1 200 OK", body }); +const summarize = ({ statusLine, body }: RawResponse) => ({ statusLine, body }); + +// Every handler below answers any path it does not treat specially with this. +const plainResponse = (req: Request) => new Response(`body of ${new URL(req.url).pathname}`); + +// A round trip on a separate connection. Anything the pipelining client wrote +// before this was readable on the server before the probe was even sent, so by +// the time the probe has been answered the server has read it (and, with the +// request ahead of it still pending, parked it). It also moves the test past the +// microtask checkpoint Bun runs inside a dispatch: releasing a handler straight +// from its `entered` promise would complete the response while the parser is +// still inside that request's dispatch, which is the ordinary synchronous +// pipelining path rather than the one under test. +async function probe(transport: Transport, server: Bun.Server, dir: string) { + expect(await (await transport.probe(server, dir)).text()).toBe("body of /probe"); +} + +// A handler that parks on `/hold*` paths until the test releases that path, and +// records the order in which requests reached JS. +function holdingHandler() { + type Gate = ReturnType>; + const hits: string[] = []; + const entered = new Map(); + const released = new Map(); + const gate = (map: Map, path: string) => { + let resolvers = map.get(path); + if (!resolvers) map.set(path, (resolvers = Promise.withResolvers())); + return resolvers; + }; + return { + hits, + entered: (path: string) => gate(entered, path).promise, + release: (path: string) => gate(released, path).resolve(), + async fetch(req: Request) { + const path = new URL(req.url).pathname; + hits.push(path); + if (path.startsWith("/hold")) { + gate(entered, path).resolve(); + await gate(released, path).promise; + } + return plainResponse(req); + }, + }; +} + +// Large enough that the first tryEnd() cannot hand the whole body to the kernel +// on any transport (loopback TCP takes at most a few MiB, a unix socket a couple +// hundred KiB), so the rest of the body is still being drained through +// onWritable when the second request head is parsed. +const BIG_BODY_LENGTH = 16 * 1024 * 1024; + +describe.each(transports)("$name", transport => { + // The response ahead is complete as far as the (sync) handler is concerned; + // only uWS's drain of the buffered body's tail is outstanding. + it.if(transport.supported)( + "a request pipelined behind a buffered body larger than the socket buffer gets the whole body, then its own response", + async () => { + using dir = tempDir("serve-pipelining", {}); + const big = Buffer.alloc(BIG_BODY_LENGTH, "x").toString("latin1"); + const hits: string[] = []; + using server = Bun.serve({ + ...transport.listen(String(dir)), + fetch(req) { + const path = new URL(req.url).pathname; + hits.push(path); + return path === "/big" ? new Response(big) : plainResponse(req); + }, + }); + using client = await RawClient.connect(transport.target(server, String(dir))); + + client.write(request("/big") + request("/small")); + await client.until(c => c.responses.length === 2); + + expect({ + hits, + closed: client.closed, + responses: client.responses.map(({ statusLine, headers, body }) => ({ + statusLine, + contentLength: headers["content-length"], + bodyLength: body.length, + bodyIsIntact: body === big || body === "body of /small", + })), + }).toEqual({ + hits: ["/big", "/small"], + closed: false, + responses: [ + { + statusLine: "HTTP/1.1 200 OK", + contentLength: String(BIG_BODY_LENGTH), + bodyLength: BIG_BODY_LENGTH, + bodyIsIntact: true, + }, + { statusLine: "HTTP/1.1 200 OK", contentLength: "14", bodyLength: 14, bodyIsIntact: true }, + ], + }); + }, + ); + + it.if(transport.supported)( + "requests pipelined behind an async handler are dispatched one at a time, each after the response ahead of it", + async () => { + using dir = tempDir("serve-pipelining", {}); + const handler = holdingHandler(); + using server = Bun.serve({ ...transport.listen(String(dir)), fetch: handler.fetch }); + using client = await RawClient.connect(transport.target(server, String(dir))); + // (Or the server giving up on the connection, which is the failure mode.) + const enteredOrClosed = (path: string) => Promise.race([handler.entered(path), client.until(c => c.closed)]); + + // All three heads arrive in one read. + client.write(request("/hold/1") + request("/hold/2") + request("/hold/3")); + await enteredOrClosed("/hold/1"); + await probe(transport, server, String(dir)); + expect(handler.hits).toEqual(["/hold/1", "/probe"]); + + // Completing a response dispatches exactly the next request, which parks + // the one behind it again. + handler.release("/hold/1"); + await enteredOrClosed("/hold/2"); + await probe(transport, server, String(dir)); + expect(handler.hits).toEqual(["/hold/1", "/probe", "/hold/2", "/probe"]); + await client.until(c => c.responses.length === 1); + + handler.release("/hold/2"); + await enteredOrClosed("/hold/3"); + expect(handler.hits).toEqual(["/hold/1", "/probe", "/hold/2", "/probe", "/hold/3"]); + + handler.release("/hold/3"); + await client.until(c => c.responses.length === 3); + expect({ closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + closed: false, + responses: [ok("body of /hold/1"), ok("body of /hold/2"), ok("body of /hold/3")], + }); + }, + ); +}); + +it("a request arriving in a later read while the handler is still running waits for the response", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ ...tcp, fetch: handler.fetch }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(request("/hold")); + await handler.entered("/hold"); + client.write(request("/after")); + await probe(tcpOnly, server, ""); + expect(handler.hits).toEqual(["/hold", "/probe"]); + + handler.release("/hold"); + await client.until(c => c.responses.length === 2); + expect({ hits: handler.hits, closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe", "/after"], + closed: false, + responses: [ok("body of /hold"), ok("body of /after")], + }); +}); + +it("a request pipelined behind a request body that the handler is still consuming waits for the response", async () => { + const seen: string[] = []; + const bodyRead = Promise.withResolvers(); + const release = Promise.withResolvers(); + using server = Bun.serve({ + ...tcp, + async fetch(req) { + if (new URL(req.url).pathname !== "/upload") return plainResponse(req); + seen.push(await req.text()); + bodyRead.resolve(); + await release.promise; + return new Response(`uploaded ${seen[0]}`); + }, + }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write("POST /upload HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\n\r\n"); + // The body and the next request share a read: the body belongs to the upload + // and must reach its handler, the request behind it must wait. + client.write("hello" + request("/after")); + await Promise.race([bodyRead.promise, client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ seen, closed: client.closed }).toEqual({ seen: ["hello"], closed: false }); + + release.resolve(); + await client.until(c => c.responses.length === 2); + expect({ closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + closed: false, + responses: [ok("uploaded hello"), ok("body of /after")], + }); +}); + +describe("a request pipelined behind a Connection: close request", () => { + // (An HTTP/1.0 request line marks the connection the same way.) + const closingThenAnother = request("/hold", "Connection: close\r\n") + request("/never"); + + it("is dropped when the response ahead of it completes later", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ ...tcp, fetch: handler.fetch }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(closingThenAnother); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.closed); + expect({ hits: handler.hits, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe"], + responses: [ok("body of /hold")], + }); + }); + + it("is dropped when the response ahead of it completes synchronously", async () => { + const hits: string[] = []; + using server = Bun.serve({ + ...tcp, + fetch(req) { + hits.push(new URL(req.url).pathname); + return plainResponse(req); + }, + }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(closingThenAnother); + // Answering /never would keep the connection open, so also stop on its response. + await client.until(c => c.closed || c.responses.length === 2); + expect({ hits, closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/hold"], + closed: true, + responses: [ok("body of /hold")], + }); + }); +}); + +it("a WebSocket upgrade pipelined behind an async handler is performed once the response ahead of it is out", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ + ...tcp, + fetch(req, server) { + if (new URL(req.url).pathname !== "/ws") return handler.fetch(req); + handler.hits.push("/ws"); + return server.upgrade(req) ? undefined : new Response("not upgraded", { status: 400 }); + }, + websocket: { + message(ws, message) { + ws.send(message); + }, + }, + }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write( + request("/hold") + + request( + "/ws", + "Upgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Version: 13\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n", + ), + ); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.responses.length === 2); + expect({ + hits: handler.hits, + closed: client.closed, + responses: client.responses.map(({ statusLine, headers, body }) => ({ + statusLine, + body, + accept: headers["sec-websocket-accept"], + })), + }).toEqual({ + hits: ["/hold", "/probe", "/ws"], + closed: false, + responses: [ + { statusLine: "HTTP/1.1 200 OK", body: "body of /hold", accept: undefined }, + // RFC 6455 1.3: the accept value for the sample nonce above. + { statusLine: "HTTP/1.1 101 Switching Protocols", body: "", accept: "s3pPLMBiTxaQ9kYGzzhZRbK+xOo=" }, + ], + }); + + // The connection is the WebSocket now: a masked text frame "hi" (mask key + // 1 2 3 4) is echoed back as an unmasked one. + client.write(new Uint8Array([0x81, 0x82, 1, 2, 3, 4, "h".charCodeAt(0) ^ 1, "i".charCodeAt(0) ^ 2])); + await client.until(c => c.unparsed.length >= 4); + expect({ closed: client.closed, frame: [...client.unparsed] }).toEqual({ + closed: false, + frame: [0x81, 0x02, "h".charCodeAt(0), "i".charCodeAt(0)], + }); +}); From 5575a579b8451bf054cca38ba1aa4b86daee7043 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:54:23 +0000 Subject: [PATCH 02/22] Bun.serve: resume reads when an upgrade drops the requests held behind it HttpContext::onData pauses the socket while request bytes are parked behind a pending response. If that response turns out to be a WebSocket upgrade, upgrade() destructs the HttpResponseData (and the parked bytes with it, as for bytes trailing a synchronous upgrade) but us_socket_adopt carries the paused flag over, so the WebSocket sent its 101 and then never read a frame. Drop the parked bytes and resume before internalEnd(), so markDone() does not arm a replay dispatch for them either. Tests folded in from the earlier pipelining PRs (#33664, #35036, #32868) for the cases bun-serve-pipelining.test.ts did not cover yet: a Bun.file() body from the handler and a Bun.file() route over each transport, a held request that carries a body with a Connection: close request behind it, held bytes that are a parse error, and the upgrade case above. --- packages/bun-uws/src/HttpResponse.h | 15 +- test/js/bun/http/bun-serve-pipelining.test.ts | 327 +++++++++++++----- 2 files changed, 245 insertions(+), 97 deletions(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index c556f4dece84..898ff5984cdf 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -378,13 +378,26 @@ struct HttpResponse : public AsyncSocket { } } + auto* responseData = getHttpResponseData(); + + /* Request bytes parked behind this handshake (HttpParser::parkedRequestBytes) + * go down with the HTTP state destructed below, like bytes trailing a + * synchronous upgrade in the same read (a client may not send anything + * before the 101 anyway, RFC 6455 4.1). Parking paused reads; the adopted + * WebSocket needs them flowing, and us_socket_adopt keeps the flag. Dropped + * before endUpgradeHandshake so markDone() does not arm a replay dispatch + * that would land on the WebSocket as a spurious drain. */ + if (!responseData->parkedRequestBytes.isEmpty()) [[unlikely]] { + responseData->parkedRequestBytes.clear(); + Super::resume(); + } + endUpgradeHandshake(); /* Grab the httpContext from res */ HttpContext *httpContext = HttpContext::fromSocket((struct us_socket_t *) this); /* Move any backpressure out of HttpResponse */ - auto* responseData = getHttpResponseData(); BackPressure backpressure(std::move(((AsyncSocketData *) responseData)->buffer)); auto* socketData = responseData->socketData; diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index f0623e1b0684..c9bed93c085c 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -9,7 +9,8 @@ import { join } from "node:path"; // second request never answered. Such a request is now held until the response // ahead of it completes and is dispatched then, so responses stay in request // order (RFC 9112 9.3.2); one held behind a Connection: close request is dropped -// with the connection (RFC 9112 9.6). +// with the connection (RFC 9112 9.6), and one held behind a request that turns +// the connection into a WebSocket is dropped with the HTTP state. type RawResponse = { statusLine: string; headers: Record; body: string }; @@ -207,52 +208,85 @@ function holdingHandler() { // onWritable when the second request head is parsed. const BIG_BODY_LENGTH = 16 * 1024 * 1024; -describe.each(transports)("$name", transport => { - // The response ahead is complete as far as the (sync) handler is concerned; - // only uWS's drain of the buffered body's tail is outstanding. - it.if(transport.supported)( - "a request pipelined behind a buffered body larger than the socket buffer gets the whole body, then its own response", - async () => { - using dir = tempDir("serve-pipelining", {}); - const big = Buffer.alloc(BIG_BODY_LENGTH, "x").toString("latin1"); - const hits: string[] = []; - using server = Bun.serve({ - ...transport.listen(String(dir)), - fetch(req) { - const path = new URL(req.url).pathname; - hits.push(path); - return path === "/big" ? new Response(big) : plainResponse(req); - }, - }); - using client = await RawClient.connect(transport.target(server, String(dir))); +const big = Buffer.alloc(BIG_BODY_LENGTH, "x").toString("latin1"); + +// The ways a big response body reaches the socket. The handler is synchronous +// in each case, so the response is complete as far as the app is concerned when +// the second head is parsed; what is still outstanding is the body transfer: +// uWS draining a buffered body's tail through onWritable, or the runtime's file +// pump (sendfile over plain TCP on Linux, read+write chunks elsewhere) for a +// Bun.file() returned from the handler or served by a file route. +type BigBody = { + name: string; + // How /big is served (`dir` holds big.txt). `hits` records every request + // that reaches the fetch handler; a route answers /big without it. + serve(dir: string, hits: string[]): { fetch(req: Request): Response; routes?: Record }; + expectedHits: string[]; +}; +const recordingHandler = (hits: string[], bigResponse?: () => Response) => (req: Request) => { + const path = new URL(req.url).pathname; + hits.push(path); + return path === "/big" && bigResponse ? bigResponse() : plainResponse(req); +}; +const bigBodies: BigBody[] = [ + { + name: "a buffered body", + serve: (_dir, hits) => ({ fetch: recordingHandler(hits, () => new Response(big)) }), + expectedHits: ["/big", "/small"], + }, + { + name: "a Bun.file() body returned from the handler", + serve: (dir, hits) => ({ fetch: recordingHandler(hits, () => new Response(Bun.file(join(dir, "big.txt")))) }), + expectedHits: ["/big", "/small"], + }, + { + name: "a Bun.file() route", + serve: (dir, hits) => ({ + routes: { "/big": new Response(Bun.file(join(dir, "big.txt"))) }, + fetch: recordingHandler(hits), + }), + expectedHits: ["/small"], + }, +]; - client.write(request("/big") + request("/small")); - await client.until(c => c.responses.length === 2); - - expect({ - hits, - closed: client.closed, - responses: client.responses.map(({ statusLine, headers, body }) => ({ - statusLine, - contentLength: headers["content-length"], - bodyLength: body.length, - bodyIsIntact: body === big || body === "body of /small", - })), - }).toEqual({ - hits: ["/big", "/small"], - closed: false, - responses: [ - { - statusLine: "HTTP/1.1 200 OK", - contentLength: String(BIG_BODY_LENGTH), - bodyLength: BIG_BODY_LENGTH, - bodyIsIntact: true, - }, - { statusLine: "HTTP/1.1 200 OK", contentLength: "14", bodyLength: 14, bodyIsIntact: true }, - ], - }); - }, - ); +describe.each(transports)("$name", transport => { + describe.each(bigBodies)("$name", bigBody => { + it.if(transport.supported)( + "larger than the socket buffer is delivered whole to a client that pipelined a request behind it, which is then answered", + async () => { + using dir = tempDir("serve-pipelining", { "big.txt": big }); + const hits: string[] = []; + using server = Bun.serve({ ...transport.listen(String(dir)), ...bigBody.serve(String(dir), hits) }); + using client = await RawClient.connect(transport.target(server, String(dir))); + + client.write(request("/big") + request("/small")); + await client.until(c => c.responses.length === 2); + + expect({ + hits, + closed: client.closed, + responses: client.responses.map(({ statusLine, headers, body }) => ({ + statusLine, + contentLength: headers["content-length"], + bodyLength: body.length, + bodyIsIntact: body === big || body === "body of /small", + })), + }).toEqual({ + hits: bigBody.expectedHits, + closed: false, + responses: [ + { + statusLine: "HTTP/1.1 200 OK", + contentLength: String(BIG_BODY_LENGTH), + bodyLength: BIG_BODY_LENGTH, + bodyIsIntact: true, + }, + { statusLine: "HTTP/1.1 200 OK", contentLength: "14", bodyLength: 14, bodyIsIntact: true }, + ], + }); + }, + ); + }); it.if(transport.supported)( "requests pipelined behind an async handler are dispatched one at a time, each after the response ahead of it", @@ -344,6 +378,60 @@ it("a request pipelined behind a request body that the handler is still consumin }); }); +// The held bytes are re-parsed from the top when they are released: a request +// with a body gets its body back, and whatever is behind it is held again while +// that request's own response is pending (here until a Connection: close request +// ends the connection after its response). +it("a held request with a body is dispatched with its body, and the request behind it waits for that response in turn", async () => { + const handler = holdingHandler(); + const uploads: string[] = []; + using server = Bun.serve({ + ...tcp, + async fetch(req) { + if (new URL(req.url).pathname !== "/upload") return handler.fetch(req); + handler.hits.push("/upload"); + uploads.push(await req.text()); + return new Response(`uploaded ${uploads.at(-1)}`); + }, + }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write( + request("/hold") + + "POST /upload HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\n\r\nhello" + + request("/last", "Connection: close\r\n"), + ); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.closed); + expect({ hits: handler.hits, uploads, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe", "/upload", "/last"], + uploads: ["hello"], + responses: [ok("body of /hold"), ok("uploaded hello"), ok("body of /last")], + }); +}); + +it("held bytes that are not a valid request get the error response after the response ahead of them, not instead of it", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ ...tcp, fetch: handler.fetch }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(request("/hold") + "GET /bad HTTP/9.9\r\nHost: x\r\n\r\n"); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.closed); + expect({ hits: handler.hits, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe"], + responses: [ok("body of /hold"), { statusLine: "HTTP/1.1 505 HTTP Version Not Supported", body: "" }], + }); +}); + describe("a request pipelined behind a Connection: close request", () => { // (An HTTP/1.0 request line marks the connection the same way.) const closingThenAnother = request("/hold", "Connection: close\r\n") + request("/never"); @@ -388,60 +476,107 @@ describe("a request pipelined behind a Connection: close request", () => { }); }); -it("a WebSocket upgrade pipelined behind an async handler is performed once the response ahead of it is out", async () => { - const handler = holdingHandler(); - using server = Bun.serve({ - ...tcp, - fetch(req, server) { - if (new URL(req.url).pathname !== "/ws") return handler.fetch(req); - handler.hits.push("/ws"); - return server.upgrade(req) ? undefined : new Response("not upgraded", { status: 400 }); - }, - websocket: { - message(ws, message) { - ws.send(message); - }, - }, +describe("WebSocket upgrade", () => { + const upgradeRequest = request( + "/ws", + "Upgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Version: 13\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n", + ); + // RFC 6455 1.3: the accept value for the sample nonce above. + const switching = { + statusLine: "HTTP/1.1 101 Switching Protocols", + body: "", + accept: "s3pPLMBiTxaQ9kYGzzhZRbK+xOo=", + }; + const withAccept = ({ statusLine, headers, body }: RawResponse) => ({ + statusLine, + body, + accept: headers["sec-websocket-accept"] as string | undefined, }); - using client = await RawClient.connect(tcpOnly.target(server, "")); + // A masked text frame "hi" (mask key 1 2 3 4); the server echoes it unmasked. + const maskedHiFrame = new Uint8Array([0x81, 0x82, 1, 2, 3, 4, "h".charCodeAt(0) ^ 1, "i".charCodeAt(0) ^ 2]); + const echoedHiFrame = [0x81, 0x02, "h".charCodeAt(0), "i".charCodeAt(0)]; + + // /ws is upgraded from the handler itself, or (held: true) from a continuation + // the test releases; every other path is the holding handler's. + function serveWithUpgrade(handler: ReturnType, { held }: { held: boolean }) { + const entered = Promise.withResolvers(); + const released = Promise.withResolvers(); + const server = Bun.serve({ + ...tcp, + fetch(req, server) { + if (new URL(req.url).pathname !== "/ws") return handler.fetch(req); + handler.hits.push("/ws"); + const upgrade = () => (server.upgrade(req) ? undefined : new Response("not upgraded", { status: 400 })); + if (!held) return upgrade(); + entered.resolve(); + return released.promise.then(upgrade); + }, + websocket: { + message(ws, message) { + ws.send(message); + }, + }, + }); + return { server, upgradeEntered: entered.promise, releaseUpgrade: released.resolve }; + } - client.write( - request("/hold") + - request( - "/ws", - "Upgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Version: 13\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n", - ), - ); - await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); - await probe(tcpOnly, server, ""); - expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + async function expectEcho(client: RawClient) { + client.write(maskedHiFrame); + await client.until(c => c.unparsed.length >= echoedHiFrame.length); + expect({ closed: client.closed, frame: [...client.unparsed] }).toEqual({ closed: false, frame: echoedHiFrame }); + } - handler.release("/hold"); - await client.until(c => c.responses.length === 2); - expect({ - hits: handler.hits, - closed: client.closed, - responses: client.responses.map(({ statusLine, headers, body }) => ({ - statusLine, - body, - accept: headers["sec-websocket-accept"], - })), - }).toEqual({ - hits: ["/hold", "/probe", "/ws"], - closed: false, - responses: [ - { statusLine: "HTTP/1.1 200 OK", body: "body of /hold", accept: undefined }, - // RFC 6455 1.3: the accept value for the sample nonce above. - { statusLine: "HTTP/1.1 101 Switching Protocols", body: "", accept: "s3pPLMBiTxaQ9kYGzzhZRbK+xOo=" }, - ], + it("pipelined behind an async handler is performed once the response ahead of it is out", async () => { + const handler = holdingHandler(); + using server = serveWithUpgrade(handler, { held: false }).server; + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(request("/hold") + upgradeRequest); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.responses.length === 2); + expect({ hits: handler.hits, closed: client.closed, responses: client.responses.map(withAccept) }).toEqual({ + hits: ["/hold", "/probe", "/ws"], + closed: false, + responses: [{ statusLine: "HTTP/1.1 200 OK", body: "body of /hold", accept: undefined }, switching], + }); + + // The connection is the WebSocket now. + await expectEcho(client); }); - // The connection is the WebSocket now: a masked text frame "hi" (mask key - // 1 2 3 4) is echoed back as an unmasked one. - client.write(new Uint8Array([0x81, 0x82, 1, 2, 3, 4, "h".charCodeAt(0) ^ 1, "i".charCodeAt(0) ^ 2])); - await client.until(c => c.unparsed.length >= 4); - expect({ closed: client.closed, frame: [...client.unparsed] }).toEqual({ - closed: false, - frame: [0x81, 0x02, "h".charCodeAt(0), "i".charCodeAt(0)], + // The request held behind the handshake is discarded with the HTTP state when + // the connection becomes a WebSocket (as bytes trailing a synchronous upgrade + // in the same read always were), and holding it must not leave the WebSocket's + // reads switched off. + it("performed by an async handler with a request pipelined behind it drops that request and reads frames", async () => { + const handler = holdingHandler(); + const { upgradeEntered, releaseUpgrade, ...serving } = serveWithUpgrade(handler, { held: true }); + using server = serving.server; + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(upgradeRequest + request("/never")); + await Promise.race([upgradeEntered, client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/ws", "/probe"], closed: false }); + + releaseUpgrade(); + await client.until(c => c.responses.length === 1); + expect({ closed: client.closed, responses: client.responses.map(withAccept) }).toEqual({ + closed: false, + responses: [switching], + }); + + await expectEcho(client); + // The echo round trip above means the server has long since processed + // everything it received before the frame; /never was not part of it. + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, responses: client.responses.length }).toEqual({ + hits: ["/ws", "/probe", "/probe"], + responses: 1, + }); }); }); From 1958e9a7ba41bb28ac1a7c912ce234b9adc42168 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:08:45 +0000 Subject: [PATCH 03/22] uws: say what the resume in upgrade() costs the adopted WebSocket The resume re-arms writable interest too, so the WebSocket gets one drain callback right after open; clearing the parked bytes before internalEnd() avoids a second one, it does not avoid that one. --- packages/bun-uws/src/HttpResponse.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 898ff5984cdf..e503f375108a 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -384,9 +384,11 @@ struct HttpResponse : public AsyncSocket { * go down with the HTTP state destructed below, like bytes trailing a * synchronous upgrade in the same read (a client may not send anything * before the 101 anyway, RFC 6455 4.1). Parking paused reads; the adopted - * WebSocket needs them flowing, and us_socket_adopt keeps the flag. Dropped - * before endUpgradeHandshake so markDone() does not arm a replay dispatch - * that would land on the WebSocket as a spurious drain. */ + * WebSocket needs them flowing, and us_socket_adopt keeps the flag. The + * resume re-arms writable too, so the WebSocket gets one drain callback with + * nothing to drain right after open; dropping the bytes before + * endUpgradeHandshake() keeps markDone() from arming a second one for a + * replay that cannot happen. */ if (!responseData->parkedRequestBytes.isEmpty()) [[unlikely]] { responseData->parkedRequestBytes.clear(); Super::resume(); From e809be22e1578a7e04165e73894af50a37ef2600 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 23:34:48 +0000 Subject: [PATCH 04/22] test: run the upgrade pipelining cases over tls as well --- test/js/bun/http/bun-serve-pipelining.test.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index c9bed93c085c..37f8830537fc 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -476,7 +476,9 @@ describe("a request pipelined behind a Connection: close request", () => { }); }); -describe("WebSocket upgrade", () => { +// upgrade() is instantiated once per socket flavor, like the parking code; the +// unix transport shares the plain instantiation. +describe.each(transports.filter(t => t.name !== "unix"))("WebSocket upgrade over $name", transport => { const upgradeRequest = request( "/ws", "Upgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Version: 13\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n", @@ -502,7 +504,7 @@ describe("WebSocket upgrade", () => { const entered = Promise.withResolvers(); const released = Promise.withResolvers(); const server = Bun.serve({ - ...tcp, + ...transport.listen(""), fetch(req, server) { if (new URL(req.url).pathname !== "/ws") return handler.fetch(req); handler.hits.push("/ws"); @@ -529,11 +531,11 @@ describe("WebSocket upgrade", () => { it("pipelined behind an async handler is performed once the response ahead of it is out", async () => { const handler = holdingHandler(); using server = serveWithUpgrade(handler, { held: false }).server; - using client = await RawClient.connect(tcpOnly.target(server, "")); + using client = await RawClient.connect(transport.target(server, "")); client.write(request("/hold") + upgradeRequest); await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); - await probe(tcpOnly, server, ""); + await probe(transport, server, ""); expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); handler.release("/hold"); @@ -556,11 +558,11 @@ describe("WebSocket upgrade", () => { const handler = holdingHandler(); const { upgradeEntered, releaseUpgrade, ...serving } = serveWithUpgrade(handler, { held: true }); using server = serving.server; - using client = await RawClient.connect(tcpOnly.target(server, "")); + using client = await RawClient.connect(transport.target(server, "")); client.write(upgradeRequest + request("/never")); await Promise.race([upgradeEntered, client.until(c => c.closed)]); - await probe(tcpOnly, server, ""); + await probe(transport, server, ""); expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/ws", "/probe"], closed: false }); releaseUpgrade(); @@ -573,7 +575,7 @@ describe("WebSocket upgrade", () => { await expectEcho(client); // The echo round trip above means the server has long since processed // everything it received before the frame; /never was not part of it. - await probe(tcpOnly, server, ""); + await probe(transport, server, ""); expect({ hits: handler.hits, responses: client.responses.length }).toEqual({ hits: ["/ws", "/probe", "/probe"], responses: 1, From ac9a470c384fb6b9b0e6ac5d16c4225c0f08c73c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:46:01 +0000 Subject: [PATCH 05/22] uws: keep a request-body resume from reopening reads over parked requests HttpContext pauses the socket while request bytes are parked behind a pending response and resumes it itself when it replays them (upgrade() when it drops them). The runtime's own resume() calls release a request-body backpressure pause and can arrive after that body completed and the bytes behind it were parked: RequestContext::detach_response, on_request_body_stream_drained, and the response sink's end(). Reopening reads there only queues more bytes behind the parked ones, and on kqueue, where the read and write filters are delivered separately, a peer FIN read that way reaches onEnd before the replay and closes the connection over the parked request. HttpResponse::resume() now leaves the read side alone while bytes are parked; the two pipelining resume sites call AsyncSocket::resume() directly and node:http's flood prevention only resumes once its parked bytes are gone, so neither is affected. Not separately observable on epoll, where the replaying writable dispatch runs before the read in the same event; the existing body backpressure and pipelining tests cover the paths involved. --- packages/bun-uws/src/HttpResponse.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index e503f375108a..447ff82a6d77 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -496,7 +496,16 @@ struct HttpResponse : public AsyncSocket { } HttpResponse *resume() { - Super::resume(); + /* While requests are parked behind this response the pause belongs to the + * pipelining code (HttpContext resumes when it replays them, upgrade() when + * it drops them). The resumes arriving here release a request-body + * backpressure pause and can land after the body completed and the bytes + * behind it were parked; reading on would only queue more behind them, or + * take a FIN that closes the connection over them. node:http's flood + * prevention only gets here once its parked bytes are gone. */ + if (getHttpResponseData()->parkedRequestBytes.isEmpty()) [[likely]] { + Super::resume(); + } this->resetTimeout(); return this; } From 5d2aab13b09e8718b5363533a00fe662de8e0d9a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 02:13:39 +0000 Subject: [PATCH 06/22] test: a request pipelined behind a streaming response Covers the response-still-being-produced case over each transport. Ending the stream is also what makes the response sink call resume() on the socket while the request behind it is parked, so this drives the HttpResponse:: resume() guard added in the previous commit (confirmed by instrumenting it locally: it is taken once per transport here). The reader learns chunked framing for it. --- test/js/bun/http/bun-serve-pipelining.test.ts | 109 ++++++++++++++++-- 1 file changed, 101 insertions(+), 8 deletions(-) diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 37f8830537fc..9c5cfddfaaf9 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -14,15 +14,18 @@ import { join } from "node:path"; type RawResponse = { statusLine: string; headers: Record; body: string }; -// Splits the byte stream into Content-Length framed responses (a 101 has no -// body). Bodies are accumulated as chunks so a multi-megabyte body does not get -// re-concatenated on every read. +// Splits the byte stream into responses framed by Content-Length or chunked +// encoding (a 101 has no body). Content-Length bodies are accumulated as chunks +// so a multi-megabyte body does not get re-concatenated on every read; the +// chunked bodies here are small and are simply re-scanned. class ResponseReader { responses: RawResponse[] = []; // Bytes after the last complete response that do not form a head yet (after // a 101 these are WebSocket frames). unparsed: Buffer = Buffer.alloc(0); #head: { statusLine: string; headers: Record } | undefined; + #chunked = false; + #pendingChunked: Buffer = Buffer.alloc(0); #bodyChunks: Buffer[] = []; #bodyHave = 0; #bodyNeed = 0; @@ -40,21 +43,49 @@ class ResponseReader { headers[line.slice(0, colon).toLowerCase()] = line.slice(colon + 1).trim(); } this.#head = { statusLine, headers }; + this.#chunked = headers["transfer-encoding"] === "chunked"; this.#bodyNeed = Number(headers["content-length"] ?? 0); chunk = this.unparsed.subarray(headEnd + 4); this.unparsed = Buffer.alloc(0); } - const take = chunk.subarray(0, this.#bodyNeed - this.#bodyHave); - this.#bodyChunks.push(take); - this.#bodyHave += take.length; - chunk = chunk.subarray(take.length); - if (this.#bodyHave < this.#bodyNeed) return; + if (this.#chunked) { + const rest = this.#takeChunkedBody(chunk); + if (rest === undefined) return; + chunk = rest; + } else { + const take = chunk.subarray(0, this.#bodyNeed - this.#bodyHave); + this.#bodyChunks.push(take); + this.#bodyHave += take.length; + chunk = chunk.subarray(take.length); + if (this.#bodyHave < this.#bodyNeed) return; + } this.responses.push({ ...this.#head, body: Buffer.concat(this.#bodyChunks).toString("latin1") }); this.#head = undefined; this.#bodyChunks = []; this.#bodyHave = 0; } } + + // Returns what follows the body once all of it has arrived, through the + // terminating zero-size chunk (nothing here sends trailers); else undefined. + #takeChunkedBody(chunk: Buffer): Buffer | undefined { + const pending = (this.#pendingChunked = Buffer.concat([this.#pendingChunked, chunk])); + const parts: Buffer[] = []; + let pos = 0; + while (true) { + const sizeLineEnd = pending.indexOf("\r\n", pos); + if (sizeLineEnd === -1) return undefined; + const size = parseInt(pending.subarray(pos, sizeLineEnd).toString("latin1"), 16); + pos = sizeLineEnd + 2; + if (pending.length < pos + size + 2) return undefined; + if (size === 0) break; + parts.push(pending.subarray(pos, pos + size)); + pos += size + 2; + } + this.#bodyChunks = parts; + this.#pendingChunked = Buffer.alloc(0); + return pending.subarray(pos + 2); + } } type Target = ({ port: number; hostname: string } | { unix: string }) & { tls?: { ca: string } }; @@ -324,6 +355,68 @@ describe.each(transports)("$name", transport => { }); }, ); + + // The response ahead is still being produced by the app: a streaming body that + // ends when the test says so. Ending it makes the response sink resume() the + // socket itself (it releases a request-body pause), which must not reopen reads + // over the held request; the replay that follows is what reopens them. + it.if(transport.supported)( + "a request pipelined behind a streaming response is answered once the stream ends", + async () => { + using dir = tempDir("serve-pipelining", {}); + const hits: string[] = []; + const streaming = Promise.withResolvers(); + const finish = Promise.withResolvers(); + using server = Bun.serve({ + ...transport.listen(String(dir)), + fetch(req) { + const path = new URL(req.url).pathname; + hits.push(path); + if (path !== "/stream") return plainResponse(req); + let pulls = 0; + return new Response( + new ReadableStream({ + async pull(controller) { + if (pulls++ === 0) { + controller.enqueue("first,"); + streaming.resolve(); + return; + } + await finish.promise; + controller.enqueue("second"); + controller.close(); + }, + }), + ); + }, + }); + using client = await RawClient.connect(transport.target(server, String(dir))); + + client.write(request("/stream") + request("/after")); + await Promise.race([streaming.promise, client.until(c => c.closed)]); + await probe(transport, server, String(dir)); + expect({ hits, closed: client.closed }).toEqual({ hits: ["/stream", "/probe"], closed: false }); + + finish.resolve(); + await client.until(c => c.responses.length === 2); + expect({ + hits, + closed: client.closed, + responses: client.responses.map(({ statusLine, headers, body }) => ({ + statusLine, + framing: headers["transfer-encoding"] ?? `content-length ${headers["content-length"]}`, + body, + })), + }).toEqual({ + hits: ["/stream", "/probe", "/after"], + closed: false, + responses: [ + { statusLine: "HTTP/1.1 200 OK", framing: "chunked", body: "first,second" }, + { statusLine: "HTTP/1.1 200 OK", framing: "content-length 14", body: "body of /after" }, + ], + }); + }, + ); }); it("a request arriving in a later read while the handler is still running waits for the response", async () => { From b779b5f8a13c610d12d086b46017101b48d1f6e7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 02:33:23 +0000 Subject: [PATCH 07/22] uws: a connection with parked requests is not idle markDone() marked the connection idle as soon as the response completed, so a graceful server.stop() issued while that response was pending (which marks busy connections close-when-idle) closed the connection right there and dropped the request parked behind it, although it had been received in full. Count parked bytes like node:http's queued responses: the connection becomes idle at the markDone() of the last replayed request, and the close-when-idle mark takes effect there. --- packages/bun-uws/src/HttpResponseData.h | 9 +++++-- test/js/bun/http/bun-serve-pipelining.test.ts | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index fa983819ebd2..8dea8d25d606 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -59,8 +59,13 @@ struct HttpResponseData : AsyncSocketData, HttpParser { HttpResponseData *httpResponseData = uwsRes->getHttpResponseData(); /* A queued pipelined response (node:http) still owes output on this - * connection, so it is not idle between the responses. */ - httpResponseData->isIdle = httpResponseData->nodeHttpQueuedPipelinedCount == 0; + * connection, and parked request bytes are received work it still owes a + * dispatch, so it is not idle in either case: a closeIdle() sweep (graceful + * stop) leaves it alone or marks it close-when-idle, and that mark takes + * effect from the markDone() of the last replayed request instead of + * closing over the parked ones here. */ + httpResponseData->isIdle = httpResponseData->nodeHttpQueuedPipelinedCount == 0 + && this->parkedRequestBytes.isEmpty(); /* Requests are parked behind this response (Bun.serve pipelining). They * are replayed from HttpContext::onWritable rather than here: every caller diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 9c5cfddfaaf9..f9c2023014c0 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -569,6 +569,30 @@ describe("a request pipelined behind a Connection: close request", () => { }); }); +// A graceful stop() closes idle connections and marks busy ones to close once +// their work is done. A request that was received and held behind the response +// in flight is part of that work: it is answered, and the connection closes after +// it rather than over it. +it("a held request is still answered when the server is stopped gracefully while the response ahead of it is pending, then the connection closes", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ ...tcp, fetch: handler.fetch }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(request("/hold") + request("/after")); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + const stopped = server.stop(); + handler.release("/hold"); + await client.until(c => c.closed); + await stopped; + expect({ hits: handler.hits, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe", "/after"], + responses: [ok("body of /hold"), ok("body of /after")], + }); +}); + // upgrade() is instantiated once per socket flavor, like the parking code; the // unix transport shares the plain instantiation. describe.each(transports.filter(t => t.name !== "unix"))("WebSocket upgrade over $name", transport => { From 0369ed62201e7bd827665316891336351d7ca551 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 20:28:03 +0000 Subject: [PATCH 08/22] verify-baseline-static: allowlist llint_op_jmp_wide32 decode false positive The x64-musl baseline static scan flags one RDPMC (0f 33) in llint_op_jmp_wide32 on this branch and nothing on main: the documented LLInt data-in-.text desync (the embedded opcode id at the start of every handler) landed on this sibling after the .text layout shift from the uws/uSockets changes. A blanket pass, not a ceiling, since the bytes are data and what they decode as moves with the layout. --- scripts/verify-baseline-static/allowlist-x64.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/verify-baseline-static/allowlist-x64.txt b/scripts/verify-baseline-static/allowlist-x64.txt index 13819585cd13..de44a64f599d 100644 --- a/scripts/verify-baseline-static/allowlist-x64.txt +++ b/scripts/verify-baseline-static/allowlist-x64.txt @@ -1349,7 +1349,11 @@ jsimd_ycc_rgb_convert_avx2.return [AVX, AVX2] # single-digit hit here is decode noise. Ceiling [AVX] so a multi-feature leak # still fails. On ELF each opcode handler has its own symbol, so layout shifts # can move the desync between llint_op_* siblings; add them here as they trip. -# (2 symbols) +# llint_op_jmp_wide32 decoded as one RDPMC (0f 33), not VEX: the bytes are the +# embedded opcode id, and what they spell moves with the layout, so it is a +# blanket pass (the guide's rule for confirmed data-in-.text misdecodes). +# (3 symbols) # ---------------------------------------------------------------------------- llint_op_enter_wide32 [AVX] llint_op_wide16_wide16 [AVX] +llint_op_jmp_wide32 From 4fabf8db3599b1363294df54186fcc4802e05b64 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:35:18 +0000 Subject: [PATCH 09/22] Bun.serve: decide at dispatch, too, whether to hold the next request onData decides to hold what follows a request when the parser reports the end of that request's message. The parser reports no end for a head that declares Content-Length: 0 and was completed from the fallback buffer (a head split across reads). The request behind such a head, in the same read, then reached the request handler with the response ahead of it still pending: ASSERT_NOT_REACHED in a debug build, the close in a release build. Derive the flag when the request handler returns as well. The derivation at the end of the message still runs, because the handler can complete the response from inside the body callback. --- packages/bun-uws/src/HttpContext.h | 20 +++++++++----- packages/bun-uws/src/HttpParser.h | 9 ++++--- test/js/bun/http/bun-serve-pipelining.test.ts | 26 +++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index d1ba050c974b..81e04a53ab35 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -620,6 +620,15 @@ struct HttpContext { ((HttpResponse *) s)->resetTimeout(); } + /* Bun.serve: park what follows this request if the handler left its + * response pending. The body callback below derives this again at the + * end of the message, but the parser does not call it for every message: + * a head with Content-Length: 0 that was completed from the fallback + * buffer (split across reads) gets no end-of-message callback. */ + if constexpr (!IsNodeHttp) { + httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); + } + /* Continue parsing */ return s; @@ -693,12 +702,11 @@ struct HttpContext { } } - /* Bun.serve: the request message is complete (a bodiless request gets an - * empty fin right after dispatch), so the next request boundary is what - * the parser reaches next. The handler may have completed the response - * anywhere up to here, synchronously or from inside this body callback, - * which is why the decision to parse or park what follows is taken now - * and not at dispatch. */ + /* Bun.serve: the request message is complete, so the next request + * boundary is what the parser reaches next. The handler may have + * completed the response anywhere up to here, also from inside this + * body callback, so the decision taken at dispatch to parse or park + * what follows is taken again now. */ if constexpr (!IsNodeHttp) { if (fin) { httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index bf9964ec3e47..ae5096fbda1d 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -638,10 +638,11 @@ struct HttpResponseData; * also holds while bytes are already parked, so later reads queue up behind * them and replay (HttpContext::replayParkedRequestBytes) keeps wire order. * - * Bun.serve: HttpContext::onData derives it (at entry and after each - * request's body fin) from cannotDispatchAnotherRequest: requests pipelined - * behind a response that is still being produced or drained wait for it. - * Reads are paused while bytes are parked, bounding them to one recv. + * Bun.serve: HttpContext::onData derives it (at entry, after each dispatch + * and after each request's body fin) from cannotDispatchAnotherRequest: + * requests pipelined behind a response that is still being produced or + * drained wait for it. Reads are paused while bytes are parked, bounding + * them to one recv. * * node:http flood prevention: set on the pause edge alongside * HTTP_NODE_READS_PAUSED (which stays set through the replay) and cleared for diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index f9c2023014c0..3dc72a917734 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -471,6 +471,32 @@ it("a request pipelined behind a request body that the handler is still consumin }); }); +// The parser reports the end of a request message to the server, and that is +// where the server decides to hold what follows. It reports no end for a head +// that declares Content-Length: 0 and was completed from the parser's buffer for +// a head split across reads, so the decision must not depend on that report. +it("a request pipelined behind a split head with Content-Length: 0 waits for the response", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ ...tcp, fetch: handler.fetch }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write("POST /hold HTTP/1.1\r\nHost: x\r\nContent-Le"); + // The server has read the first part of the head once this is answered. + await probe(tcpOnly, server, ""); + client.write("ngth: 0\r\n\r\n" + request("/after")); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/probe", "/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.responses.length === 2); + expect({ hits: handler.hits, closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/probe", "/hold", "/probe", "/after"], + closed: false, + responses: [ok("body of /hold"), ok("body of /after")], + }); +}); + // The held bytes are re-parsed from the top when they are released: a request // with a body gets its body back, and whatever is behind it is held again while // that request's own response is pending (here until a Connection: close request From ca7f810122c5525e082c3d61481170cae6bb1387 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:35:32 +0000 Subject: [PATCH 10/22] uws: answer the held requests of a peer that has already sent its FIN Over TLS the peer's close_notify is decrypted in the same read as the requests in front of it, so onEnd runs while a request is held behind a response that is still draining. onEnd defers the close for the draining response (HTTP_NODE_RECEIVED_FIN), and the close gate of onWritable then closed the connection before the replay: the held request was received in full and never answered. shouldCloseConnection() now waits for the held bytes in its HTTP_NODE_RECEIVED_FIN branch, like the HTTP_CLOSE_WHEN_IDLE branch does through isIdle. The gate closes the connection after the last replayed response. Over plain TCP and unix sockets the loop defers the FIN while reads are paused, so those transports already answered both requests. The new test covers all three. --- packages/bun-uws/src/HttpResponseData.h | 6 ++- test/js/bun/http/bun-serve-pipelining.test.ts | 49 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 620cbfcffe0f..1d1b4b977f54 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -247,10 +247,12 @@ struct HttpResponseData : AsyncSocketData, HttpParser { uint32_t nodeHttpQueuedPipelinedCount = 0; /* Whether the connection should be torn down once the in-flight response (if - * any) has completed and all buffered outgoing data has been flushed. */ + * any) has completed and all buffered outgoing data has been flushed. A peer + * that sent its FIN still gets the answers to the requests it sent before + * it: the ones parked behind this response are replayed first. */ bool shouldCloseConnection() const { return (state & HTTP_CONNECTION_CLOSE) - || ((state & HTTP_NODE_RECEIVED_FIN) && nodeHttpQueuedPipelinedCount == 0) + || ((state & HTTP_NODE_RECEIVED_FIN) && nodeHttpQueuedPipelinedCount == 0 && this->parkedRequestBytes.isEmpty()) || ((state & HTTP_CLOSE_WHEN_IDLE) && this->isIdle); } }; diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 3dc72a917734..d986d8574bf7 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "bun:test"; import { isPosix, tempDir, tls } from "harness"; +import { once } from "node:events"; +import { connect as netConnect } from "node:net"; import { join } from "node:path"; +import { connect as tlsConnect } from "node:tls"; // A request pipelined behind a response that was still in flight (the handler had // not returned yet, or it had and uWS was still draining a body that did not fit @@ -417,6 +420,52 @@ describe.each(transports)("$name", transport => { }); }, ); + + // The client ends its side right behind the requests and reads on. Over TLS its + // close_notify is decrypted in the same read as the requests, so the server sees + // the end of the stream while /small is held and /big is still draining. The + // requests came before the end, so both are answered before the server closes. + it.if(transport.supported)( + "a request held behind a draining response is answered for a client that has already ended its side", + async () => { + using dir = tempDir("serve-pipelining", {}); + const hits: string[] = []; + using server = Bun.serve({ + ...transport.listen(String(dir)), + fetch: recordingHandler(hits, () => new Response(big)), + }); + const socket = + transport.name === "tls" + ? tlsConnect({ port: server.port!, host: "127.0.0.1", ca: tls.cert, rejectUnauthorized: false }) + : transport.name === "unix" + ? netConnect({ path: join(String(dir), "pipeline.sock") }) + : netConnect({ port: server.port!, host: "127.0.0.1" }); + const reader = new ResponseReader(); + socket.on("data", chunk => reader.push(chunk)); + // A reset shows up below as a missing response. + socket.on("error", () => {}); + await once(socket, transport.name === "tls" ? "secureConnect" : "connect"); + + const closed = once(socket, "close"); + socket.end(request("/big") + request("/small")); + await closed; + + expect({ + hits, + responses: reader.responses.map(({ statusLine, body }) => ({ + statusLine, + bodyLength: body.length, + bodyIsIntact: body === big || body === "body of /small", + })), + }).toEqual({ + hits: ["/big", "/small"], + responses: [ + { statusLine: "HTTP/1.1 200 OK", bodyLength: BIG_BODY_LENGTH, bodyIsIntact: true }, + { statusLine: "HTTP/1.1 200 OK", bodyLength: 14, bodyIsIntact: true }, + ], + }); + }, + ); }); it("a request arriving in a later read while the handler is still running waits for the response", async () => { From a8613dfd593a28fd371a7b1e3407c1590432cacd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:35:46 +0000 Subject: [PATCH 11/22] test: a throw after an await, a HEAD and Expect: 100-continue ahead of a held request Three ways the response ahead is produced that the file did not name: the 500 of a handler that throws after an await, the bodiless answer to a HEAD request, and a final response that a 100 Continue preceded (with the body in the same write as the head, and with the body sent after the 100). Each one fails on main the same way: the server closes the connection when it parses the request behind it. --- test/js/bun/http/bun-serve-pipelining.test.ts | 118 +++++++++++++++++- 1 file changed, 116 insertions(+), 2 deletions(-) diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index d986d8574bf7..81072281ea8e 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -26,6 +26,9 @@ class ResponseReader { // Bytes after the last complete response that do not form a head yet (after // a 101 these are WebSocket frames). unparsed: Buffer = Buffer.alloc(0); + // How many of the next responses answer a HEAD request: such a response has + // the framing headers of the GET response and no body (RFC 9112 6.3). + headResponses = 0; #head: { statusLine: string; headers: Record } | undefined; #chunked = false; #pendingChunked: Buffer = Buffer.alloc(0); @@ -46,8 +49,10 @@ class ResponseReader { headers[line.slice(0, colon).toLowerCase()] = line.slice(colon + 1).trim(); } this.#head = { statusLine, headers }; - this.#chunked = headers["transfer-encoding"] === "chunked"; - this.#bodyNeed = Number(headers["content-length"] ?? 0); + const hasBody = this.headResponses === 0; + if (!hasBody) this.headResponses--; + this.#chunked = hasBody && headers["transfer-encoding"] === "chunked"; + this.#bodyNeed = hasBody ? Number(headers["content-length"] ?? 0) : 0; chunk = this.unparsed.subarray(headEnd + 4); this.unparsed = Buffer.alloc(0); } @@ -520,6 +525,115 @@ it("a request pipelined behind a request body that the handler is still consumin }); }); +// Other ways the response ahead is produced. Each ends through its own path in +// the server, and each has to release the held request like a plain 200 does. +it("a request pipelined behind a handler that throws after an await is answered after the 500", async () => { + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const hits: string[] = []; + using server = Bun.serve({ + ...tcp, + async fetch(req) { + const path = new URL(req.url).pathname; + hits.push(path); + if (path !== "/throw") return plainResponse(req); + entered.resolve(); + await release.promise; + throw new Error("boom"); + }, + error: error => new Response(`handled ${error.message}`, { status: 500 }), + }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.write(request("/throw") + request("/after")); + await Promise.race([entered.promise, client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits, closed: client.closed }).toEqual({ hits: ["/throw", "/probe"], closed: false }); + + release.resolve(); + await client.until(c => c.responses.length === 2); + expect({ hits, closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + hits: ["/throw", "/probe", "/after"], + closed: false, + responses: [{ statusLine: "HTTP/1.1 500 Internal Server Error", body: "handled boom" }, ok("body of /after")], + }); +}); + +it("a request pipelined behind a HEAD request waits for its response", async () => { + const handler = holdingHandler(); + using server = Bun.serve({ ...tcp, fetch: handler.fetch }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + client.headResponses = 1; + client.write("HEAD /hold HTTP/1.1\r\nHost: x\r\n\r\n" + request("/after")); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/hold", "/probe"], closed: false }); + + handler.release("/hold"); + await client.until(c => c.responses.length === 2); + expect({ + hits: handler.hits, + closed: client.closed, + responses: client.responses.map(({ statusLine, headers, body }) => ({ + statusLine, + contentLength: headers["content-length"], + body, + })), + }).toEqual({ + hits: ["/hold", "/probe", "/after"], + closed: false, + responses: [ + { statusLine: "HTTP/1.1 200 OK", contentLength: "13", body: "" }, + { statusLine: "HTTP/1.1 200 OK", contentLength: "14", body: "body of /after" }, + ], + }); +}); + +// The server answers the Expect header with 100 Continue when it dispatches the +// request, so an interim response is already on the wire when the request behind +// it is held. +describe.each([ + { name: "whose body is in the same write", bodyWaitsFor100: false }, + { name: "whose body follows the 100 Continue", bodyWaitsFor100: true }, +])("a request pipelined behind a request with Expect: 100-continue $name", ({ bodyWaitsFor100 }) => { + it("waits for the final response", async () => { + const seen: string[] = []; + const bodyRead = Promise.withResolvers(); + const release = Promise.withResolvers(); + using server = Bun.serve({ + ...tcp, + async fetch(req) { + if (new URL(req.url).pathname !== "/upload") return plainResponse(req); + seen.push(await req.text()); + bodyRead.resolve(); + await release.promise; + return new Response(`uploaded ${seen[0]}`); + }, + }); + using client = await RawClient.connect(tcpOnly.target(server, "")); + + const head = "POST /upload HTTP/1.1\r\nHost: x\r\nExpect: 100-continue\r\nContent-Length: 5\r\n\r\n"; + if (bodyWaitsFor100) { + client.write(head); + await client.until(c => c.responses.length === 1); + client.write("hello" + request("/after")); + } else { + client.write(head + "hello" + request("/after")); + } + await Promise.race([bodyRead.promise, client.until(c => c.closed)]); + await probe(tcpOnly, server, ""); + expect({ seen, closed: client.closed }).toEqual({ seen: ["hello"], closed: false }); + + release.resolve(); + await client.until(c => c.responses.length === 3); + expect({ closed: client.closed, responses: client.responses.map(summarize) }).toEqual({ + closed: false, + responses: [{ statusLine: "HTTP/1.1 100 Continue", body: "" }, ok("uploaded hello"), ok("body of /after")], + }); + }); +}); + // The parser reports the end of a request message to the server, and that is // where the server decides to hold what follows. It reports no end for a head // that declares Content-Length: 0 and was completed from the parser's buffer for From ce32ef93f4073806b00d659eb6599c6fe2598b16 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:38:44 +0000 Subject: [PATCH 12/22] uws: replay parked requests from a cursor instead of copying the rest each time Each replay moved the parked bytes out, grew the vector for the parser's fence (a reallocation, because a fresh vector has no spare capacity) and, when the replayed request left its response pending, the parser copied everything behind it into a new vector. A read full of small requests behind slow responses was copied twice per request: about N x recv bytes for N requests, up to ~10 GB of memcpy for one 512 KiB read. A replay that parks again now gives its buffer back and only moves the start of the parked bytes (parkedRequestBytesStart). The buffer still belongs to the replay call while it is parsed, because a dispatch can close or upgrade the socket and destruct the HTTP state; the parser reaches it through replayedRequestBytes. One read is copied once, whatever the number of requests in it. node:http's flood prevention replays through the same function. parkedRequestBytesStart fits in padding; replayedRequestBytes adds 8 bytes to the per-connection HTTP state. --- packages/bun-uws/src/HttpContext.h | 25 ++++++++++++++++--------- packages/bun-uws/src/HttpParser.h | 28 +++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 81e04a53ab35..966e22a2025d 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -119,19 +119,26 @@ struct HttpContext { public: /* Re-feed the bytes HttpParser parked (parkedRequestBytes) through the same - * parse path fresh socket data takes. Takes the parked bytes, so a dispatch - * during the replay that parks again starts a fresh batch behind them. The - * caller has already decided what to do about the paused read side. Returns - * what onData returns: the socket, closed, or the WebSocket it was upgraded - * into. */ + * parse path fresh socket data takes. The buffer belongs to this call while + * it is parsed, because a dispatch can close or upgrade the socket and take + * the HTTP state with it. A dispatch that parks again takes the buffer back + * through replayedRequestBytes (HttpParser::parkRequestBytes). The caller has + * already decided what to do about the paused read side. Returns what onData + * returns: the socket, closed, or the WebSocket it was upgraded into. */ template static us_socket_t *replayParkedRequestBytes(us_socket_t *s) { auto *httpResponseData = reinterpret_cast *>(us_socket_ext(s)); - WTF::Vector parked = std::exchange(httpResponseData->parkedRequestBytes, {}); - size_t length = parked.size(); + WTF::Vector replayed = std::exchange(httpResponseData->parkedRequestBytes, {}); + size_t start = std::exchange(httpResponseData->parkedRequestBytesStart, 0); + size_t length = replayed.size() - start; /* The parser fences the buffer by writing past its logical end. */ - parked.grow(length + LIBUS_RECV_BUFFER_PADDING); - return onData(s, parked.mutableSpan().data(), static_cast(length)); + replayed.grow(replayed.size() + LIBUS_RECV_BUFFER_PADDING); + httpResponseData->replayedRequestBytes = &replayed; + us_socket_t *returned = onData(s, replayed.mutableSpan().data() + start, static_cast(length)); + if (!us_socket_is_closed(s) && us_socket_kind(s) == socketKind()) { + httpResponseData->replayedRequestBytes = nullptr; + } + return returned; } us_socket_group_t *getSocketGroup() { diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index ae5096fbda1d..4235649343e6 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -30,8 +30,10 @@ #include #include #include +#include #include #include +#include #include #include "MoveOnlyFunction.h" #include "ChunkedEncoding.h" @@ -651,8 +653,32 @@ struct HttpResponseData; bool nodeHttpSpillReplayScheduled = false; /* A request on this connection had Connection: close or was HTTP/1.0, or a Bun.serve response closed it (RFC 9112 9.6). */ bool sawConnectionClose = false; + /* The parked bytes are parkedRequestBytes from here on. Not 0 once a replay + * has parked again: it gives its buffer back with a new start instead of + * copying what it did not reach, so a long pipeline behind slow responses is + * copied once and not once per request. */ + unsigned int parkedRequestBytesStart = 0; WTF::Vector parkedRequestBytes; + /* The buffer HttpContext::replayParkedRequestBytes is feeding to the parser, + * for the time of that call. */ + WTF::Vector *replayedRequestBytes = nullptr; private: + /* Parks the rest of what is being parsed, [data, data + length). In a replay + * that is the tail of the replayed buffer: the buffer comes back whole, less + * the fence the replay added, and only its start moves. */ + void parkRequestBytes(char *data, unsigned int length) { + if (WTF::Vector *replayed = std::exchange(replayedRequestBytes, nullptr)) { + char *begin = replayed->mutableSpan().data(); + if (parkedRequestBytes.isEmpty() && std::greater_equal{}(data, begin) + && std::less_equal{}(data + length, begin + replayed->size())) { + parkedRequestBytesStart = (unsigned int) (data - begin); + replayed->shrink(parkedRequestBytesStart + length); + parkedRequestBytes = std::exchange(*replayed, {}); + return; + } + } + parkedRequestBytes.append(std::span(data, length)); + } /* This guy really has only 30 bits since we reserve two highest bits to chunked encoding parsing state */ uint64_t remainingStreamingBytes = 0; @@ -1167,7 +1193,7 @@ struct HttpResponseData; * so the caller does not spill it into the size-capped header fallback * buffer. */ if (parkAtNextBoundary || !parkedRequestBytes.isEmpty()) [[unlikely]] { - parkedRequestBytes.append(std::span(data, length)); + parkRequestBytes(data, length); consumedTotal += length; return HttpParserResult::success(consumedTotal, user); } From c5a70a7a24c806c57fd31955372218b9e146c3dc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:38:44 +0000 Subject: [PATCH 13/22] Bun.serve: give the frames held behind a later server.upgrade() to the WebSocket A client that does not wait for the 101 gets its first frames into the read of the upgrade request. Since #43153 an upgrade made during the request's dispatch hands the rest of that read to the WebSocket, and one made in a later turn of the event loop failed with a 400, because by then the HTTP parser had read the frame as the next request. With requests held behind a pending response the parser no longer sees those bytes: upgrade() dropped them, so the 101 went out and the frames were lost without a sign, which is the outcome #43153 removed. upgrade() now takes the held bytes before it destructs the HTTP state and dispatches them to the WebSocket after open, as the loop would have for a read of their own. That covers frames in the read of the request and frames that arrive in a read of their own before the upgrade. An HTTP request held behind the handshake is not a valid frame, so the WebSocket fails the connection; it is never dispatched as HTTP. websocket-server-upgrade-early-frames.test.ts pinned the 400 for the later-turn upgrade; that test now expects the frames, in both arrival shapes. --- packages/bun-uws/src/HttpResponse.h | 36 ++++-- test/js/bun/http/bun-serve-pipelining.test.ts | 113 ++++++++++++++---- ...socket-server-upgrade-early-frames.test.ts | 70 +++++++---- 3 files changed, 163 insertions(+), 56 deletions(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index f1ececa1d1d6..403d707af3e2 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -393,17 +393,19 @@ struct HttpResponse : public AsyncSocket { auto* responseData = getHttpResponseData(); - /* Request bytes parked behind this handshake (HttpParser::parkedRequestBytes) - * go down with the HTTP state destructed below, like bytes trailing a - * synchronous upgrade in the same read (a client may not send anything - * before the 101 anyway, RFC 6455 4.1). Parking paused reads; the adopted - * WebSocket needs them flowing, and us_socket_adopt keeps the flag. The - * resume re-arms writable too, so the WebSocket gets one drain callback with - * nothing to drain right after open; dropping the bytes before - * endUpgradeHandshake() keeps markDone() from arming a second one for a - * replay that cannot happen. */ - if (!responseData->parkedRequestBytes.isEmpty()) [[unlikely]] { - responseData->parkedRequestBytes.clear(); + /* Bytes parked behind this handshake (HttpParser::parkedRequestBytes) follow + * the upgrade request on the wire: frames of a client that did not wait for + * the 101 (RFC 6455 4.1). The WebSocket gets them after open, below, like the + * rest of the read of an upgrade made during the dispatch + * (HttpContext::onData). Taken here because the HTTP state that owns them is + * destructed below, and before endUpgradeHandshake() so that markDone() does + * not arm a replay dispatch for them. Parking paused reads; the WebSocket + * needs them flowing, and us_socket_adopt keeps the flag. The resume re-arms + * writable too, so the WebSocket gets one drain callback with nothing to + * drain. */ + WTF::Vector earlyFrames = std::exchange(responseData->parkedRequestBytes, {}); + size_t earlyFramesStart = std::exchange(responseData->parkedRequestBytesStart, 0); + if (!earlyFrames.isEmpty()) [[unlikely]] { Super::resume(); } @@ -490,6 +492,18 @@ struct HttpResponse : public AsyncSocket { webSocketContextData->openHandler(webSocket); } + if (!earlyFrames.isEmpty() && !us_socket_is_closed(usSocket) && !us_socket_is_shut_down(usSocket)) [[unlikely]] { + /* The frame parser writes on both sides of what it is given (a spilled + * frame head in front, unmasking in blocks behind), as it may in the + * loop's padded receive buffer. */ + size_t length = earlyFrames.size() - earlyFramesStart; + WTF::Vector padded; + padded.grow(LIBUS_RECV_BUFFER_PADDING + length + LIBUS_RECV_BUFFER_PADDING); + char *frames = padded.mutableSpan().data() + LIBUS_RECV_BUFFER_PADDING; + memcpy(frames, earlyFrames.span().data() + earlyFramesStart, length); + us_dispatch_data(usSocket, frames, (int) length); + } + return usSocket; } diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 81072281ea8e..0d3f80802c63 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -12,8 +12,8 @@ import { connect as tlsConnect } from "node:tls"; // second request never answered. Such a request is now held until the response // ahead of it completes and is dispatched then, so responses stay in request // order (RFC 9112 9.3.2); one held behind a Connection: close request is dropped -// with the connection (RFC 9112 9.6), and one held behind a request that turns -// the connection into a WebSocket is dropped with the HTTP state. +// with the connection (RFC 9112 9.6), and what is held behind a request that +// turns the connection into a WebSocket goes to that WebSocket, as frames. type RawResponse = { statusLine: string; headers: Record; body: string }; @@ -800,9 +800,19 @@ describe.each(transports.filter(t => t.name !== "unix"))("WebSocket upgrade over body, accept: headers["sec-websocket-accept"] as string | undefined, }); - // A masked text frame "hi" (mask key 1 2 3 4); the server echoes it unmasked. - const maskedHiFrame = new Uint8Array([0x81, 0x82, 1, 2, 3, 4, "h".charCodeAt(0) ^ 1, "i".charCodeAt(0) ^ 2]); - const echoedHiFrame = [0x81, 0x02, "h".charCodeAt(0), "i".charCodeAt(0)]; + // A masked text frame with a short payload (mask key 1 2 3 4); the server + // echoes it unmasked. + const maskedFrame = (payload: string) => + new Uint8Array([ + 0x81, + 0x80 | payload.length, + 1, + 2, + 3, + 4, + ...Buffer.from(payload).map((byte, i) => byte ^ ((i % 4) + 1)), + ]); + const echoedFrame = (payload: string) => [0x81, payload.length, ...Buffer.from(payload)]; // /ws is upgraded from the handler itself, or (held: true) from a continuation // the test releases; every other path is the holding handler's. @@ -828,10 +838,15 @@ describe.each(transports.filter(t => t.name !== "unix"))("WebSocket upgrade over return { server, upgradeEntered: entered.promise, releaseUpgrade: released.resolve }; } - async function expectEcho(client: RawClient) { - client.write(maskedHiFrame); - await client.until(c => c.unparsed.length >= echoedHiFrame.length); - expect({ closed: client.closed, frame: [...client.unparsed] }).toEqual({ closed: false, frame: echoedHiFrame }); + // Waits for the echo of the last payload, then expects everything after the + // 101 to be the echoes of `payloads`, in order. + async function expectEchoes(client: RawClient, payloads: string[]) { + const last = Buffer.from(echoedFrame(payloads.at(-1)!)); + await client.until(c => c.unparsed.subarray(-last.length).equals(last)); + expect({ closed: client.closed, frames: [...client.unparsed] }).toEqual({ + closed: false, + frames: payloads.flatMap(echoedFrame), + }); } it("pipelined behind an async handler is performed once the response ahead of it is out", async () => { @@ -853,20 +868,22 @@ describe.each(transports.filter(t => t.name !== "unix"))("WebSocket upgrade over }); // The connection is the WebSocket now. - await expectEcho(client); + client.write(maskedFrame("hi")); + await expectEchoes(client, ["hi"]); }); - // The request held behind the handshake is discarded with the HTTP state when - // the connection becomes a WebSocket (as bytes trailing a synchronous upgrade - // in the same read always were), and holding it must not leave the WebSocket's - // reads switched off. - it("performed by an async handler with a request pipelined behind it drops that request and reads frames", async () => { + // What follows an upgrade request on the wire is frames: a client that does not + // wait for the 101 (RFC 6455 4.1) gets them into the read of the request. They + // are held like anything behind a pending response, and the upgrade gives them + // to the WebSocket, as it does with the rest of the read when it runs during the + // request's dispatch. Holding them must not leave the WebSocket's reads off. + it("performed by an async handler gives the WebSocket the frames held behind the handshake, and reads on", async () => { const handler = holdingHandler(); const { upgradeEntered, releaseUpgrade, ...serving } = serveWithUpgrade(handler, { held: true }); using server = serving.server; using client = await RawClient.connect(transport.target(server, "")); - client.write(upgradeRequest + request("/never")); + client.write(Buffer.concat([Buffer.from(upgradeRequest, "latin1"), maskedFrame("hi")])); await Promise.race([upgradeEntered, client.until(c => c.closed)]); await probe(transport, server, ""); expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/ws", "/probe"], closed: false }); @@ -878,13 +895,65 @@ describe.each(transports.filter(t => t.name !== "unix"))("WebSocket upgrade over responses: [switching], }); - await expectEcho(client); - // The echo round trip above means the server has long since processed - // everything it received before the frame; /never was not part of it. + client.write(maskedFrame("yo")); + await expectEchoes(client, ["hi", "yo"]); + }); + + // Both at once: the upgrade request waits behind a pending response with a frame + // behind it, and is then performed by an async handler. The frame is held twice, + // the second time as what the replay of the upgrade request did not reach. + it("held behind an async handler and performed by an async handler still gives the WebSocket the frame behind it", async () => { + const handler = holdingHandler(); + const { upgradeEntered, releaseUpgrade, ...serving } = serveWithUpgrade(handler, { held: true }); + using server = serving.server; + using client = await RawClient.connect(transport.target(server, "")); + + client.write(Buffer.concat([Buffer.from(request("/hold") + upgradeRequest, "latin1"), maskedFrame("hi")])); + await Promise.race([handler.entered("/hold"), client.until(c => c.closed)]); await probe(transport, server, ""); - expect({ hits: handler.hits, responses: client.responses.length }).toEqual({ - hits: ["/ws", "/probe", "/probe"], - responses: 1, + handler.release("/hold"); + await Promise.race([upgradeEntered, client.until(c => c.closed)]); + await probe(transport, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ + hits: ["/hold", "/probe", "/ws", "/probe"], + closed: false, + }); + + releaseUpgrade(); + await client.until(c => c.responses.length === 2); + expect({ closed: client.closed, responses: client.responses.map(withAccept) }).toEqual({ + closed: false, + responses: [{ statusLine: "HTTP/1.1 200 OK", body: "body of /hold", accept: undefined }, switching], + }); + + client.write(maskedFrame("yo")); + await expectEchoes(client, ["hi", "yo"]); + }); + + // A request held behind the handshake is never dispatched as HTTP: the + // connection has left HTTP by then. As frames the bytes are not valid, so the + // WebSocket fails the connection. + it("performed by an async handler fails the connection when an HTTP request is held behind the handshake", async () => { + const handler = holdingHandler(); + const { upgradeEntered, releaseUpgrade, ...serving } = serveWithUpgrade(handler, { held: true }); + using server = serving.server; + using client = await RawClient.connect(transport.target(server, "")); + + client.write(upgradeRequest + request("/never")); + await Promise.race([upgradeEntered, client.until(c => c.closed)]); + await probe(transport, server, ""); + expect({ hits: handler.hits, closed: client.closed }).toEqual({ hits: ["/ws", "/probe"], closed: false }); + + releaseUpgrade(); + await client.until(c => c.closed); + expect({ + hits: handler.hits, + responses: client.responses.map(withAccept), + framesAfterThe101: [...client.unparsed], + }).toEqual({ + hits: ["/ws", "/probe"], + responses: [switching], + framesAfterThe101: [], }); }); }); diff --git a/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts b/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts index 4f735c83f4a3..d1d66876d07b 100644 --- a/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts +++ b/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts @@ -5,9 +5,11 @@ // went out, the connection stayed open, and the frames were never seen. If the // read ended inside a frame, the next read started in the middle of it and the // server closed the connection. The `ws` package on Node parses these bytes -// (it unshifts the 'upgrade' event's head into the socket). So does Bun.serve -// when server.upgrade() runs before the request's dispatch returns: in the -// handler itself, or after an await that needs no new turn of the event loop. +// (it unshifts the 'upgrade' event's head into the socket). So does Bun.serve: +// when server.upgrade() runs before the request's dispatch returns (in the +// handler itself, or after an await that needs no new turn of the event loop) +// the WebSocket gets the rest of the read, and when it runs later it gets what +// the server held behind the pending response in the meantime. import type { Server } from "bun"; import { serve } from "bun"; import { describe, expect, it } from "bun:test"; @@ -283,27 +285,49 @@ describe.concurrent("frames in the same read as the upgrade request", () => { }); // A server.upgrade() in a later turn of the event loop runs when the read is - // over. By then the HTTP parser has read the frame as the start of the next - // request and rejected it: the client gets a 400 and a closed connection. - // (Bytes that can still begin a request line wait in the parser's buffer - // instead, and the upgrade frees that buffer.) - it("fail the connection when server.upgrade() runs in a later turn of the event loop", async () => { - const events: string[] = []; - const upgradeResult = Promise.withResolvers(); - using server = echoServer(events, { - async fetch(req, srv) { - await new Promise(resolve => setImmediate(resolve)); - upgradeResult.resolve(srv.upgrade(req)); - }, - }); - using client = await rawClient(server.port); + // over. The server does not parse what follows a request while that request's + // response is pending, it holds it: frames in the read of the upgrade request, + // and frames in a read of their own that arrives before the upgrade. The + // upgrade gives what was held to the WebSocket. + it.each(["the same read as the request", "a read of their own before the upgrade"])( + "are delivered when server.upgrade() runs in a later turn of the event loop (frames in %s)", + async arrival => { + const events: string[] = []; + const waiting = Promise.withResolvers(); + const release = Promise.withResolvers(); + using server = echoServer(events, { + async fetch(req, srv) { + if (new URL(req.url).pathname === "/plain") return new Response("plain"); + waiting.resolve(); + await release.promise; + if (srv.upgrade(req)) return; + return new Response("no", { status: 400 }); + }, + }); + using client = await rawClient(server.port); - client.socket.write(Buffer.concat([Buffer.from(upgradeRequest), text("early")])); - expect(await client.status()).toBe("HTTP/1.1 400 Bad Request"); - await client.closed; - expect(await upgradeResult.promise).toBe(false); - expect(events).toEqual([]); - }); + const early = Buffer.concat([text("early"), ping("p")]); + if (arrival === "the same read as the request") { + client.socket.write(Buffer.concat([Buffer.from(upgradeRequest), early])); + await waiting.promise; + } else { + client.socket.write(upgradeRequest); + await waiting.promise; + client.socket.write(early); + } + // The frames were on their way before this request, so the server has read + // them by the time it answers it, with the upgrade still to come. + expect(await (await fetch(new URL("/plain", server.url))).text()).toBe("plain"); + expect(events).toEqual([]); + + release.resolve(); + expect(await client.status()).toBe("HTTP/1.1 101 Switching Protocols"); + client.socket.write(text("later")); + + expect(await client.framesUntil("text:echo:later")).toEqual(["text:echo:early", "pong:p", "text:echo:later"]); + expect(events).toEqual(["open", "message:early", "ping:p", "message:later"]); + }, + ); // server.upgrade() for one connection can run in a microtask of another // connection's request. The bytes after that other request are not frames From 348b1c93d713a8011c17d468a685f7b78365493b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:04:46 +0000 Subject: [PATCH 14/22] uws: survive a us_socket_resume() that closes the socket us_socket_resume() closes a socket whose poll the kernel does not take back, and the close destructs the HTTP state. The replay then went on to take the parked bytes out of that destructed state (a crash when the failure is injected), and upgrade() went on with the handshake and the adoption of a closed socket, so open fired and close never did. The replay returns when the resume closed the socket. upgrade() resumes after the adoption and after open, where a close is an ordinary WebSocket close: open, then close with 1006, and the held frames are not dispatched. --- packages/bun-uws/src/HttpContext.h | 5 +++++ packages/bun-uws/src/HttpResponse.h | 17 ++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 966e22a2025d..c65d64719c80 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -1007,6 +1007,11 @@ struct HttpContext { /* Paused by onData when it parked them; it pauses again if the replayed * dispatch leaves a response pending with more requests behind it. */ reinterpret_cast *>(s)->resume(); + /* us_socket_resume closes a socket that the kernel does not take back, + * and the HTTP state goes with it. */ + if (us_socket_is_closed(s)) { + return s; + } return replayParkedRequestBytes(s); } diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 403d707af3e2..6f07b6eec7ff 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -399,15 +399,9 @@ struct HttpResponse : public AsyncSocket { * rest of the read of an upgrade made during the dispatch * (HttpContext::onData). Taken here because the HTTP state that owns them is * destructed below, and before endUpgradeHandshake() so that markDone() does - * not arm a replay dispatch for them. Parking paused reads; the WebSocket - * needs them flowing, and us_socket_adopt keeps the flag. The resume re-arms - * writable too, so the WebSocket gets one drain callback with nothing to - * drain. */ + * not arm a replay dispatch for them. */ WTF::Vector earlyFrames = std::exchange(responseData->parkedRequestBytes, {}); size_t earlyFramesStart = std::exchange(responseData->parkedRequestBytesStart, 0); - if (!earlyFrames.isEmpty()) [[unlikely]] { - Super::resume(); - } endUpgradeHandshake(); @@ -492,6 +486,15 @@ struct HttpResponse : public AsyncSocket { webSocketContextData->openHandler(webSocket); } + if (!earlyFrames.isEmpty()) [[unlikely]] { + /* Parking paused reads, and us_socket_adopt keeps the flag. Resumed as a + * WebSocket and not before the adoption: us_socket_resume closes a socket + * that the kernel does not take back, which from here on is an ordinary + * WebSocket close. It re-arms writable too, so the WebSocket gets one + * drain callback with nothing to drain. */ + us_socket_resume(usSocket); + } + if (!earlyFrames.isEmpty() && !us_socket_is_closed(usSocket) && !us_socket_is_shut_down(usSocket)) [[unlikely]] { /* The frame parser writes on both sides of what it is given (a spilled * frame head in front, unmasking in blocks behind), as it may in the From b9acb38dba01c558b97cfec35aeee37b9653234c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:32:31 +0000 Subject: [PATCH 15/22] uws: cut the pipelining comments down to what the code cannot say Review asked for it: about 130 added comment lines across the uWS and uSockets changes, many of them narrating the neighbouring code. Each block now keeps only its invariant or the reason for an ordering. No code change. --- packages/bun-usockets/src/libusockets.h | 8 +-- packages/bun-usockets/src/socket.c | 5 +- packages/bun-uws/src/HttpContext.h | 80 +++++++------------------ packages/bun-uws/src/HttpParser.h | 40 ++++--------- packages/bun-uws/src/HttpResponse.h | 34 ++++------- packages/bun-uws/src/HttpResponseData.h | 20 ++----- 6 files changed, 57 insertions(+), 130 deletions(-) diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index 6b5cfd3b012d..5db941185a97 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -704,12 +704,8 @@ void us_socket_local_address(us_socket_r s, char *nonnull_arg buf, int *nonnull_ struct us_socket_t *us_socket_detach(us_socket_r s) nonnull_fn_decl; int us_socket_ipc_write_fd(us_socket_r s, const char *data, int length, int fd) nonnull_fn_decl; -/* Have on_writable dispatched once the socket is writable, as if a - * us_socket_write() had just come up short. For progress that has to be made - * from on_writable but was not queued through us_socket_write(): a sendfile - * that hit EAGAIN, HTTP request bytes parked behind a response that just - * completed. Safe to call from inside on_writable itself. Leaves a paused - * socket's read side alone. */ +/* Dispatches on_writable once the socket is writable, as if a us_socket_write() + * had come up short. Safe inside on_writable. Leaves a paused read side alone. */ void us_socket_request_writable(us_socket_r s) nonnull_fn_decl; void *us_listen_socket_ext(struct us_listen_socket_t *ls) nonnull_fn_decl; LIBUS_SOCKET_DESCRIPTOR us_listen_socket_get_fd(struct us_listen_socket_t *ls) nonnull_fn_decl; diff --git a/packages/bun-usockets/src/socket.c b/packages/bun-usockets/src/socket.c index cd53dadeddb5..f32adc0571ac 100644 --- a/packages/bun-usockets/src/socket.c +++ b/packages/bun-usockets/src/socket.c @@ -415,9 +415,8 @@ static void us_internal_rearm_writable(struct us_socket_t *s) { LIBUS_SOCKET_WRITABLE | ((s->flags.is_paused || s->read_eof) ? 0 : LIBUS_SOCKET_READABLE)); } -/* See libusockets.h. last_write_failed is what keeps the loop polling writable - * past the end of the dispatch this may be called from (loop.c drops the - * interest again after an on_writable that left it clear). */ +/* See libusockets.h. loop.c drops writable interest after an on_writable that + * left last_write_failed clear, so this sets it. */ void us_socket_request_writable(struct us_socket_t *s) { if (us_socket_is_closed(s)) return; s->flags.last_write_failed = 1; diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index c65d64719c80..3b8618cb7966 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -118,13 +118,9 @@ struct HttpContext { static unsigned char socketKind() { return SSL ? US_SOCKET_KIND_UWS_HTTP_TLS : US_SOCKET_KIND_UWS_HTTP; } public: - /* Re-feed the bytes HttpParser parked (parkedRequestBytes) through the same - * parse path fresh socket data takes. The buffer belongs to this call while - * it is parsed, because a dispatch can close or upgrade the socket and take - * the HTTP state with it. A dispatch that parks again takes the buffer back - * through replayedRequestBytes (HttpParser::parkRequestBytes). The caller has - * already decided what to do about the paused read side. Returns what onData - * returns: the socket, closed, or the WebSocket it was upgraded into. */ + /* Re-feeds parkedRequestBytes through onData and returns what it returns. The + * buffer belongs to this call while it is parsed: a dispatch can close or + * upgrade the socket, which destructs the HTTP state. */ template static us_socket_t *replayParkedRequestBytes(us_socket_t *s) { auto *httpResponseData = reinterpret_cast *>(us_socket_ext(s)); @@ -316,14 +312,8 @@ struct HttpContext { return us_socket_close(s, 0, nullptr); } - /* Bun.serve: whether the next request head on this connection must be parked - * instead of parsed (HttpParser::parkAtNextBoundary): the connection's one - * response slot is still taken. A connection that a complete response marked - * close is not parked: the sawConnectionClose latch and the close check at the - * top of the request handler discard what follows it (RFC 9112 9.6). Bytes - * parked behind a pending response that turns out to close the connection are - * never replayed: the only replay site runs behind onWritable's close gate, - * which fires on exactly the conditions the replay needs. */ + /* Bun.serve: a connection has one response slot. While it is taken, the next + * request head is parked instead of parsed (HttpParser::parkAtNextBoundary). */ static bool cannotDispatchAnotherRequest(HttpResponseData *httpResponseData) { return (httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) != 0; } @@ -421,12 +411,8 @@ struct HttpContext { httpContextData->parsingSocket = s; httpResponseData->isIdle = false; - /* Bun.serve: requests pipelined behind a response that is still pending - * (async handler, or a body the socket is still draining) are parked at - * the next request boundary and replayed from onWritable once it has - * completed. Re-derived on every read: a keep-alive request arriving - * after the response completed takes the ordinary path. Body bytes of the - * pending request itself never reach a boundary, so they are unaffected. */ + /* Bun.serve: derived on every read, so a request that arrives after the + * response completed takes the ordinary path. */ if constexpr (!IsNodeHttp) { httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); } @@ -497,12 +483,10 @@ struct HttpContext { if constexpr (IsNodeHttp) hasQueuedPipelinedResponses = httpResponseData->nodeHttpQueuedPipelinedCount > 0; if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) || hasQueuedPipelinedResponses) { if constexpr (!IsNodeHttp) { - /* Bun.serve has one response slot per connection, so the parser - * parks a head that arrives while it is taken (parkAtNextBoundary, - * maintained by onData) instead of getting here. Dispatching onto - * the in-flight response would interleave the two responses on the - * wire; closing is the backstop. close() first sends what earlier - * responses of this read left in the cork buffer. */ + /* The parser parks a head that arrives while the response slot is + * taken, so this is a backstop against interleaved responses. + * Responses that completed earlier in this read can still sit in + * the cork buffer. close() sends them first. */ ASSERT_NOT_REACHED(); ((AsyncSocket *) s)->close(); return nullptr; @@ -627,11 +611,8 @@ struct HttpContext { ((HttpResponse *) s)->resetTimeout(); } - /* Bun.serve: park what follows this request if the handler left its - * response pending. The body callback below derives this again at the - * end of the message, but the parser does not call it for every message: - * a head with Content-Length: 0 that was completed from the fallback - * buffer (split across reads) gets no end-of-message callback. */ + /* Bun.serve: derived here too, because a Content-Length: 0 head completed + * from the fallback buffer gets no end-of-message callback. */ if constexpr (!IsNodeHttp) { httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); } @@ -709,11 +690,8 @@ struct HttpContext { } } - /* Bun.serve: the request message is complete, so the next request - * boundary is what the parser reaches next. The handler may have - * completed the response anywhere up to here, also from inside this - * body callback, so the decision taken at dispatch to parse or park - * what follows is taken again now. */ + /* Bun.serve: the handler may have completed the response since the + * dispatch, also from inside this body callback. */ if constexpr (!IsNodeHttp) { if (fin) { httpResponseData->parkAtNextBoundary = cannotDispatchAnotherRequest(httpResponseData); @@ -787,14 +765,10 @@ struct HttpContext { ((HttpResponse *) s)->resetTimeout(); } - /* Bun.serve: requests are parked on this connection. Invariant kept - * here and in markDone(): reads are paused while anything is parked - * (bounding it to one recv), and a replaying writable dispatch - * (onWritable) is armed as soon as the response ahead of them is - * complete, whichever of the two happened last. AsyncSocket::pause - * rather than HttpResponse::pause: the in-flight response's timeout - * must stay armed against a peer that pipelines and then stops - * reading. */ + /* Bun.serve: reads stay paused while requests are parked, which bounds + * them to one recv. markDone() arms the replay, unless the response was + * complete before anything was parked. AsyncSocket::pause and not + * HttpResponse::pause: the pending response's timeout must stay armed. */ if constexpr (!IsNodeHttp) { if (!httpResponseData->parkedRequestBytes.isEmpty()) [[unlikely]] { ((AsyncSocket *) s)->pause(); @@ -986,14 +960,9 @@ struct HttpContext { return s; } - /* Bun.serve pipelining, replay half; the tail of every writable dispatch. - * Gets here either because the response the parked requests were waiting on - * completed inside callOnWritable above (a tryEnd tail draining), or via the - * dispatch markDone() / onData arm when it completed anywhere else. Nothing - * of the completed response is on the stack at this point, so the replayed - * request can take over the connection's response slot. Waits for the - * completed response's bytes to leave the buffer: the next dispatch is - * already owed while any are left. */ + /* Bun.serve: the tail of every writable dispatch. Nothing of the completed + * response is on the stack here, and onWritable's close gate has run, so a + * connection that is closing replays nothing. */ static us_socket_t *replayParkedRequestsIfResponseComplete(us_socket_t *s) { if (us_socket_is_closed(s) || us_socket_is_shut_down(s)) { return s; @@ -1004,11 +973,8 @@ struct HttpContext { || !reinterpret_cast *>(s)->hasFullyDrained()) { return s; } - /* Paused by onData when it parked them; it pauses again if the replayed - * dispatch leaves a response pending with more requests behind it. */ reinterpret_cast *>(s)->resume(); - /* us_socket_resume closes a socket that the kernel does not take back, - * and the HTTP state goes with it. */ + /* us_socket_resume closes a socket that the kernel does not take back. */ if (us_socket_is_closed(s)) { return s; } diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index 4235649343e6..4fde8db9d9a9 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -635,37 +635,23 @@ struct HttpResponseData; private: std::string fallback; public: - /* "The parse loop running now must stop at the next request boundary and park - * the rest of the buffer, unparsed, in parkedRequestBytes." The boundary check - * also holds while bytes are already parked, so later reads queue up behind - * them and replay (HttpContext::replayParkedRequestBytes) keeps wire order. - * - * Bun.serve: HttpContext::onData derives it (at entry, after each dispatch - * and after each request's body fin) from cannotDispatchAnotherRequest: - * requests pipelined behind a response that is still being produced or - * drained wait for it. Reads are paused while bytes are parked, bounding - * them to one recv. - * - * node:http flood prevention: set on the pause edge alongside - * HTTP_NODE_READS_PAUSED (which stays set through the replay) and cleared for - * the replay so it can make progress. */ + /* The parse loop must stop at the next request boundary and park the rest in + * parkedRequestBytes. Bun.serve: a response is pending (HttpContext::onData + * derives it). node:http: set on the flood-prevention pause edge, cleared for + * the replay so it can make progress (HTTP_NODE_READS_PAUSED stays set). */ bool parkAtNextBoundary = false; bool nodeHttpSpillReplayScheduled = false; /* A request on this connection had Connection: close or was HTTP/1.0, or a Bun.serve response closed it (RFC 9112 9.6). */ bool sawConnectionClose = false; - /* The parked bytes are parkedRequestBytes from here on. Not 0 once a replay - * has parked again: it gives its buffer back with a new start instead of - * copying what it did not reach, so a long pipeline behind slow responses is - * copied once and not once per request. */ + /* Where the parked bytes start in parkedRequestBytes: a replay that parks + * again gives its buffer back instead of copying what it did not reach. */ unsigned int parkedRequestBytesStart = 0; WTF::Vector parkedRequestBytes; - /* The buffer HttpContext::replayParkedRequestBytes is feeding to the parser, - * for the time of that call. */ + /* The buffer being replayed, during HttpContext::replayParkedRequestBytes. */ WTF::Vector *replayedRequestBytes = nullptr; private: - /* Parks the rest of what is being parsed, [data, data + length). In a replay - * that is the tail of the replayed buffer: the buffer comes back whole, less - * the fence the replay added, and only its start moves. */ + /* In a replay, [data, data + length) is the tail of the replayed buffer: it + * comes back whole, less the replay's fence, and only the start moves. */ void parkRequestBytes(char *data, unsigned int length) { if (WTF::Vector *replayed = std::exchange(replayedRequestBytes, nullptr)) { char *begin = replayed->mutableSpan().data(); @@ -1187,11 +1173,9 @@ struct HttpResponseData; consumedTotal += length; return HttpParserResult::success(consumedTotal, returnedUser); } - /* This connection cannot take another request right now (see - * parkAtNextBoundary). Stop at this request boundary, before getHeaders - * touches the next head, park the rest verbatim and report it as consumed - * so the caller does not spill it into the size-capped header fallback - * buffer. */ + /* Before getHeaders touches the next head. Reported as consumed so the + * caller does not spill it into the size-capped fallback buffer. Reads + * that arrive while bytes are parked go behind them, to keep wire order. */ if (parkAtNextBoundary || !parkedRequestBytes.isEmpty()) [[unlikely]] { parkRequestBytes(data, length); consumedTotal += length; diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 6f07b6eec7ff..4a35d2d5157c 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -393,13 +393,9 @@ struct HttpResponse : public AsyncSocket { auto* responseData = getHttpResponseData(); - /* Bytes parked behind this handshake (HttpParser::parkedRequestBytes) follow - * the upgrade request on the wire: frames of a client that did not wait for - * the 101 (RFC 6455 4.1). The WebSocket gets them after open, below, like the - * rest of the read of an upgrade made during the dispatch - * (HttpContext::onData). Taken here because the HTTP state that owns them is - * destructed below, and before endUpgradeHandshake() so that markDone() does - * not arm a replay dispatch for them. */ + /* Bytes parked behind this handshake are frames of a client that did not + * wait for the 101. Taken before markDone() arms a replay for them and before + * the HTTP state that owns them is destructed. Dispatched after open. */ WTF::Vector earlyFrames = std::exchange(responseData->parkedRequestBytes, {}); size_t earlyFramesStart = std::exchange(responseData->parkedRequestBytesStart, 0); @@ -487,18 +483,16 @@ struct HttpResponse : public AsyncSocket { } if (!earlyFrames.isEmpty()) [[unlikely]] { - /* Parking paused reads, and us_socket_adopt keeps the flag. Resumed as a - * WebSocket and not before the adoption: us_socket_resume closes a socket - * that the kernel does not take back, which from here on is an ordinary - * WebSocket close. It re-arms writable too, so the WebSocket gets one - * drain callback with nothing to drain. */ + /* Parking paused reads, and us_socket_adopt keeps the flag. Not resumed + * before the adoption: us_socket_resume can close the socket, which from + * here on is an ordinary WebSocket close. It also re-arms writable, so + * one drain callback with nothing to drain follows. */ us_socket_resume(usSocket); } if (!earlyFrames.isEmpty() && !us_socket_is_closed(usSocket) && !us_socket_is_shut_down(usSocket)) [[unlikely]] { - /* The frame parser writes on both sides of what it is given (a spilled - * frame head in front, unmasking in blocks behind), as it may in the - * loop's padded receive buffer. */ + /* The frame parser may write on both sides of its input, as it can in + * the loop's padded receive buffer. */ size_t length = earlyFrames.size() - earlyFramesStart; WTF::Vector padded; padded.grow(LIBUS_RECV_BUFFER_PADDING + length + LIBUS_RECV_BUFFER_PADDING); @@ -526,13 +520,9 @@ struct HttpResponse : public AsyncSocket { } HttpResponse *resume() { - /* While requests are parked behind this response the pause belongs to the - * pipelining code (HttpContext resumes when it replays them, upgrade() when - * it drops them). The resumes arriving here release a request-body - * backpressure pause and can land after the body completed and the bytes - * behind it were parked; reading on would only queue more behind them, or - * take a FIN that closes the connection over them. node:http's flood - * prevention only gets here once its parked bytes are gone. */ + /* While requests are parked the pause belongs to the replay. A resume that + * releases a request-body pause can land after they were parked; reading on + * would queue more behind them, or take a FIN that closes over them. */ if (getHttpResponseData()->parkedRequestBytes.isEmpty()) [[likely]] { Super::resume(); } diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 1d1b4b977f54..043659d4762a 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -59,21 +59,14 @@ struct HttpResponseData : AsyncSocketData, HttpParser { HttpResponseData *httpResponseData = uwsRes->getHttpResponseData(); /* A queued pipelined response (node:http) still owes output on this - * connection, and parked request bytes are received work it still owes a - * dispatch, so it is not idle in either case: a closeIdle() sweep (graceful - * stop) leaves it alone or marks it close-when-idle, and that mark takes - * effect from the markDone() of the last replayed request instead of - * closing over the parked ones here. */ + * connection, and parked requests are still owed a dispatch, so it is not + * idle: a graceful stop closes it after the last of them, not here. */ httpResponseData->isIdle = httpResponseData->nodeHttpQueuedPipelinedCount == 0 && this->parkedRequestBytes.isEmpty(); - /* Requests are parked behind this response (Bun.serve pipelining). They - * are replayed from HttpContext::onWritable rather than here: every caller - * (internalEnd, the uws_res_end* wrappers, the request context above them) - * still tears this response down after we return, and a dispatch now would - * land in the middle of that. node:http parks too (flood prevention); its - * onWritable hook tolerates the extra writable event and decides about - * replaying itself. */ + /* Parked requests are replayed from HttpContext::onWritable and not here: + * every caller still tears this response down after we return. node:http's + * onWritable hook tolerates the extra dispatch. */ if (!this->parkedRequestBytes.isEmpty()) [[unlikely]] { us_socket_request_writable((us_socket_t *) uwsRes); } @@ -248,8 +241,7 @@ struct HttpResponseData : AsyncSocketData, HttpParser { /* Whether the connection should be torn down once the in-flight response (if * any) has completed and all buffered outgoing data has been flushed. A peer - * that sent its FIN still gets the answers to the requests it sent before - * it: the ones parked behind this response are replayed first. */ + * that sent its FIN still gets the requests it sent before it answered. */ bool shouldCloseConnection() const { return (state & HTTP_CONNECTION_CLOSE) || ((state & HTTP_NODE_RECEIVED_FIN) && nodeHttpQueuedPipelinedCount == 0 && this->parkedRequestBytes.isEmpty()) From 4a66fc42cfb756a05ad39e5f9f046a908cb3370d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:28:58 +0000 Subject: [PATCH 16/22] Bun.serve: drop what arrives behind a closing request before the hold can park it The parser checked the hold before the sawConnectionClose latch. A request with Connection: close (or HTTP/1.0) whose response is pending took the hold: bytes that arrived behind it were parked and reads were paused. Bytes that arrived after that stayed unread, and the close that follows the response then resets the connection. A unix socket reports that to the client behind the complete response (ECONNRESET), where main ends the stream cleanly. For Bun.serve the latch is now checked first, as on main: the connection keeps reading and drops the bytes, because nothing behind a closing request is ever dispatched (RFC 9112 9.6). node:http keeps its order, where the flood-prevention park comes before llhttp's closed state. --- packages/bun-uws/src/HttpParser.h | 13 +++++-- test/js/bun/http/bun-serve-pipelining.test.ts | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/bun-uws/src/HttpParser.h b/packages/bun-uws/src/HttpParser.h index 4fde8db9d9a9..92e9ef39e617 100644 --- a/packages/bun-uws/src/HttpParser.h +++ b/packages/bun-uws/src/HttpParser.h @@ -1173,6 +1173,14 @@ struct HttpResponseData; consumedTotal += length; return HttpParserResult::success(consumedTotal, returnedUser); } + /* Bun.serve: a closing connection takes nothing more (RFC 9112 9.6). Ahead + * of the park, which pauses reads: a close over bytes left unread resets + * the connection behind the complete response. */ + if constexpr (!IsNodeHttp) { + if (sawConnectionClose) [[unlikely]] { + return HttpParserResult::success(consumedTotal + length, user); + } + } /* Before getHeaders touches the next head. Reported as consumed so the * caller does not spill it into the size-capped fallback buffer. Reads * that arrive while bytes are parked go behind them, to keep wire order. */ @@ -1203,11 +1211,10 @@ struct HttpResponseData; } } /* Must stay below the tunnel check, the park and the CR/LF skip, like llhttp's closed state. */ - if (sawConnectionClose) { - if constexpr (IsNodeHttp) { + if constexpr (IsNodeHttp) { + if (sawConnectionClose) { return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_CLOSED_CONNECTION); } - return HttpParserResult::success(consumedTotal + length, user); } auto result = getHeaders(data, data + length, req->headers, req->ancientHttp, isConnectRequest, useStrictMethodValidation, useInsecureHTTPParser, maxHeaderSize); if(result.isError()) { diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 0d3f80802c63..564a9a7367ef 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -756,6 +756,42 @@ describe("a request pipelined behind a Connection: close request", () => { responses: [ok("body of /hold")], }); }); + + // The server drops what arrives behind the closing request, and it has to keep + // reading to drop it. A close over bytes that were left unread resets the + // connection, and a unix socket reports that to the client behind the complete + // response. + it.if(isPosix)("is read and dropped while the response is pending, so the connection ends cleanly", async () => { + using dir = tempDir("serve-pipelining", {}); + const unix = transports.find(transport => transport.name === "unix")!; + const handler = holdingHandler(); + using server = Bun.serve({ ...unix.listen(String(dir)), fetch: handler.fetch }); + const socket = netConnect({ path: join(String(dir), "pipeline.sock") }); + const reader = new ResponseReader(); + const seen: { ended: boolean; error?: string } = { ended: false }; + socket.on("data", chunk => reader.push(chunk)); + socket.on("end", () => (seen.ended = true)); + socket.on("error", (error: NodeJS.ErrnoException) => (seen.error = error.code)); + const closed = new Promise(resolve => socket.on("close", () => resolve())); + await once(socket, "connect"); + const write = (data: string) => new Promise(resolve => socket.write(data, () => resolve())); + + await write(request("/hold", "Connection: close\r\n")); + await handler.entered("/hold"); + // Two later reads. The server has taken each one when its probe is answered. + for (let i = 0; i < 2; i++) { + await write(request("/never")); + await probe(unix, server, String(dir)); + } + + handler.release("/hold"); + await closed; + expect({ hits: handler.hits, seen, responses: reader.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe", "/probe"], + seen: { ended: true }, + responses: [ok("body of /hold")], + }); + }); }); // A graceful stop() closes idle connections and marks busy ones to close once From 55af3658c5b2316719d92173452eaf8403e4e281 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:25:44 +0000 Subject: [PATCH 17/22] Bun.serve: read what is unread behind parked requests before a close Reads are paused while requests are parked, so what the peer writes after that stays unread in the kernel. The close gates then close over unread bytes. That resets the connection, and the kernel drops the part of the response that it has not sent yet. Two shapes reach that state. An awaited handler answers with a Connection: close response header while the client writes more requests in two later writes: over TCP and TLS the client gets 6.6 MB of an 8 MiB body and ECONNRESET, over a unix socket the whole body and ECONNRESET. A Connection: close request that is itself parked, with a later write behind it: both responses arrive, then ECONNRESET on a unix socket. Main loses the whole response in both shapes. The three close gates and the parse-error close now read and drop those bytes first (us_socket_discard_unread, at most 8 MiB, which a socket receive buffer bounds). A connection that closes dispatches none of them. Only for Bun.serve, and only when requests are parked or a replay is on the stack. --- packages/bun-usockets/src/libusockets.h | 4 + packages/bun-usockets/src/socket.c | 18 +++ packages/bun-uws/src/HttpContext.h | 5 + packages/bun-uws/src/HttpResponse.h | 16 +++ test/js/bun/http/bun-serve-pipelining.test.ts | 134 ++++++++++++++---- 5 files changed, 147 insertions(+), 30 deletions(-) diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index 5db941185a97..c00fb06c7ead 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -707,6 +707,10 @@ int us_socket_ipc_write_fd(us_socket_r s, const char *data, int length, int fd) /* Dispatches on_writable once the socket is writable, as if a us_socket_write() * had come up short. Safe inside on_writable. Leaves a paused read side alone. */ void us_socket_request_writable(us_socket_r s) nonnull_fn_decl; +/* Reads and drops up to max_bytes of what the peer already sent, without a + * dispatch. For a close while reads are paused: close() over unread bytes resets + * the connection, and the kernel then drops what it has not sent yet. */ +void us_socket_discard_unread(us_socket_r s, unsigned int max_bytes) nonnull_fn_decl; void *us_listen_socket_ext(struct us_listen_socket_t *ls) nonnull_fn_decl; LIBUS_SOCKET_DESCRIPTOR us_listen_socket_get_fd(struct us_listen_socket_t *ls) nonnull_fn_decl; int us_listen_socket_port(struct us_listen_socket_t *ls) nonnull_fn_decl; diff --git a/packages/bun-usockets/src/socket.c b/packages/bun-usockets/src/socket.c index f32adc0571ac..1c558241d28e 100644 --- a/packages/bun-usockets/src/socket.c +++ b/packages/bun-usockets/src/socket.c @@ -423,6 +423,24 @@ void us_socket_request_writable(struct us_socket_t *s) { us_internal_rearm_writable(s); } +/* See libusockets.h. Reads the raw socket: for TLS the dropped bytes are + * ciphertext, which is fine for a socket that is closed next. */ +void us_socket_discard_unread(struct us_socket_t *s, unsigned int max_bytes) { + if (us_socket_is_closed(s)) return; +#ifdef _WIN32 + const int recv_flags = MSG_PUSH_IMMEDIATE; +#else + const int recv_flags = MSG_DONTWAIT; +#endif + char buf[16 * 1024]; + while (max_bytes) { + int length = max_bytes < sizeof(buf) ? (int) max_bytes : (int) sizeof(buf); + ssize_t received = bsd_recv(us_poll_fd(&s->p), buf, length, recv_flags); + if (received <= 0) break; + max_bytes -= (unsigned int) received; + } +} + /* See libusockets.h: whether a zero-progress write on a writable event proves * the peer is gone. Only the libuv backend has to ask the kernel. */ int us_socket_stalled_write_means_peer_gone(struct us_socket_t *s) { diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 3b8618cb7966..4792f7352359 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -734,6 +734,9 @@ struct HttpContext { ((AsyncSocket *) s)->uncork(); /* For errors, we only deliver them "at most once". We don't care if they get halfways delivered or not. */ us_socket_write(s, httpErrorResponses[httpErrorStatusCode].data(), (int) httpErrorResponses[httpErrorStatusCode].length()); + if constexpr (!IsNodeHttp) { + ((HttpResponse *) s)->discardBytesUnreadBehindParkedRequests(httpResponseData); + } us_socket_shutdown(s); /* Close any socket on HTTP errors */ us_socket_close(s, 0, nullptr); @@ -782,6 +785,7 @@ struct HttpContext { if (httpResponseData->shouldCloseConnection()) { if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { if (((AsyncSocket *) s)->hasFullyDrained()) { + ((HttpResponse *) s)->discardBytesUnreadBehindParkedRequests(httpResponseData); ((AsyncSocket *) s)->shutdown(); /* We need to force close after sending FIN since we want to hinder * clients from keeping to send their huge data */ @@ -944,6 +948,7 @@ struct HttpContext { } } if (responseDone && asyncSocket->hasFullyDrained()) { + reinterpret_cast *>(s)->discardBytesUnreadBehindParkedRequests(httpResponseData); asyncSocket->shutdown(); /* We need to force close after sending FIN since we want to hinder * clients from keeping to send their huge data */ diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 4a35d2d5157c..521372db7113 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -92,6 +92,21 @@ struct HttpResponse : public AsyncSocket { getHttpResponseData()->state |= HttpResponseData::HTTP_WROTE_DATE_HEADER; } + /* Bun.serve, before a close. While requests are parked reads are paused, so + * what the peer sent since then is unread. A close over unread bytes resets + * the connection, and the kernel then drops what it has not sent yet: the + * peer loses the end of a complete response. A connection that closes + * dispatches none of those bytes, so they are read and dropped. A socket + * receive buffer bounds them. */ + void discardBytesUnreadBehindParkedRequests(HttpResponseData *httpResponseData) { + if (httpResponseData->parkedRequestBytes.isEmpty() && !httpResponseData->replayedRequestBytes) [[likely]] { + return; + } + if (!HttpContext::fromSocket((us_socket_t *) this)->isNodeHttp()) { + us_socket_discard_unread((us_socket_t *) this, 8 * 1024 * 1024); + } + } + /* Shutdown+close when the connection is marked to close (Connection: * close, peer FIN, close-when-idle), the response is complete and every * outgoing byte has been flushed. Returns true when the socket was closed. */ @@ -99,6 +114,7 @@ struct HttpResponse : public AsyncSocket { if (httpResponseData->shouldCloseConnection()) { if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { if (((AsyncSocket *) this)->hasFullyDrained()) { + discardBytesUnreadBehindParkedRequests(httpResponseData); ((AsyncSocket *) this)->shutdown(); /* We need to force close after sending FIN since we want to hinder * clients from keeping to send their huge data */ diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 564a9a7367ef..e1bd8b8bac72 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -201,6 +201,27 @@ const summarize = ({ statusLine, body }: RawResponse) => ({ statusLine, body }); // Every handler below answers any path it does not treat specially with this. const plainResponse = (req: Request) => new Response(`body of ${new URL(req.url).pathname}`); +// A node:net or node:tls client, for the tests that look at how the stream ends: +// with the server's FIN (`ended`) or with an error such as ECONNRESET. +async function connectNodeSocket(transport: Transport, server: Bun.Server, dir: string) { + const socket = + transport.name === "tls" + ? tlsConnect({ port: server.port!, host: "127.0.0.1", ca: tls.cert, rejectUnauthorized: false }) + : transport.name === "unix" + ? netConnect({ path: join(dir, "pipeline.sock") }) + : netConnect({ port: server.port!, host: "127.0.0.1" }); + const reader = new ResponseReader(); + const seen: { ended: boolean; error?: string } = { ended: false }; + socket.on("data", chunk => reader.push(chunk)); + socket.on("end", () => (seen.ended = true)); + socket.on("error", (error: NodeJS.ErrnoException) => (seen.error = error.code)); + const closed = new Promise(resolve => socket.on("close", () => resolve())); + await once(socket, transport.name === "tls" ? "secureConnect" : "connect"); + // Resolves once the kernel has the bytes. + const write = (data: string) => new Promise(resolve => socket.write(data, () => resolve())); + return { socket, reader, seen, closed, write }; +} + // A round trip on a separate connection. Anything the pipelining client wrote // before this was readable on the server before the probe was even sent, so by // the time the probe has been answered the server has read it (and, with the @@ -439,25 +460,14 @@ describe.each(transports)("$name", transport => { ...transport.listen(String(dir)), fetch: recordingHandler(hits, () => new Response(big)), }); - const socket = - transport.name === "tls" - ? tlsConnect({ port: server.port!, host: "127.0.0.1", ca: tls.cert, rejectUnauthorized: false }) - : transport.name === "unix" - ? netConnect({ path: join(String(dir), "pipeline.sock") }) - : netConnect({ port: server.port!, host: "127.0.0.1" }); - const reader = new ResponseReader(); - socket.on("data", chunk => reader.push(chunk)); - // A reset shows up below as a missing response. - socket.on("error", () => {}); - await once(socket, transport.name === "tls" ? "secureConnect" : "connect"); - - const closed = once(socket, "close"); - socket.end(request("/big") + request("/small")); - await closed; + const client = await connectNodeSocket(transport, server, String(dir)); + + client.socket.end(request("/big") + request("/small")); + await client.closed; expect({ hits, - responses: reader.responses.map(({ statusLine, body }) => ({ + responses: client.reader.responses.map(({ statusLine, body }) => ({ statusLine, bodyLength: body.length, bodyIsIntact: body === big || body === "body of /small", @@ -471,6 +481,78 @@ describe.each(transports)("$name", transport => { }); }, ); + + // Reads are paused while a request is held, so what the client writes after that + // stays unread. A close over unread bytes resets the connection, and the kernel + // then drops the part of the response that it has not sent yet. The two tests + // below close a connection in that state: the server has to read those bytes + // first. In the first test the first /never is the request that is held, and the + // second one is what stays unread. + it.if(transport.supported)( + "a response that closes the connection arrives whole when the client wrote more while it was pending", + async () => { + using dir = tempDir("serve-pipelining", {}); + const handler = holdingHandler(); + using server = Bun.serve({ + ...transport.listen(String(dir)), + async fetch(req) { + const response = await handler.fetch(req); + return new URL(req.url).pathname === "/hold" + ? new Response(big, { headers: { Connection: "close" } }) + : response; + }, + }); + const client = await connectNodeSocket(transport, server, String(dir)); + + await client.write(request("/hold")); + await handler.entered("/hold"); + for (let i = 0; i < 2; i++) { + await client.write(request("/never")); + await probe(transport, server, String(dir)); + } + + handler.release("/hold"); + await client.closed; + expect({ + hits: handler.hits, + seen: client.seen, + responses: client.reader.responses.map(({ body }) => ({ bodyLength: body.length, bodyIsIntact: body === big })), + }).toEqual({ + hits: ["/hold", "/probe", "/probe"], + seen: { ended: true }, + responses: [{ bodyLength: BIG_BODY_LENGTH, bodyIsIntact: true }], + }); + }, + ); + + it.if(transport.supported)( + "a held Connection: close request is answered and the connection ends cleanly when the client wrote more meanwhile", + async () => { + using dir = tempDir("serve-pipelining", {}); + const handler = holdingHandler(); + using server = Bun.serve({ ...transport.listen(String(dir)), fetch: handler.fetch }); + const client = await connectNodeSocket(transport, server, String(dir)); + + await client.write(request("/hold") + request("/closing", "Connection: close\r\n")); + await handler.entered("/hold"); + for (let i = 0; i < 2; i++) { + await client.write(request("/never")); + await probe(transport, server, String(dir)); + } + + handler.release("/hold"); + await client.closed; + expect({ + hits: handler.hits, + seen: client.seen, + responses: client.reader.responses.map(summarize), + }).toEqual({ + hits: ["/hold", "/probe", "/probe", "/closing"], + seen: { ended: true }, + responses: [ok("body of /hold"), ok("body of /closing")], + }); + }, + ); }); it("a request arriving in a later read while the handler is still running waits for the response", async () => { @@ -766,27 +848,19 @@ describe("a request pipelined behind a Connection: close request", () => { const unix = transports.find(transport => transport.name === "unix")!; const handler = holdingHandler(); using server = Bun.serve({ ...unix.listen(String(dir)), fetch: handler.fetch }); - const socket = netConnect({ path: join(String(dir), "pipeline.sock") }); - const reader = new ResponseReader(); - const seen: { ended: boolean; error?: string } = { ended: false }; - socket.on("data", chunk => reader.push(chunk)); - socket.on("end", () => (seen.ended = true)); - socket.on("error", (error: NodeJS.ErrnoException) => (seen.error = error.code)); - const closed = new Promise(resolve => socket.on("close", () => resolve())); - await once(socket, "connect"); - const write = (data: string) => new Promise(resolve => socket.write(data, () => resolve())); - - await write(request("/hold", "Connection: close\r\n")); + const client = await connectNodeSocket(unix, server, String(dir)); + + await client.write(request("/hold", "Connection: close\r\n")); await handler.entered("/hold"); // Two later reads. The server has taken each one when its probe is answered. for (let i = 0; i < 2; i++) { - await write(request("/never")); + await client.write(request("/never")); await probe(unix, server, String(dir)); } handler.release("/hold"); - await closed; - expect({ hits: handler.hits, seen, responses: reader.responses.map(summarize) }).toEqual({ + await client.closed; + expect({ hits: handler.hits, seen: client.seen, responses: client.reader.responses.map(summarize) }).toEqual({ hits: ["/hold", "/probe", "/probe"], seen: { ended: true }, responses: [ok("body of /hold")], From 480dfe8dbfe0211491f9b840494e46d423955fd9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:55:09 +0000 Subject: [PATCH 18/22] uws: the close in cork() reads what is unread behind parked requests, too HttpResponse::cork() had its own copy of the close gate, without the read that the other gates got in 55af3658c5. A response without a body (HEAD, 304) completes while its socket is still corked, so its close runs from there: with a request held behind it and more bytes unread, the close reset the connection (ECONNRESET on a unix socket behind the response). cork() now calls closeIfDoneAndMarked(), which is the same gate with the read. --- packages/bun-uws/src/HttpResponse.h | 12 +---- test/js/bun/http/bun-serve-pipelining.test.ts | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 521372db7113..81cb9f0ab0ad 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -939,17 +939,7 @@ struct HttpResponse : public AsyncSocket { } /* If we have no backbuffer and we are connection close and we responded fully then close */ - HttpResponseData *httpResponseData = getHttpResponseData(); - if (httpResponseData->shouldCloseConnection()) { - if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { - if (((AsyncSocket *) this)->hasFullyDrained()) { - ((AsyncSocket *) this)->shutdown(); - /* We need to force close after sending FIN since we want to hinder - * clients from keeping to send their huge data */ - ((AsyncSocket *) this)->close(); - } - } - } + closeIfDoneAndMarked(getHttpResponseData()); } else { /* We are already corked, or can't cork so let's just call the handler */ handler(); diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index e1bd8b8bac72..96643aa031d1 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -868,6 +868,54 @@ describe("a request pipelined behind a Connection: close request", () => { }); }); +// A response without a body completes while its socket is still corked, and its +// close then runs from HttpResponse::cork(), not from the gates the tests above +// go through. The client wrote more while a request was held, so the server has +// to read that before it closes. On a unix socket the reset shows as an error +// behind the response. A response this small is out before a TCP reset can cut it. +it.if(isPosix)( + "a HEAD response that closes the connection ends it cleanly when the client wrote more while it was pending", + async () => { + using dir = tempDir("serve-pipelining", {}); + const unix = transports.find(transport => transport.name === "unix")!; + const handler = holdingHandler(); + using server = Bun.serve({ + ...unix.listen(String(dir)), + async fetch(req) { + const response = await handler.fetch(req); + if (new URL(req.url).pathname === "/hold") response.headers.set("Connection", "close"); + return response; + }, + }); + const client = await connectNodeSocket(unix, server, String(dir)); + client.reader.headResponses = 1; + + await client.write("HEAD /hold HTTP/1.1\r\nHost: x\r\n\r\n"); + await handler.entered("/hold"); + // The first /never is held, the second one stays unread. + for (let i = 0; i < 2; i++) { + await client.write(request("/never")); + await probe(unix, server, String(dir)); + } + + handler.release("/hold"); + await client.closed; + expect({ + hits: handler.hits, + seen: client.seen, + responses: client.reader.responses.map(({ statusLine, headers, body }) => ({ + statusLine, + connection: headers["connection"], + body, + })), + }).toEqual({ + hits: ["/hold", "/probe", "/probe"], + seen: { ended: true }, + responses: [{ statusLine: "HTTP/1.1 200 OK", connection: "close", body: "" }], + }); + }, +); + // A graceful stop() closes idle connections and marks busy ones to close once // their work is done. A request that was received and held behind the response // in flight is part of that work: it is answered, and the connection closes after From 8e68a68f12e71c63596f15163f821f3d91024253 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 07:08:04 +0000 Subject: [PATCH 19/22] Bun.serve: linger on a close that the peer is still writing behind parked requests 55af3658c5 read what was unread once, right before the close. That covers what the server's receive buffer holds. A peer that queued more than that (a 1 MiB POST behind the parked requests) still has the rest in its own kernel, behind the closed receive window. The one read opens the window, the rest arrives after close(), and the reset drops the unsent end of the response in front: an 8 MiB answer arrived cut in 9 of 42 runs here (debug build, node client in its own process), and the client's own write failed with EPIPE in the others. The close gates now linger in that state instead. shutdownAndClose() sends the FIN and leaves the socket open with reads resumed. onData already ignores a socket that is shut down, so what the peer still sends is read and dropped. The peer's FIN closes the socket (the loop does that for a socket that has sent its own FIN). A timeout of 4 to 8 seconds and a limit of 8 MiB bound a peer that does not stop. The linger starts only for Bun.serve, only when requests are parked or a replay is on the stack, and only when us_socket_queued_input() reports unread data. Every other close is the same shutdown() and close() as before. us_socket_discard_unread() is gone again: the linger reads through the normal path, so usockets needs no new function. HTTP_LINGERING_CLOSE marks the socket. The gates skip a socket that has it, and setTimeout()/resetTimeout() leave its timeout alone: the runtime resets the timeout after the response ends, and with idleTimeout: 0 that would remove the bound. --- packages/bun-usockets/src/libusockets.h | 4 - packages/bun-usockets/src/socket.c | 18 ---- packages/bun-uws/src/HttpContext.h | 32 +++---- packages/bun-uws/src/HttpResponse.h | 62 ++++++++++---- packages/bun-uws/src/HttpResponseData.h | 3 + test/js/bun/http/bun-serve-pipelining.test.ts | 83 ++++++++++++++++++- 6 files changed, 148 insertions(+), 54 deletions(-) diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index c00fb06c7ead..5db941185a97 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -707,10 +707,6 @@ int us_socket_ipc_write_fd(us_socket_r s, const char *data, int length, int fd) /* Dispatches on_writable once the socket is writable, as if a us_socket_write() * had come up short. Safe inside on_writable. Leaves a paused read side alone. */ void us_socket_request_writable(us_socket_r s) nonnull_fn_decl; -/* Reads and drops up to max_bytes of what the peer already sent, without a - * dispatch. For a close while reads are paused: close() over unread bytes resets - * the connection, and the kernel then drops what it has not sent yet. */ -void us_socket_discard_unread(us_socket_r s, unsigned int max_bytes) nonnull_fn_decl; void *us_listen_socket_ext(struct us_listen_socket_t *ls) nonnull_fn_decl; LIBUS_SOCKET_DESCRIPTOR us_listen_socket_get_fd(struct us_listen_socket_t *ls) nonnull_fn_decl; int us_listen_socket_port(struct us_listen_socket_t *ls) nonnull_fn_decl; diff --git a/packages/bun-usockets/src/socket.c b/packages/bun-usockets/src/socket.c index 1c558241d28e..f32adc0571ac 100644 --- a/packages/bun-usockets/src/socket.c +++ b/packages/bun-usockets/src/socket.c @@ -423,24 +423,6 @@ void us_socket_request_writable(struct us_socket_t *s) { us_internal_rearm_writable(s); } -/* See libusockets.h. Reads the raw socket: for TLS the dropped bytes are - * ciphertext, which is fine for a socket that is closed next. */ -void us_socket_discard_unread(struct us_socket_t *s, unsigned int max_bytes) { - if (us_socket_is_closed(s)) return; -#ifdef _WIN32 - const int recv_flags = MSG_PUSH_IMMEDIATE; -#else - const int recv_flags = MSG_DONTWAIT; -#endif - char buf[16 * 1024]; - while (max_bytes) { - int length = max_bytes < sizeof(buf) ? (int) max_bytes : (int) sizeof(buf); - ssize_t received = bsd_recv(us_poll_fd(&s->p), buf, length, recv_flags); - if (received <= 0) break; - max_bytes -= (unsigned int) received; - } -} - /* See libusockets.h: whether a zero-progress write on a writable event proves * the peer is gone. Only the libuv backend has to ask the kernel. */ int us_socket_stalled_write_means_peer_gone(struct us_socket_t *s) { diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 4792f7352359..bb4312a81932 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -336,6 +336,17 @@ struct HttpContext { /* Balance the us_socket_ref above — every other return path * reaches the unref via returnedData. */ us_socket_unref(s); + /* Bun.serve: a lingering close drops what the peer still sends, up to + * a limit (HttpResponse::shutdownAndClose). */ + if constexpr (!IsNodeHttp) { + auto *lingering = (HttpResponseData *) us_socket_ext(s); + if (lingering->state & HttpResponseData::HTTP_LINGERING_CLOSE) { + lingering->received_bytes_per_timeout += (unsigned int) length; + if (lingering->received_bytes_per_timeout > HttpResponse::LINGERING_CLOSE_MAX_BYTES) { + return ((AsyncSocket *) s)->close(); + } + } + } return s; } @@ -734,12 +745,13 @@ struct HttpContext { ((AsyncSocket *) s)->uncork(); /* For errors, we only deliver them "at most once". We don't care if they get halfways delivered or not. */ us_socket_write(s, httpErrorResponses[httpErrorStatusCode].data(), (int) httpErrorResponses[httpErrorStatusCode].length()); + /* Close any socket on HTTP errors */ if constexpr (!IsNodeHttp) { - ((HttpResponse *) s)->discardBytesUnreadBehindParkedRequests(httpResponseData); + ((HttpResponse *) s)->shutdownAndClose(httpResponseData); + } else { + us_socket_shutdown(s); + us_socket_close(s, 0, nullptr); } - us_socket_shutdown(s); - /* Close any socket on HTTP errors */ - us_socket_close(s, 0, nullptr); } auto returnedData = result.returnedData; @@ -785,11 +797,7 @@ struct HttpContext { if (httpResponseData->shouldCloseConnection()) { if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { if (((AsyncSocket *) s)->hasFullyDrained()) { - ((HttpResponse *) s)->discardBytesUnreadBehindParkedRequests(httpResponseData); - ((AsyncSocket *) s)->shutdown(); - /* We need to force close after sending FIN since we want to hinder - * clients from keeping to send their huge data */ - ((AsyncSocket *) s)->close(); + ((HttpResponse *) s)->shutdownAndClose(httpResponseData); } } } @@ -948,11 +956,7 @@ struct HttpContext { } } if (responseDone && asyncSocket->hasFullyDrained()) { - reinterpret_cast *>(s)->discardBytesUnreadBehindParkedRequests(httpResponseData); - asyncSocket->shutdown(); - /* We need to force close after sending FIN since we want to hinder - * clients from keeping to send their huge data */ - asyncSocket->close(); + reinterpret_cast *>(s)->shutdownAndClose(httpResponseData); } } diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 81cb9f0ab0ad..011d0686cdf7 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -56,12 +56,18 @@ struct HttpResponse : public AsyncSocket { void setTimeout(uint8_t seconds) { auto* data = getHttpResponseData(); data->idleTimeout = seconds; + /* A lingering close owns the timeout (shutdownAndClose). */ + if (data->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { + return; + } Super::timeout(data->idleTimeout); } void resetTimeout() { auto* data = getHttpResponseData(); - + if (data->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { + return; + } Super::timeout(data->idleTimeout); } /* Write an unsigned 32-bit integer in hex */ @@ -92,33 +98,55 @@ struct HttpResponse : public AsyncSocket { getHttpResponseData()->state |= HttpResponseData::HTTP_WROTE_DATE_HEADER; } - /* Bun.serve, before a close. While requests are parked reads are paused, so - * what the peer sent since then is unread. A close over unread bytes resets - * the connection, and the kernel then drops what it has not sent yet: the - * peer loses the end of a complete response. A connection that closes - * dispatches none of those bytes, so they are read and dropped. A socket - * receive buffer bounds them. */ - void discardBytesUnreadBehindParkedRequests(HttpResponseData *httpResponseData) { - if (httpResponseData->parkedRequestBytes.isEmpty() && !httpResponseData->replayedRequestBytes) [[likely]] { + /* How long a close lingers, and how much it drops (onData counts). The + * timeout sweep runs every 4 seconds, so this is between 4 and 8 seconds. */ + static constexpr unsigned int LINGERING_CLOSE_SECONDS = 8; + static constexpr unsigned int LINGERING_CLOSE_MAX_BYTES = 8 * 1024 * 1024; + + /* Sends the FIN and closes: the end of every close gate. Bun.serve: reads are + * paused while requests are parked, so what the peer wrote since then is + * unread, and more can wait behind its closed receive window. A close over + * those bytes, or ahead of them, resets the connection, and the kernel then + * drops what it has not sent yet: the peer loses the end of a complete + * response. A connection that closes dispatches none of those bytes. So when + * some are queued the close lingers: FIN now, reads stay open and onData + * drops them, and the peer's FIN, the timeout or the byte limit closes the + * socket. */ + void shutdownAndClose(HttpResponseData *httpResponseData) { + if (httpResponseData->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { return; } - if (!HttpContext::fromSocket((us_socket_t *) this)->isNodeHttp()) { - us_socket_discard_unread((us_socket_t *) this, 8 * 1024 * 1024); + bool readsWerePaused = !httpResponseData->parkedRequestBytes.isEmpty() || httpResponseData->replayedRequestBytes; + if (readsWerePaused && !HttpContext::fromSocket((us_socket_t *) this)->isNodeHttp() + && us_socket_queued_input((us_socket_t *) this) == LIBUS_QUEUED_INPUT_DATA) [[unlikely]] { + httpResponseData->state |= HttpResponseData::HTTP_LINGERING_CLOSE; + httpResponseData->received_bytes_per_timeout = 0; + /* Can close the socket, which destructs httpResponseData. */ + Super::resume(); + if (!us_socket_is_closed((us_socket_t *) this)) { + Super::shutdown(); + Super::timeout(LINGERING_CLOSE_SECONDS); + } + return; } + Super::shutdown(); + /* We need to force close after sending FIN since we want to hinder + * clients from keeping to send their huge data */ + Super::close(); } /* Shutdown+close when the connection is marked to close (Connection: * close, peer FIN, close-when-idle), the response is complete and every - * outgoing byte has been flushed. Returns true when the socket was closed. */ + * outgoing byte has been flushed. Returns true when the socket was closed or + * left to a lingering close: the caller is done with it either way. */ bool closeIfDoneAndMarked(HttpResponseData *httpResponseData) { + if (httpResponseData->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { + return true; + } if (httpResponseData->shouldCloseConnection()) { if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { if (((AsyncSocket *) this)->hasFullyDrained()) { - discardBytesUnreadBehindParkedRequests(httpResponseData); - ((AsyncSocket *) this)->shutdown(); - /* We need to force close after sending FIN since we want to hinder - * clients from keeping to send their huge data */ - ((AsyncSocket *) this)->close(); + shutdownAndClose(httpResponseData); return true; } } diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 043659d4762a..e911c3a479bc 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -164,6 +164,9 @@ struct HttpResponseData : AsyncSocketData, HttpParser { * that runs after it (microtasks, the request body callback) can block, * reset the connection or end the process. */ HTTP_SEND_WHEN_COMPLETE = 1 << 18, + /* Bun.serve: a close gate sent the FIN and left the socket open to drop + * what the peer still sends (HttpResponse::shutdownAndClose). */ + HTTP_LINGERING_CLOSE = 1 << 19, /* Bits that describe the connection rather than the response in flight. * There is one HttpResponseData per socket, reused by every request on a diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 96643aa031d1..d9fda434420c 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { isPosix, tempDir, tls } from "harness"; +import { bunEnv, bunExe, isPosix, tempDir, tls } from "harness"; import { once } from "node:events"; import { connect as netConnect } from "node:net"; import { join } from "node:path"; @@ -916,6 +916,87 @@ it.if(isPosix)( }, ); +// More behind the parked requests than the server's receive buffer takes: the rest +// waits in the client's kernel and arrives when the server reads. A close right +// after one read is ahead of those bytes, and they reset the connection while +// the end of the big response is still unsent. So the close lingers: the server +// sends its FIN, drops what still comes, and closes on the client's FIN. The +// server runs in its own process, because a client on the server's event loop +// cannot write while the server closes. +it("a close lingers while the client still sends what it queued behind the parked requests", async () => { + const serverSource = ` + const big = Buffer.alloc(${BIG_BODY_LENGTH}, "x"); + const gates = new Map(); + const gate = name => gates.get(name) ?? gates.set(name, Promise.withResolvers()).get(name); + const hits = []; + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + async fetch(req) { + const { pathname, searchParams } = new URL(req.url); + const id = searchParams.get("id"); + if (pathname === "/hits") return Response.json(hits); + if (pathname === "/entered") return gate("entered" + id).promise.then(() => new Response("entered")); + if (pathname === "/release") return gate("release" + id).resolve(), new Response("released"); + hits.push(pathname); + if (pathname !== "/slow") return new Response("body of " + pathname); + gate("entered" + id).resolve(); + await gate("release" + id).promise; + return new Response(big); + }, + }); + console.log(server.port); + `; + await using server = Bun.spawn({ + cmd: [bunExe(), "-e", serverSource], + env: bunEnv, + stdout: "pipe", + stderr: "inherit", + }); + const stdout = server.stdout.getReader(); + const port = parseInt(new TextDecoder().decode((await stdout.read()).value)); + const origin = `http://127.0.0.1:${port}`; + const upload = Buffer.alloc(4 * 1024 * 1024, "a"); + + const results = []; + for (let id = 0; id < 3; id++) { + const socket = netConnect({ port, host: "127.0.0.1" }); + const reader = new ResponseReader(); + const seen: { ended: boolean; error?: string } = { ended: false }; + socket.on("data", chunk => reader.push(chunk)); + socket.on("end", () => (seen.ended = true)); + socket.on("error", (error: NodeJS.ErrnoException) => (seen.error = error.code)); + const closed = new Promise(resolve => socket.on("close", () => resolve())); + await once(socket, "connect"); + + socket.write(request(`/slow?id=${id}`) + request("/held", "Connection: close\r\n")); + expect(await (await fetch(`${origin}/entered?id=${id}`)).text()).toBe("entered"); + // Not awaited: the server does not read while /held is parked, so this write + // completes only once the close lingers. + socket.write(`POST /upload HTTP/1.1\r\nHost: x\r\nContent-Length: ${upload.length}\r\n\r\n`); + socket.write(upload); + expect(await (await fetch(`${origin}/release?id=${id}`)).text()).toBe("released"); + await closed; + results.push({ + seen, + responses: reader.responses.map(({ statusLine, body }) => ({ statusLine, bodyLength: body.length })), + }); + } + + expect(results).toEqual( + Array.from({ length: 3 }, () => ({ + seen: { ended: true }, + responses: [ + { statusLine: "HTTP/1.1 200 OK", bodyLength: BIG_BODY_LENGTH }, + { statusLine: "HTTP/1.1 200 OK", bodyLength: 13 }, + ], + })), + ); + expect(await (await fetch(`${origin}/hits`)).json()).toEqual( + Array.from({ length: 3 }, () => ["/slow", "/held"]).flat(), + ); +}); + // A graceful stop() closes idle connections and marks busy ones to close once // their work is done. A request that was received and held behind the response // in flight is part of that work: it is answered, and the connection closes after From 29164bb569a7ed6790942e1629d5997c2351c7ca Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:14:18 +0000 Subject: [PATCH 20/22] uws: take the lingering close off the request path 8e68a68f12 tested HTTP_LINGERING_CLOSE in setTimeout(), resetTimeout() and at the top of closeIfDoneAndMarked(): about four tests per keep-alive request for a state that only a close can enter. shutdownAndClose() now sets idleTimeout to the linger's own bound when it starts. The resetTimeout() calls of the response teardown then arm that bound again, also on a server with idleTimeout: 0, so setTimeout() and resetTimeout() are the same as on main again. The test in closeIfDoneAndMarked() was redundant: shutdownAndClose() has it, behind the branch that only a closing connection takes. A silent peer that never closes is still closed by the timeout with idleTimeout: 0 (8.0 s here). --- packages/bun-uws/src/HttpResponse.h | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 011d0686cdf7..47a7f1c6e4c1 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -56,18 +56,12 @@ struct HttpResponse : public AsyncSocket { void setTimeout(uint8_t seconds) { auto* data = getHttpResponseData(); data->idleTimeout = seconds; - /* A lingering close owns the timeout (shutdownAndClose). */ - if (data->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { - return; - } Super::timeout(data->idleTimeout); } void resetTimeout() { auto* data = getHttpResponseData(); - if (data->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { - return; - } + Super::timeout(data->idleTimeout); } /* Write an unsigned 32-bit integer in hex */ @@ -100,7 +94,7 @@ struct HttpResponse : public AsyncSocket { /* How long a close lingers, and how much it drops (onData counts). The * timeout sweep runs every 4 seconds, so this is between 4 and 8 seconds. */ - static constexpr unsigned int LINGERING_CLOSE_SECONDS = 8; + static constexpr uint8_t LINGERING_CLOSE_SECONDS = 8; static constexpr unsigned int LINGERING_CLOSE_MAX_BYTES = 8 * 1024 * 1024; /* Sends the FIN and closes: the end of every close gate. Bun.serve: reads are @@ -121,6 +115,9 @@ struct HttpResponse : public AsyncSocket { && us_socket_queued_input((us_socket_t *) this) == LIBUS_QUEUED_INPUT_DATA) [[unlikely]] { httpResponseData->state |= HttpResponseData::HTTP_LINGERING_CLOSE; httpResponseData->received_bytes_per_timeout = 0; + /* The teardown of the response still calls resetTimeout(), also with + * idleTimeout: 0. It has to arm this bound again, not remove it. */ + httpResponseData->idleTimeout = LINGERING_CLOSE_SECONDS; /* Can close the socket, which destructs httpResponseData. */ Super::resume(); if (!us_socket_is_closed((us_socket_t *) this)) { @@ -140,9 +137,6 @@ struct HttpResponse : public AsyncSocket { * outgoing byte has been flushed. Returns true when the socket was closed or * left to a lingering close: the caller is done with it either way. */ bool closeIfDoneAndMarked(HttpResponseData *httpResponseData) { - if (httpResponseData->state & HttpResponseData::HTTP_LINGERING_CLOSE) [[unlikely]] { - return true; - } if (httpResponseData->shouldCloseConnection()) { if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { if (((AsyncSocket *) this)->hasFullyDrained()) { From e589bad7f865a80e2384c721e02a9873932117c5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:47:53 +0000 Subject: [PATCH 21/22] Bun.serve: a lingering close needs a complete response and is never idle Two review findings on the lingering close of 8e68a68f12. shutdownAndClose() started a lingering close on every path, also from the parse-error path of onData, which is the one caller that can reach it with a response still pending. That happens when a request that was held fails to parse its body after its handler was dispatched, and the client wrote more while it was held. The socket then stayed open for the 4 to 8 seconds of the linger, so onClose did not run and the handler did not see the abort. shutdownAndClose() now lingers only when no response is pending. Every other caller checks that first, so only the parse-error path changes: it shuts down and closes at once again, as on main. A lingering close also counted as idle when the response that closed the connection completed inside a replay: markDone() then sees no parked bytes, because the replay has moved them out, and sets isIdle. closeIdle(), from closeIdleConnections() or a graceful stop(), closed that socket in the middle of the linger. shutdownAndClose() now clears isIdle when the linger starts. No request is dispatched on a socket that has sent its FIN, so nothing sets it again. The two changes depend on each other. Since 29164bb569 setTimeout() no longer looks at the linger flag, so a handler that still ran during a linger could replace its timeout through server.timeout(). With no response pending there is no such handler. Two tests in bun-serve-pipelining.test.ts, with a client that keeps its side open after the FIN of the server. Both fail without the change. --- packages/bun-uws/src/HttpResponse.h | 6 +- test/js/bun/http/bun-serve-pipelining.test.ts | 104 ++++++++++++++++-- 2 files changed, 102 insertions(+), 8 deletions(-) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 47a7f1c6e4c1..6e05772e58ce 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -111,13 +111,17 @@ struct HttpResponse : public AsyncSocket { return; } bool readsWerePaused = !httpResponseData->parkedRequestBytes.isEmpty() || httpResponseData->replayedRequestBytes; - if (readsWerePaused && !HttpContext::fromSocket((us_socket_t *) this)->isNodeHttp() + /* A parse error closes with a response still pending: closing now aborts its handler. */ + bool responsePending = httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING; + if (readsWerePaused && !responsePending && !HttpContext::fromSocket((us_socket_t *) this)->isNodeHttp() && us_socket_queued_input((us_socket_t *) this) == LIBUS_QUEUED_INPUT_DATA) [[unlikely]] { httpResponseData->state |= HttpResponseData::HTTP_LINGERING_CLOSE; httpResponseData->received_bytes_per_timeout = 0; /* The teardown of the response still calls resetTimeout(), also with * idleTimeout: 0. It has to arm this bound again, not remove it. */ httpResponseData->idleTimeout = LINGERING_CLOSE_SECONDS; + /* markDone() in a replay sees no parked bytes and sets isIdle: keep closeIdle() off this socket. */ + httpResponseData->isIdle = false; /* Can close the socket, which destructs httpResponseData. */ Super::resume(); if (!us_socket_is_closed((us_socket_t *) this)) { diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index d9fda434420c..16ae205c7ae8 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -202,24 +202,31 @@ const summarize = ({ statusLine, body }: RawResponse) => ({ statusLine, body }); const plainResponse = (req: Request) => new Response(`body of ${new URL(req.url).pathname}`); // A node:net or node:tls client, for the tests that look at how the stream ends: -// with the server's FIN (`ended`) or with an error such as ECONNRESET. -async function connectNodeSocket(transport: Transport, server: Bun.Server, dir: string) { +// with the server's FIN (`ended`) or with an error such as ECONNRESET. With +// `allowHalfOpen` the client does not answer the server's FIN with its own, so +// a lingering close stays open until the test ends the socket. +async function connectNodeSocket( + transport: Transport, + server: Bun.Server, + dir: string, + options: { allowHalfOpen?: boolean } = {}, +) { const socket = transport.name === "tls" - ? tlsConnect({ port: server.port!, host: "127.0.0.1", ca: tls.cert, rejectUnauthorized: false }) + ? tlsConnect({ port: server.port!, host: "127.0.0.1", ca: tls.cert, rejectUnauthorized: false, ...options }) : transport.name === "unix" - ? netConnect({ path: join(dir, "pipeline.sock") }) - : netConnect({ port: server.port!, host: "127.0.0.1" }); + ? netConnect({ path: join(dir, "pipeline.sock"), ...options }) + : netConnect({ port: server.port!, host: "127.0.0.1", ...options }); const reader = new ResponseReader(); const seen: { ended: boolean; error?: string } = { ended: false }; socket.on("data", chunk => reader.push(chunk)); - socket.on("end", () => (seen.ended = true)); + const ended = new Promise(resolve => socket.on("end", () => ((seen.ended = true), resolve()))); socket.on("error", (error: NodeJS.ErrnoException) => (seen.error = error.code)); const closed = new Promise(resolve => socket.on("close", () => resolve())); await once(socket, transport.name === "tls" ? "secureConnect" : "connect"); // Resolves once the kernel has the bytes. const write = (data: string) => new Promise(resolve => socket.write(data, () => resolve())); - return { socket, reader, seen, closed, write }; + return { socket, reader, seen, ended, closed, write }; } // A round trip on a separate connection. Anything the pipelining client wrote @@ -997,6 +1004,89 @@ it("a close lingers while the client still sends what it queued behind the parke ); }); +// A request body that fails to parse closes the connection behind a 400. The +// request came out of the park here, so its handler is still running and the +// client wrote more meanwhile. That close must not linger: a lingering close +// keeps the socket open, and the handler does not see the abort until it ends. +// The client keeps its side open, so only the server can end the connection. +it("a held request whose body fails to parse is aborted at once when the client wrote more meanwhile", async () => { + const handler = holdingHandler(); + const aborted: string[] = []; + using server = Bun.serve({ + ...tcp, + fetch(req) { + const path = new URL(req.url).pathname; + if (path === "/aborted") return Response.json(aborted); + req.signal.addEventListener("abort", () => aborted.push(path)); + return handler.fetch(req); + }, + }); + const client = await connectNodeSocket(tcpOnly, server, "", { allowHalfOpen: true }); + + await client.write(request("/hold")); + await handler.entered("/hold"); + // Held behind /hold. "Z" is not a chunk size. + await client.write("POST /hold-body HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\n\r\nZ\r\n"); + await probe(tcpOnly, server, ""); + // Stays unread: the server does not read while a request is held. + await client.write(request("/never")); + await probe(tcpOnly, server, ""); + + handler.release("/hold"); + // The server's FIN, or the close when a reset gets ahead of it. + await Promise.race([client.ended, client.closed]); + expect({ + aborted: await (await fetch(`${server.url}aborted`)).json(), + hits: handler.hits, + }).toEqual({ + aborted: ["/hold-body"], + hits: ["/hold", "/probe", "/probe", "/hold-body"], + }); + handler.release("/hold-body"); + client.socket.destroy(); +}); + +// closeIdleConnections() and a graceful stop() close the connections that are +// idle. A lingering close is not idle: it stays open so that the close does not +// land on what the client still sends. The request that closes the connection +// comes out of the park here, the case where the connection counted as idle. A +// sweep that closes the socket makes the client's next write fail. +it.if(isPosix)("closeIdleConnections() leaves a lingering close to end by itself", async () => { + using dir = tempDir("serve-pipelining", {}); + const unix = transports.find(transport => transport.name === "unix")!; + const socketPath = join(String(dir), "pipeline.sock"); + const handler = holdingHandler(); + using server = Bun.serve({ + ...unix.listen(String(dir)), + fetch(req, server) { + if (new URL(req.url).pathname !== "/sweep") return handler.fetch(req); + server.closeIdleConnections(); + return new Response("swept"); + }, + }); + const client = await connectNodeSocket(unix, server, String(dir), { allowHalfOpen: true }); + + await client.write(request("/hold")); + await handler.entered("/hold"); + // /closing is held. /never stays unread, so the close behind /closing lingers. + await client.write(request("/closing", "Connection: close\r\n")); + await probe(unix, server, String(dir)); + await client.write(request("/never")); + + handler.release("/hold"); + await client.ended; + expect(await (await fetch("http://localhost/sweep", { unix: socketPath })).text()).toBe("swept"); + // The server still reads, and it closes on the client's FIN. + await client.write(request("/late")); + client.socket.end(); + await client.closed; + expect({ hits: handler.hits, seen: client.seen, responses: client.reader.responses.map(summarize) }).toEqual({ + hits: ["/hold", "/probe", "/closing"], + seen: { ended: true }, + responses: [ok("body of /hold"), ok("body of /closing")], + }); +}); + // A graceful stop() closes idle connections and marks busy ones to close once // their work is done. A request that was received and held behind the response // in flight is part of that work: it is answered, and the connection closes after From f987e1b41bd8646e5f05b95952f4120e9eb56505 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 22 Sep 2026 11:27:59 +0000 Subject: [PATCH 22/22] uws: keep the lingering-close bit across a dispatch, and fail a reset test fast Two optional review findings. HTTP_LINGERING_CLOSE describes the connection, not the response in flight, so it belongs in HTTP_CONNECTION_SCOPED. Without it resetResponseState() clears the bit on a dispatch, which would drop the byte cap of the linger in onData and the no-op guard in shutdownAndClose(). I could not reach that dispatch. A lingering socket is shut down, and onData drops its reads. Over TLS the shutdown waits for spilled ciphertext, but the replay runs only after hasFullyDrained(), which counts the spill. With the dispatch instrumented, the pipelining file starts 12 lingering closes and dispatches nothing on any of them. So this is a correctness rule of the header, not a fix for a reachable defect, and it has no test. The `ended` promise of connectNodeSocket settled only on 'end'. A connection that is reset never ends, so a test that awaits it would reach its timeout instead of failing on what `seen` recorded. It now settles on 'end', 'error' and 'close'. With a reset simulated, the closeIdleConnections() test fails in about 0.5 s and reports ECONNRESET, where it used to hang. --- packages/bun-uws/src/HttpResponseData.h | 2 +- test/js/bun/http/bun-serve-pipelining.test.ts | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index f89864f79c13..3616c712585e 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -177,7 +177,7 @@ struct HttpResponseData : AsyncSocketData, HttpParser { * word (resetResponseState) - these have to survive that. */ HTTP_CONNECTION_SCOPED = HTTP_NODE_PARSING_STOPPED | HTTP_NODE_READS_PAUSED | HTTP_NODE_TUNNEL_AFTER_BODY | HTTP_NODE_RECEIVED_FIN | HTTP_CLOSE_WHEN_IDLE - | HTTP_NODE_PEER_ENDED, + | HTTP_NODE_PEER_ENDED | HTTP_LINGERING_CLOSE, }; /* Begin a new response on this connection. Clearing the word in one go is diff --git a/test/js/bun/http/bun-serve-pipelining.test.ts b/test/js/bun/http/bun-serve-pipelining.test.ts index 16ae205c7ae8..1d363d2b84ee 100644 --- a/test/js/bun/http/bun-serve-pipelining.test.ts +++ b/test/js/bun/http/bun-serve-pipelining.test.ts @@ -220,9 +220,14 @@ async function connectNodeSocket( const reader = new ResponseReader(); const seen: { ended: boolean; error?: string } = { ended: false }; socket.on("data", chunk => reader.push(chunk)); - const ended = new Promise(resolve => socket.on("end", () => ((seen.ended = true), resolve()))); - socket.on("error", (error: NodeJS.ErrnoException) => (seen.error = error.code)); - const closed = new Promise(resolve => socket.on("close", () => resolve())); + // `ended` settles on every terminal event, not on 'end' alone: a connection + // that is reset never ends, and a test awaiting it would reach its timeout + // instead of failing on what `seen` recorded. + const terminal = Promise.withResolvers(); + const ended = terminal.promise; + socket.on("end", () => ((seen.ended = true), terminal.resolve())); + socket.on("error", (error: NodeJS.ErrnoException) => ((seen.error = error.code), terminal.resolve())); + const closed = new Promise(resolve => socket.on("close", () => (terminal.resolve(), resolve()))); await once(socket, transport.name === "tls" ? "secureConnect" : "connect"); // Resolves once the kernel has the bytes. const write = (data: string) => new Promise(resolve => socket.write(data, () => resolve()));