diff --git a/src/js/node/_http2_upgrade.ts b/src/js/node/_http2_upgrade.ts index 354267a9f34d..df3ed0112918 100644 --- a/src/js/node/_http2_upgrade.ts +++ b/src/js/node/_http2_upgrade.ts @@ -101,13 +101,22 @@ function tlsSocketWrite(this: TLSProxySocket, chunk: Buffer, encoding: string, c } // _destroy: called when the stream is destroyed (e.g. tlsSocket.destroy(err)). -// Cleans up the native TLS handle. +// Cleans up the native TLS handle and takes the raw socket down with it. // Mirrors net.ts Socket.prototype._destroy. function tlsSocketDestroy(this: TLSProxySocket, err: Error | null, callback: (err?: Error | null) => void) { - const h = this._ctx.nativeHandle; + const ctx = this._ctx; + const h = ctx.nativeHandle; if (h) { h.close(); - this._ctx.nativeHandle = null; + ctx.nativeHandle = null; + } + // Closing the native handle only end()s the raw socket (close_notify + FIN), which keeps it + // open, and counted by the net.Server that accepted it, until the peer closes as well. Node's + // TLSWrap destroys the wrapped socket whenever the TLS socket goes down, so a failed handshake + // or a torn-down session releases the connection even if the peer never closes its side. + const rawSocket = ctx.rawSocket; + if (!rawSocket.destroyed) { + rawSocket.destroy(); } // Must invoke pending write callback with error per Writable stream contract const writeCb = this._writeCallback; diff --git a/test/js/node/http2/node-http2-upgrade.test.mts b/test/js/node/http2/node-http2-upgrade.test.mts index 57a8b73d93bc..15a7445684aa 100644 --- a/test/js/node/http2/node-http2-upgrade.test.mts +++ b/test/js/node/http2/node-http2-upgrade.test.mts @@ -16,6 +16,7 @@ import fs from "node:fs"; import http2 from "node:http2"; import net from "node:net"; import path from "node:path"; +import { Duplex } from "node:stream"; import { afterEach, describe, test } from "node:test"; import tls from "node:tls"; import { fileURLToPath } from "node:url"; @@ -461,6 +462,180 @@ describe("HTTP/2 upgrade — server TLS options", () => { }); }); +// The net.Server keeps counting a connection it accepted until the socket it handed to +// h2Server.emit("connection") is destroyed, and netServer.close(cb) only calls back once that count +// is zero. Node destroys that socket together with the TLSSocket it wrapped it in, so the connection +// is released no matter how the server side went down. Every case below tears the connection down +// from the server side while the peer deliberately keeps its end of the TCP connection open, so the +// accepted socket only gets released if the upgrade path releases it. +describe("HTTP/2 upgrade — the accepted socket is released when the server side goes down", () => { + type Accepted = { raw: net.Socket; closed: Promise }; + + async function acceptInto(h2Server: http2.Http2SecureServer) { + const accepted = Promise.withResolvers(); + const netServer = net.createServer(raw => { + const closed = new Promise(resolve => raw.once("close", hadError => resolve(hadError))); + accepted.resolve({ raw, closed }); + h2Server.emit("connection", raw); + }); + const port = await new Promise(resolve => { + netServer.listen(0, "127.0.0.1", () => resolve((netServer.address() as net.AddressInfo).port)); + }); + return { netServer, port, accepted: accepted.promise }; + } + + // A TCP connection the peer never ends or destroys itself: the server's FIN only produces 'end' + // here (allowHalfOpen), so the server's accepted socket stays open until the server destroys it. + function connectHeldOpen(port: number) { + const tcp = net.connect({ port, host: "127.0.0.1", allowHalfOpen: true }); + // Writes into a connection the server has already closed are expected to fail. + tcp.on("error", () => {}); + return tcp; + } + + // A TLS client riding on a held-open TCP connection through an in-memory carrier: whatever the + // client's TLS layer does when the server closes the TLS session (end, destroy) stays on the + // carrier and never closes the TCP connection underneath. + function connectTlsHeldOpen(port: number, options: tls.ConnectionOptions) { + const tcp = connectHeldOpen(port); + const carrier = new Duplex({ + read() {}, + write(chunk, _encoding, callback) { + // The client's reply to the server's close_notify may be written after the server (or the + // test's cleanup) has closed the connection; there is nobody left to deliver it to. + if (tcp.destroyed) return callback(); + tcp.write(chunk, () => callback()); + }, + final(callback) { + callback(); + }, + }); + tcp.on("data", chunk => carrier.push(chunk)); + tcp.on("end", () => carrier.push(null)); + const client = tls.connect({ socket: carrier, rejectUnauthorized: false, ...options }); + client.on("error", () => {}); + client.resume(); + return { tcp, client }; + } + + // With the peer holding its side open, the accepted socket's 'close' can only come from the + // server destroying it; a server that merely end()s it leaves this waiting until the test times out. + async function assertReleased(netServer: net.Server, accepted: Promise) { + const { raw, closed } = await accepted; + // Torn down without an error, like the socket underneath a node TLSSocket. + assert.strictEqual(await closed, false); + assert.strictEqual(raw.destroyed, true); + const connections = await new Promise((resolve, reject) => { + netServer.getConnections((err, count) => (err ? reject(err) : resolve(count))); + }); + assert.strictEqual(connections, 0); + await new Promise(resolve => netServer.close(() => resolve())); + } + + function cleanup(netServer: net.Server, tcp: net.Socket) { + tcp.destroy(); + if (netServer.listening) netServer.close(); + } + + for (const handleClientError of [false, true]) { + const variant = handleClientError ? "with a clientError listener" : "without a clientError listener"; + test(`after a failed handshake (${variant})`, async () => { + const h2Server = http2.createSecureServer(TLS); + const reported: string[] = []; + if (handleClientError) h2Server.on("clientError", () => reported.push("clientError")); + const tlsClientError = once(h2Server, "tlsClientError").then(() => reported.push("tlsClientError")); + const { netServer, port, accepted } = await acceptInto(h2Server); + + const tcp = connectHeldOpen(port); + tcp.on("connect", () => tcp.write("GET / HTTP/1.1\r\nHost: example\r\n\r\n")); + tcp.resume(); + try { + await tlsClientError; + assert.deepStrictEqual(reported, handleClientError ? ["clientError", "tlsClientError"] : ["tlsClientError"]); + await assertReleased(netServer, accepted); + } finally { + cleanup(netServer, tcp); + } + }); + } + + for (const withError of [true, false]) { + test(`after the session is destroyed ${withError ? "with" : "without"} an error`, async () => { + const h2Server = http2.createSecureServer(TLS); + const sessionClosed = new Promise(resolve => { + h2Server.once("session", (session: http2.ServerHttp2Session) => { + session.on("error", () => {}); + session.once("close", resolve); + if (withError) session.destroy(new Error("torn down by the test")); + else session.destroy(); + }); + }); + const { netServer, port, accepted } = await acceptInto(h2Server); + + const { tcp } = connectTlsHeldOpen(port, { ALPNProtocols: ["h2"] }); + try { + await sessionClosed; + await assertReleased(netServer, accepted); + } finally { + cleanup(netServer, tcp); + } + }); + } + + test("after the client certificate is rejected", async () => { + const h2Server = http2.createSecureServer({ ...TLS, requestCert: true, rejectUnauthorized: true }); + h2Server.on("session", () => assert.fail("a rejected client must not get a session")); + const tlsClientError = once(h2Server, "tlsClientError"); + const { netServer, port, accepted } = await acceptInto(h2Server); + + // The client presents a certificate the server has no CA for. + const { tcp } = connectTlsHeldOpen(port, { ALPNProtocols: ["h2"], key: TLS.key, cert: TLS.cert }); + try { + const [err] = await tlsClientError; + assert.ok(err instanceof Error); + await assertReleased(netServer, accepted); + } finally { + cleanup(netServer, tcp); + } + }); + + test("after a client that negotiated no protocol is turned away", async () => { + // Nothing handles 'unknownProtocol', so the server answers with a 403 and destroys the + // connection once unknownProtocolTimeout has passed. + const h2Server = http2.createSecureServer({ ...TLS, unknownProtocolTimeout: 0 }); + h2Server.on("session", () => assert.fail("a client without ALPN must not get a session")); + const { netServer, port, accepted } = await acceptInto(h2Server); + + const { tcp } = connectTlsHeldOpen(port, {}); + try { + await assertReleased(netServer, accepted); + } finally { + cleanup(netServer, tcp); + } + }); + + test("but not while the session is alive", async () => { + const h2Server = http2.createSecureServer(TLS); + let session: http2.ServerHttp2Session | undefined; + const sessionStarted = once(h2Server, "session").then(([s]) => { + session = s; + }); + const { netServer, port, accepted } = await acceptInto(h2Server); + + const { tcp, client } = connectTlsHeldOpen(port, { ALPNProtocols: ["h2"] }); + try { + await Promise.all([sessionStarted, once(client, "secureConnect")]); + const { raw } = await accepted; + assert.strictEqual(raw.destroyed, false); + const connections = await new Promise(resolve => netServer.getConnections((_, count) => resolve(count))); + assert.strictEqual(connections, 1); + } finally { + session?.destroy(); + cleanup(netServer, tcp); + } + }); +}); + if (typeof Bun !== "undefined") { describe("Node.js compatibility", () => { test("tests should run on node.js", async () => {