From 0ac4cf59278e88c603eb4ff17cf574e4c5792088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Sat, 21 Mar 2026 12:33:46 -0300 Subject: [PATCH 1/4] fix: close connection after async chunked response with Connection: close Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/bun-uws/src/HttpResponse.h | 23 ++++ .../node-http-async-chunked-close.test.ts | 116 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 test/js/node/http/node-http-async-chunked-close.test.ts diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 99618dd72b8..e308839b719 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -154,6 +154,19 @@ struct HttpResponse : public AsyncSocket { } } else { this->uncork(); + /* After uncorking, check if we should close this connection. + * This handles the case where writeHead corked the socket outside + * the uWS request handler (async response), so the close check + * in HttpContext's onData handler never runs for this response. */ + if (httpResponseData->state & HttpResponseData::HTTP_CONNECTION_CLOSE) { + if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { + if (((AsyncSocket *) this)->getBufferedAmount() == 0) { + ((AsyncSocket *) this)->shutdown(); + ((AsyncSocket *) this)->close(); + return true; + } + } + } } /* tryEnd can never fail when in chunked mode, since we do not have tryWrite (yet), only write */ @@ -219,6 +232,16 @@ struct HttpResponse : public AsyncSocket { } } else { this->uncork(); + /* After uncorking, check if we should close this connection. + * Same fix as the chunked path above. */ + if (httpResponseData->state & HttpResponseData::HTTP_CONNECTION_CLOSE) { + if ((httpResponseData->state & HttpResponseData::HTTP_RESPONSE_PENDING) == 0) { + if (((AsyncSocket *) this)->getBufferedAmount() == 0) { + ((AsyncSocket *) this)->shutdown(); + ((AsyncSocket *) this)->close(); + } + } + } } } diff --git a/test/js/node/http/node-http-async-chunked-close.test.ts b/test/js/node/http/node-http-async-chunked-close.test.ts new file mode 100644 index 00000000000..15da4b2233c --- /dev/null +++ b/test/js/node/http/node-http-async-chunked-close.test.ts @@ -0,0 +1,116 @@ +import { test, expect } from "bun:test"; +import * as http from "node:http"; +import * as net from "node:net"; + +test("http.Server closes connection after async chunked response with Connection: close", async () => { + // Server that responds asynchronously with chunked encoding + const server = http.createServer((req, res) => { + setTimeout(() => { + res.writeHead(200, { "Content-Type": "text/plain" }); + res.write("chunk1"); + res.write("chunk2"); + res.write("chunk3"); + res.end(); + }, 10); + }); + + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const port = (server.address() as net.AddressInfo).port; + + const result = await new Promise<{ pass: boolean; body: string }>(resolve => { + const socket = net.createConnection(port, "127.0.0.1", () => { + socket.write("GET / HTTP/1.1\r\nHost: test\r\nConnection: close\r\n\r\n"); + }); + + const chunks: Buffer[] = []; + socket.on("data", c => chunks.push(c)); + socket.on("end", () => { + resolve({ pass: true, body: Buffer.concat(chunks).toString() }); + }); + socket.setTimeout(3000, () => { + resolve({ pass: false, body: Buffer.concat(chunks).toString() }); + socket.destroy(); + }); + }); + + server.close(); + + expect(result.body).toContain("chunk1"); + expect(result.body).toContain("chunk2"); + expect(result.body).toContain("chunk3"); + expect(result.pass).toBe(true); +}); + +test("proxy with createConnection closes socket after chunked upstream response", async () => { + // Backend responds with chunked transfer encoding + const backend = http.createServer((_req, res) => { + res.writeHead(200, { + "Content-Type": "text/plain", + "Transfer-Encoding": "chunked", + }); + res.write("chunk1"); + res.write("chunk2"); + res.write("chunk3"); + res.end(); + }); + + // Proxy forwards via createConnection to backend + const proxy = http.createServer((req, res) => { + const sock = net.createConnection((backend.address() as net.AddressInfo).port, "127.0.0.1"); + + const proxyReq = http.request({ + method: req.method, + path: req.url, + headers: { ...req.headers, connection: "close" }, + createConnection: () => sock as net.Socket, + }); + + proxyReq.on("response", proxyRes => { + const headers: Record = {}; + for (const [k, v] of Object.entries(proxyRes.headers)) { + if (k.toLowerCase() === "transfer-encoding") continue; + headers[k] = v; + } + headers["connection"] = "close"; + res.writeHead(proxyRes.statusCode ?? 502, headers); + proxyRes.pipe(res); + }); + + proxyReq.on("error", err => { + console.error("proxyReq error:", err.message); + res.writeHead(502); + res.end("Bad Gateway"); + }); + + req.pipe(proxyReq); + }); + + await new Promise(resolve => backend.listen(0, "127.0.0.1", resolve)); + await new Promise(resolve => proxy.listen(0, "127.0.0.1", resolve)); + + const proxyPort = (proxy.address() as net.AddressInfo).port; + + const result = await new Promise<{ pass: boolean; body: string }>(resolve => { + const socket = net.createConnection(proxyPort, "127.0.0.1", () => { + socket.write("GET / HTTP/1.1\r\nHost: test\r\nConnection: close\r\n\r\n"); + }); + + const chunks: Buffer[] = []; + socket.on("data", c => chunks.push(c)); + socket.on("end", () => { + resolve({ pass: true, body: Buffer.concat(chunks).toString() }); + }); + socket.setTimeout(3000, () => { + resolve({ pass: false, body: Buffer.concat(chunks).toString() }); + socket.destroy(); + }); + }); + + backend.close(); + proxy.close(); + + expect(result.body).toContain("chunk1"); + expect(result.body).toContain("chunk2"); + expect(result.body).toContain("chunk3"); + expect(result.pass).toBe(true); +}); From 0b70714cc29ab641de33165dac36cf00e62d2cec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Sat, 21 Mar 2026 12:47:15 -0300 Subject: [PATCH 2/4] fix: use try/finally for server cleanup, remove proxy test Address CodeRabbit review: wrap server cleanup in try/finally for exception safety. Remove proxy/createConnection test that depends on changes from another branch. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../node-http-async-chunked-close.test.ts | 120 ++++-------------- 1 file changed, 24 insertions(+), 96 deletions(-) diff --git a/test/js/node/http/node-http-async-chunked-close.test.ts b/test/js/node/http/node-http-async-chunked-close.test.ts index 15da4b2233c..13d359b967a 100644 --- a/test/js/node/http/node-http-async-chunked-close.test.ts +++ b/test/js/node/http/node-http-async-chunked-close.test.ts @@ -17,100 +17,28 @@ test("http.Server closes connection after async chunked response with Connection await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); const port = (server.address() as net.AddressInfo).port; - const result = await new Promise<{ pass: boolean; body: string }>(resolve => { - const socket = net.createConnection(port, "127.0.0.1", () => { - socket.write("GET / HTTP/1.1\r\nHost: test\r\nConnection: close\r\n\r\n"); - }); - - const chunks: Buffer[] = []; - socket.on("data", c => chunks.push(c)); - socket.on("end", () => { - resolve({ pass: true, body: Buffer.concat(chunks).toString() }); - }); - socket.setTimeout(3000, () => { - resolve({ pass: false, body: Buffer.concat(chunks).toString() }); - socket.destroy(); - }); - }); - - server.close(); - - expect(result.body).toContain("chunk1"); - expect(result.body).toContain("chunk2"); - expect(result.body).toContain("chunk3"); - expect(result.pass).toBe(true); -}); - -test("proxy with createConnection closes socket after chunked upstream response", async () => { - // Backend responds with chunked transfer encoding - const backend = http.createServer((_req, res) => { - res.writeHead(200, { - "Content-Type": "text/plain", - "Transfer-Encoding": "chunked", - }); - res.write("chunk1"); - res.write("chunk2"); - res.write("chunk3"); - res.end(); - }); - - // Proxy forwards via createConnection to backend - const proxy = http.createServer((req, res) => { - const sock = net.createConnection((backend.address() as net.AddressInfo).port, "127.0.0.1"); - - const proxyReq = http.request({ - method: req.method, - path: req.url, - headers: { ...req.headers, connection: "close" }, - createConnection: () => sock as net.Socket, - }); - - proxyReq.on("response", proxyRes => { - const headers: Record = {}; - for (const [k, v] of Object.entries(proxyRes.headers)) { - if (k.toLowerCase() === "transfer-encoding") continue; - headers[k] = v; - } - headers["connection"] = "close"; - res.writeHead(proxyRes.statusCode ?? 502, headers); - proxyRes.pipe(res); - }); - - proxyReq.on("error", err => { - console.error("proxyReq error:", err.message); - res.writeHead(502); - res.end("Bad Gateway"); - }); - - req.pipe(proxyReq); - }); - - await new Promise(resolve => backend.listen(0, "127.0.0.1", resolve)); - await new Promise(resolve => proxy.listen(0, "127.0.0.1", resolve)); - - const proxyPort = (proxy.address() as net.AddressInfo).port; - - const result = await new Promise<{ pass: boolean; body: string }>(resolve => { - const socket = net.createConnection(proxyPort, "127.0.0.1", () => { - socket.write("GET / HTTP/1.1\r\nHost: test\r\nConnection: close\r\n\r\n"); - }); - - const chunks: Buffer[] = []; - socket.on("data", c => chunks.push(c)); - socket.on("end", () => { - resolve({ pass: true, body: Buffer.concat(chunks).toString() }); - }); - socket.setTimeout(3000, () => { - resolve({ pass: false, body: Buffer.concat(chunks).toString() }); - socket.destroy(); - }); - }); - - backend.close(); - proxy.close(); - - expect(result.body).toContain("chunk1"); - expect(result.body).toContain("chunk2"); - expect(result.body).toContain("chunk3"); - expect(result.pass).toBe(true); + try { + const result = await new Promise<{ pass: boolean; body: string }>(resolve => { + const socket = net.createConnection(port, "127.0.0.1", () => { + socket.write("GET / HTTP/1.1\r\nHost: test\r\nConnection: close\r\n\r\n"); + }); + + const chunks: Buffer[] = []; + socket.on("data", c => chunks.push(c)); + socket.on("end", () => { + resolve({ pass: true, body: Buffer.concat(chunks).toString() }); + }); + socket.setTimeout(3000, () => { + resolve({ pass: false, body: Buffer.concat(chunks).toString() }); + socket.destroy(); + }); + }); + + expect(result.body).toContain("chunk1"); + expect(result.body).toContain("chunk2"); + expect(result.body).toContain("chunk3"); + expect(result.pass).toBe(true); + } finally { + server.close(); + } }); From 1792019f2975e20561980ad7076a83717f8d6343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Sun, 22 Mar 2026 23:23:11 -0300 Subject: [PATCH 3/4] fix: return after close in non-chunked async path to prevent use-after-free The non-chunked async uncork path called shutdown() + close() but did not return immediately afterward, allowing execution to fall through to `return success` where `this` is a dangling pointer. Add `return true` after close() to match the chunked path pattern. Found during review of #28397 (robobun). Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/bun-uws/src/HttpResponse.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index e308839b719..91469110c7a 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -239,6 +239,9 @@ struct HttpResponse : public AsyncSocket { if (((AsyncSocket *) this)->getBufferedAmount() == 0) { ((AsyncSocket *) this)->shutdown(); ((AsyncSocket *) this)->close(); + /* Return immediately after close to prevent + * use-after-free on the freed socket. */ + return true; } } } From eee3974db9096c2e6ce548434143653458a258c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Sun, 22 Mar 2026 23:46:42 -0300 Subject: [PATCH 4/4] fix: add socket error handler to prevent test hang on connection failure Co-Authored-By: Claude Opus 4.6 (1M context) --- test/js/node/http/node-http-async-chunked-close.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/js/node/http/node-http-async-chunked-close.test.ts b/test/js/node/http/node-http-async-chunked-close.test.ts index 13d359b967a..5d38a171729 100644 --- a/test/js/node/http/node-http-async-chunked-close.test.ts +++ b/test/js/node/http/node-http-async-chunked-close.test.ts @@ -25,6 +25,10 @@ test("http.Server closes connection after async chunked response with Connection const chunks: Buffer[] = []; socket.on("data", c => chunks.push(c)); + socket.on("error", err => { + resolve({ pass: false, body: `Connection error: ${err.message}` }); + socket.destroy(); + }); socket.on("end", () => { resolve({ pass: true, body: Buffer.concat(chunks).toString() }); });