Skip to content
Open
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
36 changes: 24 additions & 12 deletions packages/bun-uws/src/HttpContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -744,9 +744,17 @@ struct HttpContext {
* nothing in AsyncSocketData::buffer). A retry that moves zero bytes
* after the peer's FIN is EPIPE; close instead of spinning. Except
* on libuv, where the retry can stall while the TLS layer's spill
* is still blocked on a healthy socket; there the kernel is asked. */
* is still blocked on a healthy socket; there the kernel is asked.
* HTTP_END_CALLED (with HTTP_RESPONSE_PENDING) is what identifies a
* tryEnd tail; it is the only deferred shape whose progress shows
* up in `offset`. A deferred file body (HTTP_FIXED_LENGTH_FILE_BODY)
* moves bytes with sendfile()/write() without touching it and
* would read as stalled here every time; it notices a dead peer on
* its own (a failed sendfile force-closes, a failed write() lands
* in the buffer and trips the flushed == 0 check above). */
if ((httpResponseData->state & HttpResponseData<SSL>::HTTP_NODE_RECEIVED_FIN)
&& (httpResponseData->state & HttpResponseData<SSL>::HTTP_RESPONSE_PENDING)
&& (httpResponseData->state & HttpResponseData<SSL>::HTTP_END_CALLED)
&& httpResponseData->offset == offsetBefore
&& asyncSocket->hasFullyDrained()
&& us_socket_stalled_write_means_peer_gone((us_socket_t *) asyncSocket)) {
Expand Down Expand Up @@ -861,22 +869,26 @@ struct HttpContext {
return s;
}
} else {
/* Bun.serve: response bytes already handed to uWS must drain before
* the connection shuts down (from the existing shouldCloseConnection()
* gates), not be discarded by the close() below. Only a response that
* is fully determined qualifies: a tryEnd tail (content-length path
* sets HTTP_END_CALLED while offset < total keeps HTTP_RESPONSE_PENDING)
* or a completed response that has not fully drained. A streaming body
* the application is still producing (HTTP_END_CALLED clear,
* HTTP_RESPONSE_PENDING set) closes here so onAborted / request.signal
* fires on client disconnect. */
/* Bun.serve: a response that is already fully determined must finish
* and drain before the connection shuts down (from the existing
* shouldCloseConnection() gates), not be cut off by the close() below.
* That is a tryEnd tail (the content-length path sets HTTP_END_CALLED
* while offset < total keeps HTTP_RESPONSE_PENDING), a file body the
* runtime is still delivering under a Content-Length that has already
* gone out (HTTP_FIXED_LENGTH_FILE_BODY), or a completed response that
* has not fully drained. A streaming body the application is still
* producing (neither bit, HTTP_RESPONSE_PENDING set) closes here so
* onAborted / request.signal fires on client disconnect. A deferred
* connection whose peer is really gone still closes: the kernel
* reports the reset, or the next write()/sendfile() fails (onWritable
* above, FileResponseStream). */
HttpResponseData<SSL> *httpResponseData = (HttpResponseData<SSL> *) us_socket_ext(s);
uint32_t state = httpResponseData->state;
bool tryEndTail = (state & HttpResponseData<SSL>::HTTP_END_CALLED)
bool determinedTail = (state & (HttpResponseData<SSL>::HTTP_END_CALLED | HttpResponseData<SSL>::HTTP_FIXED_LENGTH_FILE_BODY))
&& (state & HttpResponseData<SSL>::HTTP_RESPONSE_PENDING);
bool doneButBuffered = !(state & HttpResponseData<SSL>::HTTP_RESPONSE_PENDING)
&& !asyncSocket->hasFullyDrained();
if (tryEndTail || doneButBuffered) {
if (determinedTail || doneButBuffered) {
httpResponseData->state |= HttpResponseData<SSL>::HTTP_NODE_RECEIVED_FIN;
return s;
}
Expand Down
14 changes: 13 additions & 1 deletion packages/bun-uws/src/HttpResponseData.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ struct HttpResponseData : AsyncSocketData<SSL>, HttpParser {
* 'upgrade' listener's socket. */
HTTP_NODE_TUNNEL_AFTER_BODY = 1 << 14,
/* The peer half-closed (FIN) while there was still something to flush
* before teardown: buffered/pinned response bytes, or (with
* before teardown: buffered/pinned response bytes, a file body still
* being delivered (HTTP_FIXED_LENGTH_FILE_BODY), or (with
* httpAllowHalfOpen) an in-flight or queued pipelined response. The
* connection is shut down from the shouldCloseConnection()/onWritable
* gates once those have drained. Without httpAllowHalfOpen, onWritable
Expand All @@ -151,6 +152,17 @@ struct HttpResponseData : AsyncSocketData<SSL>, HttpParser {
* shutdown sweep; the shouldCloseConnection() gates act on it once the
* in-flight work completes. */
HTTP_CLOSE_WHEN_IDLE = 1 << 17,
/* The response in flight is a file of known size whose Content-Length
* is already on the wire, delivered by the runtime itself
* (FileResponseStream: sendfile() from the fd, or read()+write()
* chunks) rather than handed to uWS up front. Unlike a tryEnd tail
* nothing else in this object shows that such a response is already
* fully determined, so onEnd<false> needs this bit to let it finish
* after a peer FIN instead of closing on it (which leaves the client a
* short body under that Content-Length). Per-response: cleared by
* resetResponseState(), moot once markDone() clears
* HTTP_RESPONSE_PENDING. */
HTTP_FIXED_LENGTH_FILE_BODY = 1 << 18,

/* Bits that describe the connection rather than the response in flight.
* There is one HttpResponseData per socket, reused by every request on a
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/server/FileResponseStream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ impl FileResponseStream {

let resp = this_ref.resp.get();
resp.timeout(opts.idle_timeout);
if opts.length.is_some() {
resp.mark_fixed_length_file_body();
}
resp.on_aborted(
|p: *mut FileResponseStream, r| {
// SAFETY: uWS hands back the userdata pointer set below; the
Expand Down
11 changes: 11 additions & 0 deletions src/uws_sys/Response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,11 @@ impl<const SSL: bool> Response<SSL> {
c::uws_res_mark_wrote_content_length_header(Self::ssl_flag(), self.as_raw())
}

/// Sets `HTTP_FIXED_LENGTH_FILE_BODY`; see `HttpResponseData.h`.
pub(crate) fn mark_fixed_length_file_body(&mut self) {
c::uws_res_mark_fixed_length_file_body(Self::ssl_flag(), self.as_raw())
}

pub(crate) fn mark_wrote_date_header(&mut self) {
c::uws_res_mark_wrote_date_header(Self::ssl_flag(), self.as_raw())
}
Expand Down Expand Up @@ -721,6 +726,11 @@ impl AnyResponse {
any_dispatch!(self, |r| r.mark_wrote_content_length_header())
}

/// See `Response::mark_fixed_length_file_body`.
pub fn mark_fixed_length_file_body(self) {
any_dispatch!(self, |r| r.mark_fixed_length_file_body())
}

pub fn mark_wrote_date_header(self) {
any_dispatch!(self, |r| r.mark_wrote_date_header())
}
Expand Down Expand Up @@ -1081,6 +1091,7 @@ pub mod c {
// unsafe.
unsafe extern "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_fixed_length_file_body(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);
Expand Down
2 changes: 2 additions & 0 deletions src/uws_sys/h3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ impl Response {
pub(crate) fn mark_wrote_content_length_header(&mut self) {
c::uws_h3_res_mark_wrote_content_length_header(self)
}
/// No-op: only the TCP peer-FIN handling reads this bit.
pub(crate) fn mark_fixed_length_file_body(&mut self) {}
pub(crate) fn mark_wrote_date_header(&mut self) {
c::uws_h3_res_mark_wrote_date_header(self)
}
Expand Down
10 changes: 10 additions & 0 deletions src/uws_sys/libuwsockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,16 @@ extern "C"
}
}

void uws_res_mark_fixed_length_file_body(int ssl, uws_res_r res) {
if (ssl) {
uWS::HttpResponse<true> *uwsRes = (uWS::HttpResponse<true> *)res;
uwsRes->getHttpResponseData()->state |= uWS::HttpResponseData<true>::HTTP_FIXED_LENGTH_FILE_BODY;
} else {
uWS::HttpResponse<false> *uwsRes = (uWS::HttpResponse<false> *)res;
uwsRes->getHttpResponseData()->state |= uWS::HttpResponseData<false>::HTTP_FIXED_LENGTH_FILE_BODY;
}
}

void uws_res_mark_wrote_date_header(int ssl, uws_res_r res) {
if (ssl) {
uWS::HttpResponse<true> *uwsRes = (uWS::HttpResponse<true> *)res;
Expand Down
104 changes: 84 additions & 20 deletions test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4235,16 +4235,21 @@ it("survives aborted uploads while responding with a tee()d request-body branch"
});

// A client that half-closes its write side right after the request (the raw
// socket.end(request) pattern) must receive every response byte already handed
// to uWS, not just what the kernel accepted on the first send. No
// Connection: close on the request: the post-drain shutdown is driven by the
// HTTP_NODE_RECEIVED_FIN clause of shouldCloseConnection(), not by
// HTTP_CONNECTION_CLOSE.
// socket.end(request) pattern) must receive the whole of a response whose
// length is already settled: a body handed to uWS up front, or a file body the
// runtime streams itself under a Content-Length it has already sent. Not just
// what the kernel accepted on the first send. No Connection: close on the
// request: the post-drain shutdown is driven by the HTTP_NODE_RECEIVED_FIN
// clause of shouldCloseConnection(), not by HTTP_CONNECTION_CLOSE.
describe("a client half-close after the request does not truncate a large response body", () => {
const BODY = 8 * 1024 * 1024;

// The advertised Content-Length, the body bytes actually delivered, and
// whether the server closed cleanly (FIN rather than a reset).
const WHOLE_RESPONSE = { contentLength: BODY, body: BODY, ended: true };

function countBody(socket: net.Socket | nodeTls.TLSSocket) {
const out = { body: 0, ended: false };
const out = { contentLength: -1, body: 0, ended: false };
let head = "";
let gotHead = false;
socket.on("data", chunk => {
Expand All @@ -4253,6 +4258,7 @@ describe("a client half-close after the request does not truncate a large respon
const i = head.indexOf("\r\n\r\n");
if (i >= 0) {
gotHead = true;
out.contentLength = Number(/^content-length:\s*(\d+)/im.exec(head.slice(0, i))?.[1] ?? -1);
out.body = Buffer.byteLength(head.slice(i + 4), "latin1");
}
} else {
Expand All @@ -4264,22 +4270,28 @@ describe("a client half-close after the request does not truncate a large respon
return out;
}

async function halfCloseRequest(port: number): Promise<{ body: number; ended: boolean }> {
const socket = connect(port, "127.0.0.1");
async function halfCloseRequest(port: number, secure = false): Promise<typeof WHOLE_RESPONSE> {
const socket = secure
? nodeTls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false })
: connect(port, "127.0.0.1");
const out = countBody(socket);
const closed = new Promise<void>(r => socket.once("close", () => r()));
await new Promise<void>(r => socket.once("connect", () => r()));
await new Promise<void>(r => socket.once(secure ? "secureConnect" : "connect", () => r()));
socket.end("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n");
await closed;
return out;
}

function bigFile(prefix: string) {
return tempDir(prefix, { "big.bin": Buffer.alloc(BODY, "a") });
}

it("fetch handler (tryEnd tail)", async () => {
using server = serve({
port: 0,
fetch: () => new Response(Buffer.alloc(BODY, "a"), { headers: { "content-length": String(BODY) } }),
});
expect(await halfCloseRequest(server.port)).toEqual({ body: BODY, ended: true });
expect(await halfCloseRequest(server.port)).toEqual(WHOLE_RESPONSE);
});

it("static route (tryEnd tail)", async () => {
Expand All @@ -4290,7 +4302,7 @@ describe("a client half-close after the request does not truncate a large respon
},
fetch: () => new Response("miss", { status: 404 }),
});
expect(await halfCloseRequest(server.port)).toEqual({ body: BODY, ended: true });
expect(await halfCloseRequest(server.port)).toEqual(WHOLE_RESPONSE);
});

it("https fetch handler (tryEnd tail)", async () => {
Expand All @@ -4299,13 +4311,36 @@ describe("a client half-close after the request does not truncate a large respon
tls,
fetch: () => new Response(Buffer.alloc(BODY, "a"), { headers: { "content-length": String(BODY) } }),
});
const socket = nodeTls.connect({ port: server.port, host: "127.0.0.1", rejectUnauthorized: false });
const out = countBody(socket);
const closed = new Promise<void>(r => socket.once("close", () => r()));
await new Promise<void>(r => socket.once("secureConnect", () => r()));
socket.end("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n");
await closed;
expect(out).toEqual({ body: BODY, ended: true });
expect(await halfCloseRequest(server.port, true)).toEqual(WHOLE_RESPONSE);
});

// A file body is not handed to uWS up front: FileResponseStream writes the
// Content-Length, then moves the bytes itself (sendfile() on Linux over plain
// TCP, read()+write() chunks over TLS and on the other platforms), so uWS
// only knows the response is settled because the stream marks it as such
// (HTTP_FIXED_LENGTH_FILE_BODY). Without that mark the FIN closed the socket
// mid-transfer: a 200 with Content-Length: 8388608 followed by a few MiB and
// then the server's FIN.
it("fetch handler returning Bun.file() (file body)", async () => {
using dir = bigFile("half-close-file");
using server = serve({ port: 0, fetch: () => new Response(file(join(String(dir), "big.bin"))) });
expect(await halfCloseRequest(server.port)).toEqual(WHOLE_RESPONSE);
});

it("Bun.file() route (file body)", async () => {
using dir = bigFile("half-close-file-route");
using server = serve({
port: 0,
routes: { "/": file(join(String(dir), "big.bin")) },
fetch: () => new Response("miss", { status: 404 }),
});
expect(await halfCloseRequest(server.port)).toEqual(WHOLE_RESPONSE);
});

it("https fetch handler returning Bun.file() (file body)", async () => {
using dir = bigFile("half-close-file-tls");
using server = serve({ port: 0, tls, fetch: () => new Response(file(join(String(dir), "big.bin"))) });
expect(await halfCloseRequest(server.port, true)).toEqual(WHOLE_RESPONSE);
});

// The deferred connection must close promptly, not spin the writable
Expand Down Expand Up @@ -4337,9 +4372,38 @@ describe("a client half-close after the request does not truncate a large respon
expect(server.pendingRequests).toBe(0);
});

// Same for a deferred file body. It is not covered by onWritable's
// zero-progress check (its progress never shows up in the uWS write offset,
// which is why that check is scoped to tryEnd tails): the peer's reset is
// reported by the kernel, or the next sendfile()/write() fails and the
// stream tears the request down itself. Either way the request must not sit
// there until idleTimeout.
it("a file body closes without spinning when the peer goes away mid-transfer", async () => {
using dir = bigFile("half-close-file-peer-gone");
const dispatched = Promise.withResolvers<void>();
using server = serve({
port: 0,
idleTimeout: 60,
fetch() {
dispatched.resolve();
return new Response(file(join(String(dir), "big.bin")));
},
});
const socket = connect(server.port, "127.0.0.1");
socket.on("error", () => {});
await new Promise<void>(r => socket.once("connect", () => r()));
socket.end("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n");
socket.once("data", () => socket.destroy());
await dispatched.promise;
const deadline = Date.now() + 4000;
while (server.pendingRequests > 0 && Date.now() < deadline) await Bun.sleep(5);
expect(server.pendingRequests).toBe(0);
});

// The defer in onEnd is gated on the response being fully determined
// (HTTP_END_CALLED). A streaming body the handler is still producing must
// close on client FIN so onAborted / request.signal fires.
// (HTTP_END_CALLED or HTTP_FIXED_LENGTH_FILE_BODY). A streaming body the
// handler is still producing must close on client FIN so onAborted /
// request.signal fires.
it("request.signal still fires on client FIN for a streaming body", async () => {
const aborted = Promise.withResolvers<void>();
using server = serve({
Expand Down
Loading