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
66 changes: 33 additions & 33 deletions src/js/node/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4084,26 +4084,6 @@ Server.prototype[kRealListen] = function (
data: this,
pauseOnConnect: this.pauseOnConnect,
});
// Mirror libuv uv_pipe_chmod: readableAll/writableAll relax the unix socket
// file's group/other permission bits. Skipped on Windows and abstract
// sockets (no filesystem entry). uSockets binds synchronously, so the file
// exists by the time Bun.listen returns.
// https://github.com/nodejs/node/blob/614050b657e9757c1097aa85f92f2cb51149dc0d/lib/net.js#L1899
if ((readableAll || writableAll) && process.platform !== "win32" && path.charCodeAt(0) !== 0) {
let desired = 0;
if (readableAll) desired |= 0o44; // S_IRGRP | S_IROTH
if (writableAll) desired |= 0o22; // S_IWGRP | S_IWOTH
try {
const fs = require("node:fs");
const cur = fs.statSync(path).mode;
if ((cur & desired) !== desired) fs.chmodSync(path, cur | desired);
} catch (e) {
// _handle is a Bun.listen SocketListener: it exposes stop(), not close().
this._handle?.stop?.(true);
this._handle = null;
throw e;
}
}
} else if (fd != null) {
this._handle = Bun.listen({
fd,
Expand All @@ -4117,6 +4097,9 @@ Server.prototype[kRealListen] = function (
data: this,
pauseOnConnect: this.pauseOnConnect,
});
// The native listener owns the fd now, so the cluster handle must not close it.
const clusterHandle = this[kClusterHandle];
if (clusterHandle != null && clusterHandle.sharedFd === fd) clusterHandle.adopted = true;
} else {
this._handle = Bun.listen({
port,
Expand All @@ -4132,21 +4115,38 @@ Server.prototype[kRealListen] = function (
});
}

this._handle[owner_symbol] = this;
this._handle.onconnection = onconnection;
try {
// uv_pipe_chmod: https://github.com/nodejs/node/blob/614050b657e9757c1097aa85f92f2cb51149dc0d/lib/net.js#L1899
if (path && (readableAll || writableAll) && process.platform !== "win32" && path.charCodeAt(0) !== 0) {
let desired = 0;
if (readableAll) desired |= 0o44; // S_IRGRP | S_IROTH
if (writableAll) desired |= 0o22; // S_IWGRP | S_IWOTH
const fs = require("node:fs");
const cur = fs.statSync(path).mode;
if ((cur & desired) !== desired) fs.chmodSync(path, cur | desired);
}

const addr = this.address();
if (addr && typeof addr === "object") {
const familyLast = String(addr.family).slice(-1);
this._connectionKey = `${familyLast}:${addr.address}:${port}`;
}
this._handle[owner_symbol] = this;
this._handle.onconnection = onconnection;

if (contexts) {
for (const [name, context] of contexts) {
// tls.ts stores the InternalSecureContext wrapper; the native side wants
// the native SSL_CTX wrapper at `.context`.
addServerName(this._handle, name, context.context ?? context);
const addr = this.address();
if (addr && typeof addr === "object") {
const familyLast = String(addr.family).slice(-1);
this._connectionKey = `${familyLast}:${addr.address}:${port}`;
}

if (contexts) {
for (const [name, context] of contexts) {
// tls.ts stores the InternalSecureContext wrapper; the native side wants
// the native SSL_CTX wrapper at `.context`.
Comment thread
robobun marked this conversation as resolved.
addServerName(this._handle, name, context.context ?? context);
}
}
} catch (e) {
// _handle is a Bun.listen SocketListener: it exposes stop(), not close().
this._handle.stop(true);
this._handle = null;
throw e;
}

// Unref the handle if the server was unref'ed prior to listening
Expand Down Expand Up @@ -4336,8 +4336,8 @@ function listenInCluster(
onListen,
sharedFd,
);
handle.adopted = true;
} catch (err) {
// With `handle.adopted` set by kRealListen, close() releases the key but not the fd.
server[kClusterHandle] = null;
server[kClusterUnixPath] = undefined;
handle[kClusterOwner] = null;
Expand Down
63 changes: 63 additions & 0 deletions test/js/node/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,69 @@ if (cluster.isPrimary) {
expect(stdout).toContain("reply: echo:hi");
}, 30_000);

test("TLS cluster worker state matches the event that ends listen(): 'listening' or 'error'", async () => {
const dir = tempDirWithFiles("bun-test", {
"cert.pem": tlsCerts.cert,
"key.pem": tlsCerts.key,
"main.ts": `
const cluster = require("node:cluster");
const tls = require("node:tls");
const fs = require("node:fs");
const path = require("node:path");
const key = fs.readFileSync(path.join(__dirname, "key.pem"));
const cert = fs.readFileSync(path.join(__dirname, "cert.pem"));

if (cluster.isPrimary) {
const worker = cluster.fork();
worker.on("message", msg => console.log("state:", JSON.stringify(msg)));
cluster.on("listening", (w, address) => {
const c = tls.connect({ port: address.port, host: "127.0.0.1", rejectUnauthorized: false });
c.setEncoding("utf8");
c.on("data", d => {
console.log("reply:", d);
c.end();
worker.kill();
process.exit(0);
});
c.on("error", e => {
console.log("client error:", e.code);
process.exit(1);
});
});
} else {
const onConnection = socket => socket.end("ok");
const first = tls.createServer({ key, cert }, onConnection);
// Bun rejects the second name when it loads the entries into the listener
// that adopted the shared fd (#43092). Node accepts both.
const name = "a.b.c.d.e.f.g.h.i.j.k.example";
first.addContext(name, { key, cert });
first.addContext(name + ".", { key, cert });
const report = outcome =>
process.send({
outcome,
listening: first.listening,
hasAddress: first.address() !== null,
hasHandle: first._handle != null,
});
first.on("listening", () => report("listening"));
first.on("error", () => {
report("error");
// The worker's cluster state is still usable after the failure.
tls.createServer({ key, cert }, onConnection).listen(0);
});
first.listen(0);
}
`,
});
const { stdout } = await bunRun(joinP(dir, "main.ts"), bunEnv);
const state = stdout.match(/^state: (.*)$/m)?.[1];
expect([
'{"outcome":"listening","listening":true,"hasAddress":true,"hasHandle":true}',
'{"outcome":"error","listening":false,"hasAddress":false,"hasHandle":false}',
]).toContain(state);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(stdout).toContain("reply: ok");
}, 30_000);

test("plain worker listening on a key already owned by a TLS shared-only handle fails with EINVAL", async () => {
const dir = tempDirWithFiles("bun-test", {
"cert.pem": tlsCerts.cert,
Expand Down
32 changes: 32 additions & 0 deletions test/js/node/tls/node-tls-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,38 @@ it("an asynchronous SNICallback resolving cb(null, null) still honors addContext
await once(server, "close");
});

it("the server state matches the event that ends listen(): 'listening' or 'error'", async () => {
// Bun loads the addContext() entries into the native listener after the
// bind, and rejects the second of these two names there (#43092). Node
// accepts both and emits 'listening'. After either event the server state
// has to match it: a failed listen() leaves the server closed, as in Node.
const altCert = { key: rawKey, cert: cert };
const server: Server = createServer(COMMON_CERT, socket => socket.end());
const name = "a.b.c.d.e.f.g.h.i.j.k.example";
server.addContext(name, altCert);
server.addContext(name + ".", altCert);
Comment thread
robobun marked this conversation as resolved.
server.listen(0, "127.0.0.1");
const outcome = await new Promise<string>(resolve => {
server.once("listening", () => resolve("listening"));
server.once("error", () => resolve("error"));
});
const state = {
outcome,
listening: server.listening,
hasAddress: server.address() !== null,
hasHandle: (server as any)._handle != null,
};
if (outcome === "listening") {
expect(state).toEqual({ outcome: "listening", listening: true, hasAddress: true, hasHandle: true });
server.close();
await once(server, "close");
} else {
expect(state).toEqual({ outcome: "error", listening: false, hasAddress: false, hasHandle: false });
const closeErr = await new Promise<any>(resolve => server.close(resolve));
expect(closeErr.code).toBe("ERR_SERVER_NOT_RUNNING");
}
});

describe("tls.Server socket destroySoon", () => {
// destroySoon() after end(big) must deliver every byte even when the TLS write
// batcher's final flush spills (#31584). The spill/kernel-buffer race hits ~4% of
Expand Down
Loading