diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 904facfe16b6..b0a0d7f36b22 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -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, @@ -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, @@ -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`. + 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 @@ -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; diff --git a/test/js/node/cluster.test.ts b/test/js/node/cluster.test.ts index 3248646bd3b0..e08180b02327 100644 --- a/test/js/node/cluster.test.ts +++ b/test/js/node/cluster.test.ts @@ -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); + 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, diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index a672381930ae..87b6314b80c4 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -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); + server.listen(0, "127.0.0.1"); + const outcome = await new Promise(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(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