From d117519f777ca0e1f84a74ca6840795d1d031944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Tue, 17 Mar 2026 23:51:03 -0300 Subject: [PATCH 1/7] fix(node:http): support createConnection option in http.request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The createConnection option was completely ignored — the callback was never called. This broke libraries that rely on custom connection creation (e.g., WebSocket libraries, proxy agents). When createConnection is provided, bypass the internal fetch infrastructure and use a socket-based HTTP/1.1 path: serialize the request, write it to the user-provided socket, and parse the response inline. This mirrors the pattern already used by HTTP/2 (http2.ts L3728). Closes #7471 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 195 ++++++++++++++ test/regression/issue/7471.test.ts | 394 +++++++++++++++++++++++++++++ 2 files changed, 589 insertions(+) create mode 100644 test/regression/issue/7471.test.ts diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index 37cde3a014f..b92e47236d9 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -51,6 +51,9 @@ const { reqSymbol, callCloseCallback, emitCloseNTAndComplete, + bodyStreamSymbol, + statusCodeSymbol, + statusMessageSymbol, } = require("internal/http"); const { globalAgent } = require("node:_http_agent"); @@ -266,11 +269,201 @@ function ClientRequest(input, options, cb) { let fetching = false; + const startFetchViaSocket = () => { + fetching = true; + + const method = this[kMethod]; + const path = this[kPath]; + const host = this[kHost]; + const port = this[kPort]; + const protocol = this[kProtocol]; + const socketPath = this[kSocketPath]; + + // Pass full options to createConnection, matching Node.js behavior. + // The options object is compatible with net.connect() / tls.connect(). + const connectOptions = { ...options, host, port, path: socketPath || undefined }; + + let socket; + try { + socket = createConnection(connectOptions); + } catch (err) { + process.nextTick((self, e) => self.emit("error", e), this, err); + return false; + } + + this.socket = socket; + this[kAbortController]?.signal?.addEventListener("abort", () => socket.destroy(), { once: true }); + + // --- Write HTTP/1.1 request --- + const writeRequest = () => { + const headers = this.getHeaders(); + let head = `${method} ${path} HTTP/1.1\r\n`; + + if (!headers.host && !headers.Host) { + const dp = protocol === "https:" ? 443 : 80; + head += port && port !== dp ? `Host: ${host}:${port}\r\n` : `Host: ${host}\r\n`; + } + + const chunks = this[kBodyChunks]; + let body; + if (chunks?.length > 0) { + const bufs = chunks.map(c => (typeof c === "string" ? Buffer.from(c) : c)); + body = bufs.length === 1 ? bufs[0] : Buffer.concat(bufs); + } + + for (const key of Object.keys(headers)) { + const val = headers[key]; + if (val === undefined) continue; + if ($isJSArray(val)) { + for (const v of val) head += `${key}: ${v}\r\n`; + } else { + head += `${key}: ${val}\r\n`; + } + } + + if (body) head += `Content-Length: ${body.byteLength}\r\n`; + head += "\r\n"; + socket.write(head); + if (body) socket.write(body); + }; + + // --- Parse HTTP/1.1 response --- + let hdrBuf = Buffer.alloc(0); + let hdrDone = false; + let cLen = -1; + let chunked = false; + let bodyRcvd = 0; + let chkBuf = Buffer.alloc(0); + let res: any = null; + + const responseComplete = () => { + if (res && !res.complete) { + res.push(null); + res.complete = true; + } + this[kClearTimeout](); + fetching = false; + this[kFetchRequest] = null; + maybeEmitClose(); + }; + + const feedBody = (data: Buffer) => { + if (!res || res._dumped) return; + if (chunked) { + chkBuf = chkBuf.length ? Buffer.concat([chkBuf, data]) : data; + while (true) { + const nl = chkBuf.indexOf("\r\n"); + if (nl === -1) break; + const sz = parseInt(chkBuf.slice(0, nl).toString(), 16); + if (isNaN(sz)) { socket.destroy(); return; } + if (sz === 0) { responseComplete(); return; } + if (chkBuf.length < nl + 2 + sz + 2) break; + res.push(chkBuf.slice(nl + 2, nl + 2 + sz)); + chkBuf = chkBuf.slice(nl + 2 + sz + 2); + } + } else if (cLen >= 0) { + bodyRcvd += data.length; + res.push(data); + if (bodyRcvd >= cLen) responseComplete(); + } else { + res.push(data); + } + }; + + const onHeaders = (sep: number) => { + const lines = hdrBuf.slice(0, sep).toString("latin1").split("\r\n"); + const statusLine = lines[0]; + const sp1 = statusLine.indexOf(" "); + const sp2 = statusLine.indexOf(" ", sp1 + 1); + const statusCode = parseInt(sp2 === -1 ? statusLine.slice(sp1 + 1) : statusLine.slice(sp1 + 1, sp2), 10); + const statusMessage = sp2 === -1 ? "" : statusLine.slice(sp2 + 1); + + const respHeaders: any = Object.create(null); + const rawHeaders: string[] = []; + for (let i = 1; i < lines.length; i++) { + const colon = lines[i].indexOf(":"); + if (colon === -1) continue; + const k = lines[i].slice(0, colon), v = lines[i].slice(colon + 1).trim(), lk = k.toLowerCase(); + rawHeaders.push(k, v); + if (lk === "set-cookie") respHeaders[lk] = respHeaders[lk] ? [...respHeaders[lk], v] : [v]; + else respHeaders[lk] = v; + if (lk === "content-length") cLen = parseInt(v, 10); + else if (lk === "transfer-encoding" && v.toLowerCase().includes("chunked")) chunked = true; + } + hdrDone = true; + + const prevIsHTTPS = getIsNextIncomingMessageHTTPS(); + setIsNextIncomingMessageHTTPS(protocol === "https:"); + res = new IncomingMessage(null, {}); + setIsNextIncomingMessageHTTPS(prevIsHTTPS); + + res[statusCodeSymbol] = statusCode; + res[statusMessageSymbol] = statusMessage; + res.headers = respHeaders; + res.rawHeaders = rawHeaders; + res[bodyStreamSymbol] = true; // Prevent _read from accessing fetch APIs + res.socket = socket; + this.res = res; + res.req = this; + this[kClearTimeout](); + + if (this.aborted) { maybeEmitClose(); return; } + if (!this.emit("response", res)) res._dump(); + maybeEmitClose(); + + if (method === "HEAD" || cLen === 0 || statusCode === 204 || statusCode === 304) { + responseComplete(); + return; + } + + const rest = hdrBuf.slice(sep + 4); + if (rest.length > 0) feedBody(rest); + }; + + socket.on("data", (chunk) => { + if (!hdrDone) { + hdrBuf = hdrBuf.length ? Buffer.concat([hdrBuf, chunk]) : chunk; + const sep = hdrBuf.indexOf("\r\n\r\n"); + if (sep !== -1) onHeaders(sep); + } else { + feedBody(chunk); + } + }); + socket.on("error", (err) => { + if (isAbortError(err)) return; + try { this.emit("error", err); } catch {} + }); + socket.on("end", () => { + if (hdrDone && !chunked && cLen < 0 && res && !res.complete) responseComplete(); + }); + socket.on("close", () => { + if (res && !res.complete) responseComplete(); + socketCloseListener(); + }); + + // Wait for connection (TCP or TLS handshake) before writing, matching http2 pattern + if (socket.connecting || socket.secureConnecting) { + const connectEvent = socket.secureConnecting ? "secureConnect" : "connect"; + socket.once(connectEvent, writeRequest); + } else { + process.nextTick(writeRequest); + } + + return true; + }; + const startFetch = (customBody?) => { if (fetching) { return false; } + // Socket-based path: when createConnection is provided, bypass the fetch + // infrastructure and use raw HTTP/1.1 over the user-provided socket. + if (typeof createConnection === "function") { + if (!this.finished) return false; // Wait until end() is called + return startFetchViaSocket(); + } + fetching = true; const method = this[kMethod]; @@ -918,6 +1111,8 @@ function ClientRequest(input, options, cb) { const { signal: _signal, ...optsWithoutSignal } = options; this[kOptions] = optsWithoutSignal; + const createConnection = options.createConnection; + this._httpMessage = this; process.nextTick(emitContinueAndSocketNT, this); diff --git a/test/regression/issue/7471.test.ts b/test/regression/issue/7471.test.ts new file mode 100644 index 00000000000..7e5b64b579e --- /dev/null +++ b/test/regression/issue/7471.test.ts @@ -0,0 +1,394 @@ +import { test, expect, describe } from "bun:test"; +import { bunEnv, bunExe } from "harness"; +import { tmpdir } from "os"; +import { join } from "path"; + +describe("http.request createConnection", () => { + test("is called for GET requests", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + let called = false; + const server = http.createServer((req, res) => res.end("hello")); + server.listen(0, () => { + http.get({ + port: server.address().port, + path: "/test", + createConnection: (opts) => { called = true; return net.connect(opts); }, + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ called, data: d, status: res.statusCode })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ called: true, data: "hello", status: 200 }); + expect(exitCode).toBe(0); + }); + + test("works with POST body", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => { + let b = ""; + req.on("data", (c) => b += c); + req.on("end", () => res.end("echo:" + b)); + }); + server.listen(0, () => { + const req = http.request({ + port: server.address().port, + method: "POST", + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ data: d, status: res.statusCode })); + server.close(); + }); + }); + req.end("payload"); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ data: "echo:payload", status: 200 }); + expect(exitCode).toBe(0); + }); + + test("works with unix socket", async () => { + if (process.platform === "win32") return; + const sockPath = join(tmpdir(), `bun-test-7471-${Date.now()}.sock`); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const sockPath = ${JSON.stringify(sockPath)}; + let called = false; + const server = http.createServer((req, res) => res.end("unix ok")); + server.listen(sockPath, () => { + http.get({ + socketPath: sockPath, + path: "/", + createConnection: (opts) => { called = true; return net.connect(opts); }, + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ called, data: d })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ called: true, data: "unix ok" }); + expect(exitCode).toBe(0); + }); + + test("receives full options object", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + let receivedOpts = null; + const server = http.createServer((req, res) => res.end("ok")); + server.listen(0, () => { + const port = server.address().port; + http.get({ + host: "127.0.0.1", + port, + path: "/check", + createConnection: (opts) => { + receivedOpts = { host: opts.host, port: opts.port }; + return net.connect(opts); + }, + }, (res) => { + res.resume(); + res.on("end", () => { + console.log(JSON.stringify(receivedOpts)); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r.host).toBe("127.0.0.1"); + expect(typeof r.port).toBe("number"); + expect(exitCode).toBe(0); + }); + + test("handles chunked transfer encoding", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => { + res.writeHead(200, { "Transfer-Encoding": "chunked" }); + res.write("chunk1"); + res.write("chunk2"); + res.end("chunk3"); + }); + server.listen(0, () => { + http.get({ + port: server.address().port, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ data: d, status: res.statusCode })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ data: "chunk1chunk2chunk3", status: 200 }); + expect(exitCode).toBe(0); + }); + + test("handles response with no body (204)", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => { + res.writeHead(204); + res.end(); + }); + server.listen(0, () => { + http.get({ + port: server.address().port, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ data: d, status: res.statusCode })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ data: "", status: 204 }); + expect(exitCode).toBe(0); + }); + + test("emits socket event with real socket", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => res.end("ok")); + server.listen(0, () => { + const req = http.get({ + port: server.address().port, + createConnection: (opts) => net.connect(opts), + }, (res) => { + res.resume(); + res.on("end", () => server.close()); + }); + req.on("socket", (sock) => { + console.log(JSON.stringify({ isSocket: sock instanceof net.Socket })); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r.isSocket).toBe(true); + expect(exitCode).toBe(0); + }); + + test("handles custom headers", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => { + res.end(JSON.stringify({ + xCustom: req.headers["x-custom"], + accept: req.headers["accept"], + })); + }); + server.listen(0, () => { + http.get({ + port: server.address().port, + headers: { "X-Custom": "test-value", "Accept": "application/json" }, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(d); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ xCustom: "test-value", accept: "application/json" }); + expect(exitCode).toBe(0); + }); + + test("response headers are parsed correctly", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => { + res.writeHead(201, "Created", { + "X-Response-Id": "abc123", + "Content-Type": "text/plain", + }); + res.end("created"); + }); + server.listen(0, () => { + http.get({ + port: server.address().port, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ + status: res.statusCode, + statusMsg: res.statusMessage, + xId: res.headers["x-response-id"], + ct: res.headers["content-type"], + data: d, + })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ + status: 201, + statusMsg: "Created", + xId: "abc123", + ct: "text/plain", + data: "created", + }); + expect(exitCode).toBe(0); + }); + + test("works without createConnection (no regression)", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const server = http.createServer((req, res) => res.end("normal")); + server.listen(0, () => { + http.get({ port: server.address().port }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ data: d, status: res.statusCode })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ data: "normal", status: 200 }); + expect(exitCode).toBe(0); + }); +}); From 092d315f71a2c1c3f16479ee5060bef203db7b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Wed, 18 Mar 2026 00:49:49 -0300 Subject: [PATCH 2/7] fix(node:http): use HTTPParser (llhttp) instead of hand-rolled parser Replace the hand-rolled HTTP/1.1 response parser in the createConnection socket path with Bun's existing HTTPParser binding (backed by llhttp). This reuses the same parser the HTTP server already uses, giving us complete RFC 9112 compliance: chunked extensions, trailer headers, 100 Continue, header overflow, transfer-encoding validation, etc. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 117 ++++++++++++++--------------- test/regression/issue/7471.test.ts | 57 ++++++++++++++ 2 files changed, 112 insertions(+), 62 deletions(-) diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index b92e47236d9..503523131b9 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -56,6 +56,8 @@ const { statusMessageSymbol, } = require("internal/http"); +const { HTTPParser, freeParser } = require("node:_http_common"); + const { globalAgent } = require("node:_http_agent"); const { IncomingMessage } = require("node:_http_incoming"); const { OutgoingMessage } = require("node:_http_outgoing"); @@ -327,13 +329,14 @@ function ClientRequest(input, options, cb) { if (body) socket.write(body); }; - // --- Parse HTTP/1.1 response --- - let hdrBuf = Buffer.alloc(0); - let hdrDone = false; - let cLen = -1; - let chunked = false; - let bodyRcvd = 0; - let chkBuf = Buffer.alloc(0); + // --- Parse HTTP/1.1 response using llhttp (via HTTPParser) --- + const parser = new HTTPParser(); + parser._headers = []; + parser._url = ""; + parser.maxHeaderPairs = 2000; + parser.socket = socket; + parser.initialize(HTTPParser.RESPONSE, {}); + let res: any = null; const responseComplete = () => { @@ -344,53 +347,27 @@ function ClientRequest(input, options, cb) { this[kClearTimeout](); fetching = false; this[kFetchRequest] = null; + freeParser(parser, this, socket); maybeEmitClose(); }; - const feedBody = (data: Buffer) => { - if (!res || res._dumped) return; - if (chunked) { - chkBuf = chkBuf.length ? Buffer.concat([chkBuf, data]) : data; - while (true) { - const nl = chkBuf.indexOf("\r\n"); - if (nl === -1) break; - const sz = parseInt(chkBuf.slice(0, nl).toString(), 16); - if (isNaN(sz)) { socket.destroy(); return; } - if (sz === 0) { responseComplete(); return; } - if (chkBuf.length < nl + 2 + sz + 2) break; - res.push(chkBuf.slice(nl + 2, nl + 2 + sz)); - chkBuf = chkBuf.slice(nl + 2 + sz + 2); - } - } else if (cLen >= 0) { - bodyRcvd += data.length; - res.push(data); - if (bodyRcvd >= cLen) responseComplete(); - } else { - res.push(data); + // Build headers object from the flat [key, val, key, val, ...] array llhttp produces + const buildHeaders = (rawHeaders: string[]) => { + const headers: any = Object.create(null); + for (let i = 0; i < rawHeaders.length; i += 2) { + const lk = rawHeaders[i].toLowerCase(); + const v = rawHeaders[i + 1]; + if (lk === "set-cookie") headers[lk] = headers[lk] ? [...headers[lk], v] : [v]; + else headers[lk] = v; } + return headers; }; - const onHeaders = (sep: number) => { - const lines = hdrBuf.slice(0, sep).toString("latin1").split("\r\n"); - const statusLine = lines[0]; - const sp1 = statusLine.indexOf(" "); - const sp2 = statusLine.indexOf(" ", sp1 + 1); - const statusCode = parseInt(sp2 === -1 ? statusLine.slice(sp1 + 1) : statusLine.slice(sp1 + 1, sp2), 10); - const statusMessage = sp2 === -1 ? "" : statusLine.slice(sp2 + 1); - - const respHeaders: any = Object.create(null); - const rawHeaders: string[] = []; - for (let i = 1; i < lines.length; i++) { - const colon = lines[i].indexOf(":"); - if (colon === -1) continue; - const k = lines[i].slice(0, colon), v = lines[i].slice(colon + 1).trim(), lk = k.toLowerCase(); - rawHeaders.push(k, v); - if (lk === "set-cookie") respHeaders[lk] = respHeaders[lk] ? [...respHeaders[lk], v] : [v]; - else respHeaders[lk] = v; - if (lk === "content-length") cLen = parseInt(v, 10); - else if (lk === "transfer-encoding" && v.toLowerCase().includes("chunked")) chunked = true; + parser[HTTPParser.kOnHeadersComplete] = (vMaj, vMin, headers, _method, _url, statusCode, statusMessage, upgrade, shouldKeepAlive) => { + if (headers === undefined) { + headers = parser._headers; + parser._headers = []; } - hdrDone = true; const prevIsHTTPS = getIsNextIncomingMessageHTTPS(); setIsNextIncomingMessageHTTPS(protocol === "https:"); @@ -399,34 +376,49 @@ function ClientRequest(input, options, cb) { res[statusCodeSymbol] = statusCode; res[statusMessageSymbol] = statusMessage; - res.headers = respHeaders; - res.rawHeaders = rawHeaders; + res.httpVersion = `${vMaj}.${vMin}`; + res.headers = buildHeaders(headers); + res.rawHeaders = headers; res[bodyStreamSymbol] = true; // Prevent _read from accessing fetch APIs res.socket = socket; this.res = res; res.req = this; this[kClearTimeout](); - if (this.aborted) { maybeEmitClose(); return; } + if (this.aborted) { maybeEmitClose(); return 2; } if (!this.emit("response", res)) res._dump(); maybeEmitClose(); - if (method === "HEAD" || cLen === 0 || statusCode === 204 || statusCode === 304) { - responseComplete(); - return; + // Return value: 0 = parse body, 1 = skip body (HEAD) + return method === "HEAD" ? 1 : 0; + }; + + parser[HTTPParser.kOnBody] = (chunk) => { + if (res && !res._dumped) res.push(chunk); + }; + + parser[HTTPParser.kOnMessageComplete] = () => { + // Handle trailing headers — override the noop prototype getter/setter + if (parser._headers.length && res) { + const trailers = buildHeaders(parser._headers); + const rawTrailers = parser._headers.slice(); + Object.defineProperty(res, "trailers", { value: trailers, writable: true, enumerable: true, configurable: true }); + Object.defineProperty(res, "rawTrailers", { value: rawTrailers, writable: true, enumerable: true, configurable: true }); + parser._headers = []; } + responseComplete(); + }; - const rest = hdrBuf.slice(sep + 4); - if (rest.length > 0) feedBody(rest); + parser[HTTPParser.kOnHeaders] = (headers) => { + // Accumulate trailing headers (called when headers arrive in fragments or as trailers) + parser._headers.push(...headers); }; socket.on("data", (chunk) => { - if (!hdrDone) { - hdrBuf = hdrBuf.length ? Buffer.concat([hdrBuf, chunk]) : chunk; - const sep = hdrBuf.indexOf("\r\n\r\n"); - if (sep !== -1) onHeaders(sep); - } else { - feedBody(chunk); + const ret = parser.execute(chunk); + if (ret instanceof Error) { + socket.destroy(); + this.emit("error", ret); } }); socket.on("error", (err) => { @@ -434,7 +426,8 @@ function ClientRequest(input, options, cb) { try { this.emit("error", err); } catch {} }); socket.on("end", () => { - if (hdrDone && !chunked && cLen < 0 && res && !res.complete) responseComplete(); + parser.finish(); + if (res && !res.complete) responseComplete(); }); socket.on("close", () => { if (res && !res.complete) responseComplete(); diff --git a/test/regression/issue/7471.test.ts b/test/regression/issue/7471.test.ts index 7e5b64b579e..b3c6204b607 100644 --- a/test/regression/issue/7471.test.ts +++ b/test/regression/issue/7471.test.ts @@ -362,6 +362,63 @@ describe("http.request createConnection", () => { expect(exitCode).toBe(0); }); + test("handles chunked extensions and trailer headers", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + + // Use a raw TCP server to send a response with chunked extensions and trailers + const server = net.createServer((sock) => { + sock.on("data", () => { + sock.write( + "HTTP/1.1 200 OK\\r\\n" + + "Transfer-Encoding: chunked\\r\\n" + + "Trailer: X-Checksum\\r\\n" + + "\\r\\n" + + "5;ext=val\\r\\nhello\\r\\n" + + "6\\r\\n world\\r\\n" + + "0\\r\\n" + + "X-Checksum: abc123\\r\\n" + + "\\r\\n" + ); + sock.end(); + }); + }); + server.listen(0, () => { + http.get({ + port: server.address().port, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ + data: d, + status: res.statusCode, + trailers: res.trailers, + })); + server.close(); + }); + }); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r.data).toBe("hello world"); + expect(r.status).toBe(200); + expect(r.trailers).toEqual({ "x-checksum": "abc123" }); + expect(exitCode).toBe(0); + }); + test("works without createConnection (no regression)", async () => { await using proc = Bun.spawn({ cmd: [ From 98d5b400e1e79fcc84a7bcd4284cdad5b6e7620d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Wed, 18 Mar 2026 01:02:47 -0300 Subject: [PATCH 3/7] fix: address CodeRabbit review comments - Bracket IPv6 addresses in Host header per RFC 3986 - Log swallowed errors in debug builds instead of silent catch - Use tempDir from harness for test isolation Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 7 +++++-- test/regression/issue/7471.test.ts | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index 503523131b9..5bb30a321b3 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -303,7 +303,8 @@ function ClientRequest(input, options, cb) { if (!headers.host && !headers.Host) { const dp = protocol === "https:" ? 443 : 80; - head += port && port !== dp ? `Host: ${host}:${port}\r\n` : `Host: ${host}\r\n`; + const hostStr = isIPv6(host) ? `[${host}]` : host; + head += port && port !== dp ? `Host: ${hostStr}:${port}\r\n` : `Host: ${hostStr}\r\n`; } const chunks = this[kBodyChunks]; @@ -423,7 +424,9 @@ function ClientRequest(input, options, cb) { }); socket.on("error", (err) => { if (isAbortError(err)) return; - try { this.emit("error", err); } catch {} + try { this.emit("error", err); } catch (e) { + if (!!$debug) globalReportError(e); + } }); socket.on("end", () => { parser.finish(); diff --git a/test/regression/issue/7471.test.ts b/test/regression/issue/7471.test.ts index b3c6204b607..061c945878a 100644 --- a/test/regression/issue/7471.test.ts +++ b/test/regression/issue/7471.test.ts @@ -1,6 +1,5 @@ import { test, expect, describe } from "bun:test"; -import { bunEnv, bunExe } from "harness"; -import { tmpdir } from "os"; +import { bunEnv, bunExe, tempDir } from "harness"; import { join } from "path"; describe("http.request createConnection", () => { @@ -82,7 +81,8 @@ describe("http.request createConnection", () => { test("works with unix socket", async () => { if (process.platform === "win32") return; - const sockPath = join(tmpdir(), `bun-test-7471-${Date.now()}.sock`); + using dir = tempDir("bun-test-7471", {}); + const sockPath = join(String(dir), "test.sock"); await using proc = Bun.spawn({ cmd: [ From d76c5b986425660573f1ab71b401f11879d785a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Wed, 18 Mar 2026 01:13:53 -0300 Subject: [PATCH 4/7] fix: address second round of CodeRabbit review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Don't duplicate Content-Length when caller already set framing headers - Handle 1xx informational responses (100 Continue, 103 Early Hints) without treating them as terminal — emit "information" event and let the parser continue for the final response - Premature EOF now surfaces ConnResetException("aborted") instead of silently marking the response as complete - Create socket immediately on startFetch (don't wait for end()) so flushHeaders() and socket event timing work correctly Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 69 +++++++++++++++++++---- test/regression/issue/7471.test.ts | 89 ++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 10 deletions(-) diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index 5bb30a321b3..5cb62c195ac 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -314,7 +314,13 @@ function ClientRequest(input, options, cb) { body = bufs.length === 1 ? bufs[0] : Buffer.concat(bufs); } + // Check if caller already set framing headers + let hasContentLength = false; + let hasTransferEncoding = false; for (const key of Object.keys(headers)) { + const lk = key.toLowerCase(); + if (lk === "content-length") hasContentLength = true; + else if (lk === "transfer-encoding") hasTransferEncoding = true; const val = headers[key]; if (val === undefined) continue; if ($isJSArray(val)) { @@ -324,7 +330,10 @@ function ClientRequest(input, options, cb) { } } - if (body) head += `Content-Length: ${body.byteLength}\r\n`; + // Only add Content-Length if caller didn't set framing headers + if (body && !hasContentLength && !hasTransferEncoding) { + head += `Content-Length: ${body.byteLength}\r\n`; + } head += "\r\n"; socket.write(head); if (body) socket.write(body); @@ -370,6 +379,20 @@ function ClientRequest(input, options, cb) { parser._headers = []; } + // 1xx informational responses (100 Continue, 103 Early Hints, etc.) + // are not terminal — emit "information" and let the parser continue + // waiting for the final response. + if (statusCode >= 100 && statusCode < 200) { + this.emit("information", { + statusCode, + statusMessage, + httpVersion: `${vMaj}.${vMin}`, + headers: buildHeaders(headers), + rawHeaders: headers, + }); + return 1; // skip body, parser stays active for next response + } + const prevIsHTTPS = getIsNextIncomingMessageHTTPS(); setIsNextIncomingMessageHTTPS(protocol === "https:"); res = new IncomingMessage(null, {}); @@ -386,7 +409,7 @@ function ClientRequest(input, options, cb) { res.req = this; this[kClearTimeout](); - if (this.aborted) { maybeEmitClose(); return 2; } + if (this.aborted) { maybeEmitClose(); return 1; } if (!this.emit("response", res)) res._dump(); maybeEmitClose(); @@ -399,8 +422,12 @@ function ClientRequest(input, options, cb) { }; parser[HTTPParser.kOnMessageComplete] = () => { + // For 1xx informational responses, res is not set — don't free the parser, + // just let it continue parsing the next (final) response. + if (!res) return; + // Handle trailing headers — override the noop prototype getter/setter - if (parser._headers.length && res) { + if (parser._headers.length) { const trailers = buildHeaders(parser._headers); const rawTrailers = parser._headers.slice(); Object.defineProperty(res, "trailers", { value: trailers, writable: true, enumerable: true, configurable: true }); @@ -430,19 +457,42 @@ function ClientRequest(input, options, cb) { }); socket.on("end", () => { parser.finish(); - if (res && !res.complete) responseComplete(); + // If the response is still incomplete after parser.finish(), the connection + // was closed prematurely — surface an error instead of silently completing. + if (res && !res.complete) { + res.destroy(new ConnResetException("aborted")); + } }); socket.on("close", () => { - if (res && !res.complete) responseComplete(); + if (res && !res.complete) { + res.destroy(new ConnResetException("aborted")); + } socketCloseListener(); }); - // Wait for connection (TCP or TLS handshake) before writing, matching http2 pattern - if (socket.connecting || socket.secureConnecting) { + // Ensure socket is connected before writing + let connected = !socket.connecting && !socket.secureConnecting; + if (!connected) { const connectEvent = socket.secureConnecting ? "secureConnect" : "connect"; - socket.once(connectEvent, writeRequest); - } else { + socket.once(connectEvent, () => { + connected = true; + if (this.finished) writeRequest(); + }); + } + + // Write request when both connected and body is ready. + // If end() was already called (this.finished), write immediately. + // Otherwise, the send() function will trigger writing via resolveNextChunk. + if (this.finished && connected) { process.nextTick(writeRequest); + } else if (!this.finished) { + // Override resolveNextChunk so that when end() signals completion, + // we write the request to the socket. + const origResolve = resolveNextChunk; + resolveNextChunk = (end) => { + origResolve?.(end); + if (end && connected) writeRequest(); + }; } return true; @@ -456,7 +506,6 @@ function ClientRequest(input, options, cb) { // Socket-based path: when createConnection is provided, bypass the fetch // infrastructure and use raw HTTP/1.1 over the user-provided socket. if (typeof createConnection === "function") { - if (!this.finished) return false; // Wait until end() is called return startFetchViaSocket(); } diff --git a/test/regression/issue/7471.test.ts b/test/regression/issue/7471.test.ts index 061c945878a..c0d97db695d 100644 --- a/test/regression/issue/7471.test.ts +++ b/test/regression/issue/7471.test.ts @@ -419,6 +419,95 @@ describe("http.request createConnection", () => { expect(exitCode).toBe(0); }); + test("handles 100 Continue before final response", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + // Raw TCP server that sends 100 Continue then 200 + const server = net.createServer((sock) => { + sock.on("data", () => { + sock.write("HTTP/1.1 100 Continue\\r\\n\\r\\nHTTP/1.1 200 OK\\r\\nContent-Length: 4\\r\\n\\r\\ndone"); + sock.end(); + }); + }); + server.listen(0, () => { + let infoReceived = false; + const req = http.request({ + port: server.address().port, + method: "POST", + headers: { "Expect": "100-continue" }, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ data: d, status: res.statusCode, infoReceived })); + server.close(); + }); + }); + req.on("information", (info) => { + infoReceived = info.statusCode === 100; + }); + req.end("body"); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r).toEqual({ data: "done", status: 200, infoReceived: true }); + expect(exitCode).toBe(0); + }); + + test("does not duplicate Content-Length when caller sets it", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("http"); + const net = require("net"); + const server = http.createServer((req, res) => { + // Echo back the content-length header(s) the server received + const cl = req.headers["content-length"]; + res.end("cl:" + cl); + }); + server.listen(0, () => { + const body = "hello"; + const req = http.request({ + port: server.address().port, + method: "POST", + headers: { "Content-Length": Buffer.byteLength(body) }, + createConnection: (opts) => net.connect(opts), + }, (res) => { + let d = ""; + res.on("data", (c) => d += c); + res.on("end", () => { + console.log(JSON.stringify({ data: d })); + server.close(); + }); + }); + req.end(body); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const r = JSON.parse(stdout.trim()); + expect(r.data).toBe("cl:5"); + expect(exitCode).toBe(0); + }); + test("works without createConnection (no regression)", async () => { await using proc = Bun.spawn({ cmd: [ From 82355208f42bc3cd9764d5e41136418620fdf0d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Wed, 18 Mar 2026 01:35:01 -0300 Subject: [PATCH 5/7] fix: handle upgrade/CONNECT responses and avoid duplicate close events - 101 Switching Protocols now emits "upgrade" with the live socket instead of being caught by the 1xx informational branch - CONNECT 200 emits "connect" with the live socket for tunneling - Socket "close" handler no longer calls socketCloseListener() which would re-emit "close" on the real socket; inlines the close logic - EOF before headers now emits an error on the request Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index 5cb62c195ac..df259fb4c16 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -381,8 +381,9 @@ function ClientRequest(input, options, cb) { // 1xx informational responses (100 Continue, 103 Early Hints, etc.) // are not terminal — emit "information" and let the parser continue - // waiting for the final response. - if (statusCode >= 100 && statusCode < 200) { + // waiting for the final response. Note: 101 Switching Protocols is + // handled below as an upgrade, not here. + if (statusCode >= 100 && statusCode < 200 && statusCode !== 101) { this.emit("information", { statusCode, statusMessage, @@ -393,6 +394,20 @@ function ClientRequest(input, options, cb) { return 1; // skip body, parser stays active for next response } + // Upgrade (101 Switching Protocols) or CONNECT tunnel (200) — + // surface the live socket and stop HTTP parsing. + if (upgrade || (method === "CONNECT" && statusCode === 200)) { + const builtHeaders = buildHeaders(headers); + const head = Buffer.alloc(0); + if (upgrade) { + this.emit("upgrade", res || { statusCode, statusMessage, headers: builtHeaders, rawHeaders: headers }, socket, head); + } else { + this.emit("connect", res || { statusCode, statusMessage, headers: builtHeaders, rawHeaders: headers }, socket, head); + } + freeParser(parser, this, socket); + return 1; // skip body + } + const prevIsHTTPS = getIsNextIncomingMessageHTTPS(); setIsNextIncomingMessageHTTPS(protocol === "https:"); res = new IncomingMessage(null, {}); @@ -464,10 +479,21 @@ function ClientRequest(input, options, cb) { } }); socket.on("close", () => { + // Handle premature close if (res && !res.complete) { res.destroy(new ConnResetException("aborted")); + } else if (!res) { + // EOF before headers — emit error on the request + this.emit("error", new ConnResetException("aborted")); + } + // Mark the request as closed/destroyed without calling socketCloseListener(), + // which would re-emit "close" on this.socket (the real socket) causing duplicates. + this.destroyed = true; + if (!this._closed) { + this._closed = true; + callCloseCallback(this); + this.emit("close"); } - socketCloseListener(); }); // Ensure socket is connected before writing From 10217dca5c89968fd998fcebce42ad150a0bf2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Wed, 18 Mar 2026 01:53:14 -0300 Subject: [PATCH 6/7] fix: remove dead code, free parser on close, use instanceof for TLS - Remove dead `res ||` in upgrade/connect branch (res is always null) - Free parser on premature socket close to avoid resource leaks - Use `instanceof tls.TLSSocket` for deterministic TLS detection instead of ephemeral `secureConnecting` state, matching http2.ts Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index df259fb4c16..1f286fbeb94 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -59,6 +59,8 @@ const { const { HTTPParser, freeParser } = require("node:_http_common"); const { globalAgent } = require("node:_http_agent"); +const { getLazy } = require("internal/shared"); +const tls = getLazy(() => require("node:tls")); const { IncomingMessage } = require("node:_http_incoming"); const { OutgoingMessage } = require("node:_http_outgoing"); @@ -399,11 +401,8 @@ function ClientRequest(input, options, cb) { if (upgrade || (method === "CONNECT" && statusCode === 200)) { const builtHeaders = buildHeaders(headers); const head = Buffer.alloc(0); - if (upgrade) { - this.emit("upgrade", res || { statusCode, statusMessage, headers: builtHeaders, rawHeaders: headers }, socket, head); - } else { - this.emit("connect", res || { statusCode, statusMessage, headers: builtHeaders, rawHeaders: headers }, socket, head); - } + const infoRes = { statusCode, statusMessage, headers: builtHeaders, rawHeaders: headers }; + this.emit(upgrade ? "upgrade" : "connect", infoRes, socket, head); freeParser(parser, this, socket); return 1; // skip body } @@ -486,6 +485,8 @@ function ClientRequest(input, options, cb) { // EOF before headers — emit error on the request this.emit("error", new ConnResetException("aborted")); } + // Free parser resources on any close to avoid leaks + freeParser(parser, this, socket); // Mark the request as closed/destroyed without calling socketCloseListener(), // which would re-emit "close" on this.socket (the real socket) causing duplicates. this.destroyed = true; @@ -496,10 +497,12 @@ function ClientRequest(input, options, cb) { } }); - // Ensure socket is connected before writing - let connected = !socket.connecting && !socket.secureConnecting; + // Ensure socket is connected before writing. + // Use instanceof for deterministic TLS detection, matching http2.ts pattern. + const isTLSSocket = socket instanceof tls().TLSSocket; + let connected = !socket.connecting && !(isTLSSocket && socket.secureConnecting); if (!connected) { - const connectEvent = socket.secureConnecting ? "secureConnect" : "connect"; + const connectEvent = isTLSSocket ? "secureConnect" : "connect"; socket.once(connectEvent, () => { connected = true; if (this.finished) writeRequest(); From bf2e5a2cca5078b5be24c7fb8621e18ca53b7b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Fern=C3=A1ndez?= Date: Sun, 22 Mar 2026 23:15:21 -0300 Subject: [PATCH 7/7] fix: harden createConnection upgrade/connect handling Apply edge-case fixes found during review of #28397 (robobun): 1. buildHeaders() now joins duplicate headers with ", " (matching Node.js behavior) instead of silently overwriting. 2. Upgrade/connect events now emit a proper IncomingMessage instance instead of a plain JS object, matching the Node.js API contract. 3. Leftover bytes from the same TCP segment are now captured via chunk.slice(ret) after parser.execute() returns and passed as the head buffer, instead of always emitting Buffer.alloc(0). 4. Added `upgraded` flag so the close handler doesn't emit a spurious ConnResetException('aborted') after a normal upgrade close. 5. Added `parserFreed` guard to prevent double-free when both the upgrade branch and the close handler call freeParser(). Named data/end handlers are removed after upgrade to prevent use-after-free on the freed parser. Ref: oven-sh/bun#28397 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/js/node/_http_client.ts | 135 +++++++++++++++++++++++++++++------- 1 file changed, 109 insertions(+), 26 deletions(-) diff --git a/src/js/node/_http_client.ts b/src/js/node/_http_client.ts index 1f286fbeb94..8bf24dd67e7 100644 --- a/src/js/node/_http_client.ts +++ b/src/js/node/_http_client.ts @@ -350,6 +350,16 @@ function ClientRequest(input, options, cb) { parser.initialize(HTTPParser.RESPONSE, {}); let res: any = null; + let parserFreed = false; + let upgraded = false; + let pendingUpgrade: { res: any; event: string } | null = null; + + const safelyFreeParser = () => { + if (!parserFreed) { + parserFreed = true; + freeParser(parser, this, socket); + } + }; const responseComplete = () => { if (res && !res.complete) { @@ -359,23 +369,39 @@ function ClientRequest(input, options, cb) { this[kClearTimeout](); fetching = false; this[kFetchRequest] = null; - freeParser(parser, this, socket); + safelyFreeParser(); maybeEmitClose(); }; - // Build headers object from the flat [key, val, key, val, ...] array llhttp produces + // Build headers object from the flat [key, val, key, val, ...] array llhttp produces. + // Duplicates are joined with ", " (matching Node.js), except set-cookie which is an array. const buildHeaders = (rawHeaders: string[]) => { const headers: any = Object.create(null); for (let i = 0; i < rawHeaders.length; i += 2) { const lk = rawHeaders[i].toLowerCase(); const v = rawHeaders[i + 1]; - if (lk === "set-cookie") headers[lk] = headers[lk] ? [...headers[lk], v] : [v]; - else headers[lk] = v; + if (lk === "set-cookie") { + headers[lk] = headers[lk] ? [...headers[lk], v] : [v]; + } else if (headers[lk] !== undefined) { + headers[lk] += ", " + v; + } else { + headers[lk] = v; + } } return headers; }; - parser[HTTPParser.kOnHeadersComplete] = (vMaj, vMin, headers, _method, _url, statusCode, statusMessage, upgrade, shouldKeepAlive) => { + parser[HTTPParser.kOnHeadersComplete] = ( + vMaj, + vMin, + headers, + _method, + _url, + statusCode, + statusMessage, + upgrade, + shouldKeepAlive, + ) => { if (headers === undefined) { headers = parser._headers; parser._headers = []; @@ -398,12 +424,30 @@ function ClientRequest(input, options, cb) { // Upgrade (101 Switching Protocols) or CONNECT tunnel (200) — // surface the live socket and stop HTTP parsing. + // The emit is deferred: we store the upgrade info and return 2 (skip body). + // The "data" handler captures leftover bytes via chunk.slice(ret) after + // parser.execute() returns, so we don't lose pipelined data. if (upgrade || (method === "CONNECT" && statusCode === 200)) { + upgraded = true; const builtHeaders = buildHeaders(headers); - const head = Buffer.alloc(0); - const infoRes = { statusCode, statusMessage, headers: builtHeaders, rawHeaders: headers }; - this.emit(upgrade ? "upgrade" : "connect", infoRes, socket, head); - freeParser(parser, this, socket); + + const prevIsHTTPS = getIsNextIncomingMessageHTTPS(); + setIsNextIncomingMessageHTTPS(protocol === "https:"); + const upgradeRes = new IncomingMessage(null, {}); + setIsNextIncomingMessageHTTPS(prevIsHTTPS); + + upgradeRes[statusCodeSymbol] = statusCode; + upgradeRes[statusMessageSymbol] = statusMessage; + upgradeRes.httpVersion = `${vMaj}.${vMin}`; + upgradeRes.headers = builtHeaders; + upgradeRes.rawHeaders = headers; + upgradeRes.socket = socket; + + // Store for deferred emit in the "data" handler + pendingUpgrade = { res: upgradeRes, event: upgrade ? "upgrade" : "connect" }; + safelyFreeParser(); + socket.removeListener("data", onData); + socket.removeListener("end", onEnd); return 1; // skip body } @@ -423,7 +467,10 @@ function ClientRequest(input, options, cb) { res.req = this; this[kClearTimeout](); - if (this.aborted) { maybeEmitClose(); return 1; } + if (this.aborted) { + maybeEmitClose(); + return 1; + } if (!this.emit("response", res)) res._dump(); maybeEmitClose(); @@ -431,7 +478,7 @@ function ClientRequest(input, options, cb) { return method === "HEAD" ? 1 : 0; }; - parser[HTTPParser.kOnBody] = (chunk) => { + parser[HTTPParser.kOnBody] = chunk => { if (res && !res._dumped) res.push(chunk); }; @@ -444,49 +491,85 @@ function ClientRequest(input, options, cb) { if (parser._headers.length) { const trailers = buildHeaders(parser._headers); const rawTrailers = parser._headers.slice(); - Object.defineProperty(res, "trailers", { value: trailers, writable: true, enumerable: true, configurable: true }); - Object.defineProperty(res, "rawTrailers", { value: rawTrailers, writable: true, enumerable: true, configurable: true }); + Object.defineProperty(res, "trailers", { + value: trailers, + writable: true, + enumerable: true, + configurable: true, + }); + Object.defineProperty(res, "rawTrailers", { + value: rawTrailers, + writable: true, + enumerable: true, + configurable: true, + }); parser._headers = []; } responseComplete(); }; - parser[HTTPParser.kOnHeaders] = (headers) => { + parser[HTTPParser.kOnHeaders] = headers => { // Accumulate trailing headers (called when headers arrive in fragments or as trailers) parser._headers.push(...headers); }; - socket.on("data", (chunk) => { + // Named handlers so they can be removed after upgrade to prevent use-after-free. + const onData = chunk => { + if (parserFreed) { + // After upgrade, if there's a pending upgrade emit, capture leftover bytes + // from the same TCP segment and emit the upgrade event now. + if (pendingUpgrade) { + const { res: upgradeRes, event } = pendingUpgrade; + pendingUpgrade = null; + this.emit(event, upgradeRes, socket, chunk); + } + return; + } const ret = parser.execute(chunk); if (ret instanceof Error) { socket.destroy(); this.emit("error", ret); + return; } - }); - socket.on("error", (err) => { - if (isAbortError(err)) return; - try { this.emit("error", err); } catch (e) { - if (!!$debug) globalReportError(e); + // After parser.execute(), check if an upgrade was detected. If so, + // emit the upgrade event with any leftover bytes from this chunk. + if (pendingUpgrade) { + const { res: upgradeRes, event } = pendingUpgrade; + pendingUpgrade = null; + const head = typeof ret === "number" && ret < chunk.length ? chunk.slice(ret) : Buffer.alloc(0); + this.emit(event, upgradeRes, socket, head); } - }); - socket.on("end", () => { + }; + const onEnd = () => { + if (parserFreed) return; parser.finish(); // If the response is still incomplete after parser.finish(), the connection // was closed prematurely — surface an error instead of silently completing. if (res && !res.complete) { res.destroy(new ConnResetException("aborted")); } + }; + socket.on("data", onData); + socket.on("error", err => { + if (isAbortError(err)) return; + try { + this.emit("error", err); + } catch (e) { + if (!!$debug) globalReportError(e); + } }); + socket.on("end", onEnd); socket.on("close", () => { - // Handle premature close + // Handle premature close — but not after a successful upgrade, + // where res is null and the socket close is expected. if (res && !res.complete) { res.destroy(new ConnResetException("aborted")); - } else if (!res) { + } else if (!res && !upgraded) { // EOF before headers — emit error on the request this.emit("error", new ConnResetException("aborted")); } // Free parser resources on any close to avoid leaks - freeParser(parser, this, socket); + safelyFreeParser(); // Mark the request as closed/destroyed without calling socketCloseListener(), // which would re-emit "close" on this.socket (the real socket) causing duplicates. this.destroyed = true; @@ -518,7 +601,7 @@ function ClientRequest(input, options, cb) { // Override resolveNextChunk so that when end() signals completion, // we write the request to the socket. const origResolve = resolveNextChunk; - resolveNextChunk = (end) => { + resolveNextChunk = end => { origResolve?.(end); if (end && connected) writeRequest(); };