Skip to content
24 changes: 19 additions & 5 deletions src/js/node/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -881,10 +881,17 @@ const ServerHandlers: SocketHandler<NetSocket> = {
// after secureConnection event we emmit secure and secureConnect
self.emit("secure", self);
self.emit("secureConnect", verifyError);
if (server?.pauseOnConnect) {
self.pause();
} else {
self.resume();
// readableFlowing === null: no 'connection' / 'secureConnection' handler
// touched the stream state, so apply the server's default. A handler that
// paused (false) or attached 'data'/'readable' (true/false) is honored -
// the post-emit resume() here must not stomp a pause() made inside either
// handler.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (self.readableFlowing === null) {
if (server?.pauseOnConnect) {
self.pause();
} else {
self.resume();
}
}
},
error(socket, error) {
Expand Down Expand Up @@ -2110,7 +2117,14 @@ Socket.prototype.resume = function resume() {
};

Socket.prototype.pause = function pause() {
if (!this.destroyed) {
// While the TLS handshake is in flight the native handle must keep reading:
// stopping the poll here would starve the TLS engine of the ClientHello /
// server Finished and wedge the handshake forever. In Node, TLSWrap reads
// the underlying socket independently of the TLSSocket's stream state, so a
// pre-handshake pause() only affects delivery of *decrypted* bytes. Mirror
// that: pause only the Duplex; the data handler applies native backpressure
// at highWaterMark once application data starts arriving.
if (!this.destroyed && !this.secureConnecting) {
this._handle?.pause?.();
// libuv only counts a stream handle as active - and therefore as keeping
// the event loop alive - while it is reading. A paused socket lets the
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
'use strict';
const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

// Test that `tls.Server` constructor options are passed to the parent
// constructor.

const assert = require('assert');
const fixtures = require('../common/fixtures');
const tls = require('tls');

const options = {
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem'),
};

{
const server = tls.createServer(options, common.mustCall((socket) => {
assert.strictEqual(socket.allowHalfOpen, false);
assert.strictEqual(socket.isPaused(), false);
}));

assert.strictEqual(server.allowHalfOpen, false);
assert.strictEqual(server.pauseOnConnect, false);

server.listen(0, common.mustCall(() => {
const socket = tls.connect({
port: server.address().port,
rejectUnauthorized: false
}, common.mustCall(() => {
socket.end();
}));

socket.on('close', () => {
server.close();
});
}));
}

{
const server = tls.createServer({
allowHalfOpen: true,
pauseOnConnect: true,
...options
}, common.mustCall((socket) => {
assert.strictEqual(socket.allowHalfOpen, true);
assert.strictEqual(socket.isPaused(), true);
socket.on('end', socket.end);
}));

assert.strictEqual(server.allowHalfOpen, true);
assert.strictEqual(server.pauseOnConnect, true);

server.listen(0, common.mustCall(() => {
const socket = tls.connect({
port: server.address().port,
rejectUnauthorized: false
}, common.mustCall(() => {
socket.end();
}));

socket.on('close', () => {
server.close();
});
}));
}
133 changes: 133 additions & 0 deletions test/js/node/tls/node-tls-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1303,3 +1303,136 @@
}
await once(server, "close");
});

describe("pausing a TLS socket before the handshake does not stall it", () => {
// Before the fix Socket.prototype.pause() stopped native reads unconditionally,
// so a pre-handshake pause() starved the TLS engine of the ClientHello and the
// handshake never completed. Node's TLSWrap keeps reading the underlying fd
// regardless of the TLSSocket's stream state; these tests match that.

it("server: s.pause() inside the 'connection' handler", async () => {
const server: Server = createServer(COMMON_CERT);
let connSock: TLSSocket | undefined;
server.on("connection", s => {
connSock = s as TLSSocket;
s.pause();
});
const accepted = Promise.withResolvers<TLSSocket>();
server.on("secureConnection", s => accepted.resolve(s));
server.on("tlsClientError", accepted.reject);
server.listen(0, "127.0.0.1");
await once(server, "listening");
let cli: TLSSocket | undefined;
let srv: TLSSocket | undefined;
try {
const port = (server.address() as AddressInfo).port;
cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false });
cli.on("error", () => {});
// Before the fix this hung: the native poll was switched to write-only
// and the TLS engine never saw the ClientHello.
await once(cli, "secureConnect");
srv = await accepted.promise;
expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false });

let stopped = true;
let got = "";
srv.on("data", d => {
if (stopped) throw new Error("data event fired while paused");
got += d;
});
cli.write("hello");
// Round-trip the other way so we know the client's write has traversed
// the event loop on the server side while still paused.
srv.write("ack");
expect((await once(cli, "data"))[0].toString()).toBe("ack");
expect({ got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({
got: "",
flowing: false,
readableLength: 5,
});

Check failure on line 1352 in test/js/node/tls/node-tls-server.test.ts

View check run for this annotation

Claude / Claude Code Review

readableLength assertion races with independent socket read event

The reverse-direction round-trip (`srv.write("ack")` → `await once(cli, "data")`) does not causally establish that `cli.write("hello")` has been dispatched to `srv`'s data handler, so the `readableLength: 5` assertion can run with `srv.readableLength === 0` on platforms that don't order ready-fd dispatch (kqueue/macOS, IOCP/Windows). Poll `srv.readableLength` with a bounded deadline (or await a promise resolved from an overridden `srv.push`) instead of using an unrelated round-trip as the barrie
Comment thread
robobun marked this conversation as resolved.
Outdated
stopped = false;
const dataP = once(srv, "data");
srv.resume();
await dataP;
expect(got).toBe("hello");
} finally {
cli?.destroy();
srv?.destroy();
connSock?.destroy();
server.close();
}
await once(server, "close");
});

it("server: pauseOnConnect: true", async () => {
const server: Server = createServer({ ...COMMON_CERT, pauseOnConnect: true });
const accepted = Promise.withResolvers<{ paused: boolean; flowing: boolean | null; socket: TLSSocket }>();
server.on("secureConnection", s =>
accepted.resolve({ paused: s.isPaused(), flowing: s.readableFlowing, socket: s }),
);
server.on("tlsClientError", accepted.reject);
server.listen(0, "127.0.0.1");
await once(server, "listening");
let cli: TLSSocket | undefined;
let srv: TLSSocket | undefined;
try {
const port = (server.address() as AddressInfo).port;
cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false });
cli.on("error", () => {});
await once(cli, "secureConnect");
const { paused, flowing, socket } = await accepted.promise;
srv = socket;
expect({ paused, flowing }).toEqual({ paused: true, flowing: false });

cli.write("hello");
srv.write("ack");
expect((await once(cli, "data"))[0].toString()).toBe("ack");
expect({ flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({
flowing: false,
readableLength: 5,
});
srv.resume();
expect((await once(srv, "data"))[0].toString()).toBe("hello");
} finally {
cli?.destroy();
srv?.destroy();
server.close();
}
await once(server, "close");
});

it("client: pauseOnConnect: true", async () => {
const server: Server = createServer(COMMON_CERT);
const accepted = Promise.withResolvers<TLSSocket>();
server.on("secureConnection", s => accepted.resolve(s));
server.on("tlsClientError", accepted.reject);
server.listen(0, "127.0.0.1");
await once(server, "listening");
let cli: TLSSocket | undefined;
let srv: TLSSocket | undefined;
try {
const port = (server.address() as AddressInfo).port;
cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, pauseOnConnect: true });
cli.on("error", () => {});
// Before the fix SocketHandlers2.open called self.pause() -> native
// pause and the handshake never completed.
await once(cli, "secureConnect");
srv = await accepted.promise;
// Sanity: data flows both ways. The client side is resumed explicitly
// because Node leaves the TLSSocket at readableFlowing === null here
// (pauseOnConnect applies to the underlying net.Socket, which is a
// separate object in Node) while Bun preserves the pause() on the same
// TLSSocket instance; resuming makes the observable match either way.
cli.resume();
srv.write("from-server");
expect((await once(cli, "data"))[0].toString()).toBe("from-server");
cli.write("from-client");
expect((await once(srv, "data"))[0].toString()).toBe("from-client");
} finally {
cli?.destroy();
srv?.destroy();
server.close();
}
await once(server, "close");
});
});
Loading