Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/bun-uws/src/HttpResponse.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ struct HttpResponse : public AsyncSocket<SSL> {
}
}

/* Completes a response whose bytes the caller already wrote (bodyLength of them are body), with the uncork and close gate internalEnd() ends on. */
void finishResponseInCorkBuffer(size_t bodyLength) {
HttpResponseData<SSL> *httpResponseData = getHttpResponseData();
httpResponseData->state |= HttpResponseData<SSL>::HTTP_END_CALLED;
Expand Down
4 changes: 0 additions & 4 deletions src/runtime/bake/dev_server/deferred_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,6 @@ impl DeferredRequest {
saved.deinit();
}
Handler::BundledHtmlPage(r) => {
// Reached from JS event-loop tasks (on_plugins_rejected, the
// bundle-completion OOM cleanup), so end_without_body
// alone cannot close the socket; write Content-Length so the
// client has framing.
r.response.write_status(b"500 Internal Server Error");
r.response.write_header_int(b"Content-Length", 0);
r.response.end_without_body(true);
Expand Down
3 changes: 0 additions & 3 deletions src/runtime/server/HTMLBundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,9 +647,6 @@ impl Route {
// To protect privacy, do not show errors to end users in production.
// TODO: Show a generic error page.
}
// This runs from a JS event-loop task, not a uWS handler,
// so `end_without_body(true)` alone cannot close the
// socket; write Content-Length so the client has framing.
resp.write_status(b"500 Build Failed");
resp.write_header_int(b"Content-Length", 0);
resp.end_without_body(true);
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/server/NodeHTTPResponse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1791,7 +1791,7 @@ impl NodeHTTPResponse {
if let Some(raw_response) = self.writer() {
raw_response.clear_on_writable();
raw_response.clear_timeout();
raw_response.end_without_body(true);
raw_response.abandon();
}
self.on_request_complete();
}
Expand Down
6 changes: 0 additions & 6 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1335,12 +1335,6 @@ where
self.reclaim_promise_cell();
// SAFETY: FFI handle
resp.end_without_body(close_connection);
// This end can run uncorked (e.g. render_production_error from a
// rejection microtask), where no cork or parser gate runs the
// close check for Connection: close or a graceful-stop mark. The
// shim no-ops when the socket is corked (the cork wrapper's own
// gate runs later) or already closed.
resp.close_if_done_and_marked();
// end_request_streaming_and_drain() must run after the last
// `resp` access: its drain_microtasks() can re-enter lsquic (H3)
// and free the stream out from under the local `resp` copy.
Expand Down
17 changes: 17 additions & 0 deletions src/uws_sys/Response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,16 @@ impl<const SSL: bool> Response<SSL> {
}
}

/// Like `end`, this sends the response and can close the socket before it returns.
pub fn end_without_body(&mut self, close_connection: bool) {
c::uws_res_end_without_body(Self::ssl_flag(), self.as_raw(), close_connection)
}

/// `end_without_body(true)` for node:http `res.destroy()`: no uncork and no close gate.
pub(crate) fn abandon(&mut self) {
c::uws_res_abandon(Self::ssl_flag(), self.as_raw())
}

pub(crate) fn end_send_file(&mut self, write_offset: u64, close_connection: bool) {
c::uws_res_end_sendfile(
Self::ssl_flag(),
Expand Down Expand Up @@ -968,6 +974,16 @@ impl AnyResponse {
any_dispatch!(self, |r| r.end_without_body(close_connection))
}

/// HTTP/1 only: an HTTP/2 or HTTP/3 stream ends as `end_without_body(true)` does.
pub fn abandon(self) {
match self {
AnyResponse::SSL(ptr) => TLSResponse::as_handle(ptr).abandon(),
AnyResponse::TCP(ptr) => TCPResponse::as_handle(ptr).abandon(),
AnyResponse::H3(ptr) => H3Response::as_handle(ptr).end_without_body(true),
AnyResponse::H2(ptr) => H2Response::as_handle(ptr).end_without_body(true),
}
}

/// HTTP/1 only: the handlers of an HTTP/2 or HTTP/3 stream belong to one response.
pub fn clear_handlers_of<U>(self, user_data: *mut U) {
match self {
Expand Down Expand Up @@ -1369,6 +1385,7 @@ pub mod c {
res: &mut uws_res,
close_connection: bool,
);
pub(crate) safe fn uws_res_abandon(ssl: i32, res: &mut uws_res);
pub(crate) safe fn uws_res_end_sendfile(
ssl: i32,
res: &mut uws_res,
Expand Down
99 changes: 42 additions & 57 deletions src/uws_sys/libuwsockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,29 @@ __attribute__((always_inline)) static inline auto withWebSocket(int ssl, uws_web
return fn((TCPWebSocket *)ws);
}

/* Finishes the head of a response that sends no body bytes. */
template <bool SSL>
static void endHeadWithoutBody(uWS::HttpResponse<SSL> *uwsRes, bool close_connection)
{
auto *data = uwsRes->getHttpResponseData();
/* After write()/flushHeaders() or an earlier end terminated the header section, header bytes would land inside the body (node:http res.destroy() mid-response). */
bool headers_open = !(data->state & (uWS::HttpResponseData<SSL>::HTTP_WRITE_CALLED | uWS::HttpResponseData<SSL>::HTTP_END_CALLED));
if (close_connection)
{
/* The header is advisory, as in internalEnd(): HTTP_CONNECTION_CLOSE is what makes the close gates tear the connection down. */
if (headers_open && !(data->state & uWS::HttpResponseData<SSL>::HTTP_CONNECTION_CLOSE))
{
uwsRes->writeHeader("Connection", "close");
}
data->state |= uWS::HttpResponseData<SSL>::HTTP_CONNECTION_CLOSE;
}
if (headers_open)
{
// Some HTTP clients require the complete "<header>\r\n\r\n" to be sent. If not, they may throw a ConnectionError.
uwsRes->uWS::template AsyncSocket<SSL>::write("\r\n", 2);
}
}

/* upgradeContext and id are only passed to behavior.upgrade */
template <bool SSL>
static auto makeBehavior(const uws_socket_behavior_t &behavior, void *upgradeContext = nullptr, size_t id = 0)
Expand Down Expand Up @@ -985,8 +1008,8 @@ extern "C"
uwsRes->resetTimeout();
}
}
/* Completion gate for response-end paths that bypass internalEnd (the
* sendfile path): closes the socket when the connection is marked to close
/* Completion gate for uws_res_end_sendfile, the one end that does not gate
* itself: closes the socket 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. Corked responses are left to the
* cork() wrapper's own post-uncork gate. */
Expand Down Expand Up @@ -1036,64 +1059,26 @@ extern "C"

void uws_res_end_without_body(int ssl, uws_res_r res, bool close_connection)
{
if (ssl)
{
uWS::HttpResponse<true> *uwsRes = (uWS::HttpResponse<true> *)res;
auto *data = uwsRes->getHttpResponseData();
/* Once write()/flushHeaders() (HTTP_WRITE_CALLED) or an earlier end
* (HTTP_END_CALLED) terminated the header section, header bytes written
* here would land inside the body (node:http res.destroy() mid-response
* ends up here). Setting HTTP_CONNECTION_CLOSE is what makes the close
* gates tear the connection down; the header itself is only advisory,
* same as in internalEnd(). */
bool headers_open = !(data->state & (uWS::HttpResponseData<true>::HTTP_WRITE_CALLED | uWS::HttpResponseData<true>::HTTP_END_CALLED));
if (close_connection)
{
if (headers_open && !(data->state & uWS::HttpResponseData<true>::HTTP_CONNECTION_CLOSE))
{
uwsRes->writeHeader("Connection", "close");
}
data->state |= uWS::HttpResponseData<true>::HTTP_CONNECTION_CLOSE;
}
if (headers_open)
{
uwsRes->AsyncSocket<true>::write("\r\n", 2);
}
data->state |= uWS::HttpResponseData<true>::HTTP_END_CALLED;
data->markDone(uwsRes);
uwsRes->resetTimeout();
/* No close gate here: callers (FileResponseStream::finish,
* DevServer/HTMLBundle error paths) keep using the response after this
* returns, so closing inside this call would destruct the ext under
* them. Corked callers get the cork() wrapper's post-uncork gate;
* uncorked ones run uws_res_close_if_done_and_marked themselves once
* they are done with the response. */
}
else
{
uWS::HttpResponse<false> *uwsRes = (uWS::HttpResponse<false> *)res;
withSSL(ssl, [&](auto isSSL) {
auto *uwsRes = (uWS::HttpResponse<decltype(isSSL)::value> *)res;
endHeadWithoutBody(uwsRes, close_connection);
/* Like end(): this sends the response and can close the socket before it returns. */
uwsRes->finishResponseInCorkBuffer(0);
});
}

/* uws_res_end_without_body(res, true) for node:http res.destroy(), which drops what is still corked and closes the socket itself: no uncork, no close gate. */
void uws_res_abandon(int ssl, uws_res_r res)
{
withSSL(ssl, [&](auto isSSL) {
constexpr bool SSL = decltype(isSSL)::value;
auto *uwsRes = (uWS::HttpResponse<SSL> *)res;
endHeadWithoutBody(uwsRes, true);
auto *data = uwsRes->getHttpResponseData();
/* See the SSL arm above. */
bool headers_open = !(data->state & (uWS::HttpResponseData<false>::HTTP_WRITE_CALLED | uWS::HttpResponseData<false>::HTTP_END_CALLED));
if (close_connection)
{
if (headers_open && !(data->state & uWS::HttpResponseData<false>::HTTP_CONNECTION_CLOSE))
{
uwsRes->writeHeader("Connection", "close");
}
data->state |= uWS::HttpResponseData<false>::HTTP_CONNECTION_CLOSE;
}
if (headers_open)
{
// Some HTTP clients require the complete "<header>\r\n\r\n" to be sent.
// If not, they may throw a ConnectionError.
uwsRes->AsyncSocket<false>::write("\r\n", 2);
}
data->state |= uWS::HttpResponseData<false>::HTTP_END_CALLED;
data->state |= uWS::HttpResponseData<SSL>::HTTP_END_CALLED;
data->markDone(uwsRes);
uwsRes->resetTimeout();
/* No close gate here; see the SSL arm above. */
}
});
}

bool uws_res_write(int ssl, uws_res_r res, const char *data, size_t *length) nonnull_fn_decl;
Expand Down
52 changes: 52 additions & 0 deletions test/js/bun/http/bun-serve-html.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,58 @@ describe("requests while a dev server bundle is in flight", () => {
});
});

// The first request to an html route waits for the bundle and is answered from an event-loop task, outside the
// read that delivered it. Both answers here end without a body on a connection that has to close.
describe("the request that waited for an html route's bundle gets its connection closed", () => {
const page = (script: string) =>
`<!DOCTYPE html><html><head><title>t</title></head><body><script type="module" src="${script}"></script></body></html>`;
const serve = /*ts*/ `
import net from "node:net";
import html from "./index.html";

using server = Bun.serve({
port: 0,
hostname: "127.0.0.1",
development: process.env.DEVELOPMENT === "true",
idleTimeout: 255,
routes: { "/": html },
fetch: () => new Response("fetch"),
});
// Resolves when the server ends the connection. The client never ends it first.
const { promise: reply, resolve, reject } = Promise.withResolvers();
let received = "";
// The dev server only answers requests for its own host.
const request = process.env.REQUEST.replace("HOST", "127.0.0.1:" + server.port);
const socket = net.connect(server.port, "127.0.0.1", () => socket.write(request));
socket.on("data", chunk => (received += chunk.toString("latin1")));
socket.on("error", reject);
socket.on("end", () => resolve(received));
console.log((await reply).split("\\r\\n")[0]);
`;

test.concurrent.each([
{
answer: "the 500 of a production build that failed",
files: { "index.html": page("./missing.ts") },
env: { DEVELOPMENT: "false", REQUEST: "GET / HTTP/1.1\r\nHost: HOST\r\n\r\n" },
status: "HTTP/1.1 500 Build Failed",
},
{
answer: "the 405 of the dev server to Connection: close",
files: { "index.html": page("./app.ts"), "app.ts": `console.log("app");` },
env: {
DEVELOPMENT: "true",
REQUEST: "POST / HTTP/1.1\r\nHost: HOST\r\nConnection: close\r\nContent-Length: 0\r\n\r\n",
},
status: "HTTP/1.1 405 Method Not Allowed",
},
])("$answer", async ({ files, env, status }) => {
using dir = tempDir("bun-serve-html-bodyless-close", { ...files, "serve.ts": serve });
const { stdout, stderr, exitCode } = await runServeFixture(dir, env);
expect({ stdout, exitCode }, stderr).toEqual({ stdout: status, exitCode: 0 });
});
});

// The rejection ends the bundle, and the next bundle reuses the memory that the first one freed.
test.concurrent("starts the queued bundle after an onLoad plugin's rejection ends a dev server bundle", async () => {
const rounds = [0, 1, 2, 3, 4, 5];
Expand Down
Loading
Loading