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
11 changes: 11 additions & 0 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,10 @@ Server.prototype.listen = function () {
port = arg0.port;
host = arg0.host;
socketPath = arg0.path;
// Node normalizes an explicit `port: undefined` or `port: null` to 0.
if ((port === undefined && "port" in arg0) || port === null) {
port = 0;
}

const otherTLS = arg0.tls;
if (otherTLS && $isObject(otherTLS)) {
Expand Down Expand Up @@ -578,6 +582,13 @@ Server.prototype.listen = function () {
}
}

// Node precedence: a port wins over `path`, and a `path` listen ignores `host`.
if ((typeof port === "number" || typeof port === "string") && socketPath) {
socketPath = undefined;
} else if (socketPath) {
host = undefined;
}
Comment on lines +586 to +590

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) When path wins the precedence check, port is not cleared, so a truthy non-number/non-string port (e.g. true, [8080], new Number(80)) is still forwarded to Bun.serve alongside unix, where the new ServerConfig guard now throws — listen({ port: true, path }) bound the socket on base but emits 'error' after this PR, and Node binds the pipe. Fix: set port = undefined in the else if (socketPath) branch so the pipe path never forwards a port, matching src/js/node/net.ts:3745.

Extended reasoning...

listen({ port: true, path: sock }): line 548 sets port = true; line 552 skips (not undefined/null); line 578 skips (not string); line 586 typeof true is "boolean" so the first branch is false; line 588 takes else if (socketPath) and clears host but leaves port = true and socketPath = sock. kRealListen then calls Bun.serve({ port: true, unix: sock, ... }). In ServerConfig::from_js, get_truthy("port") returns Some(true), to_number(true) is 1, has_port = true (line 1087); the unix block then hits the new has_port guard and throws "Cannot specify both port and unix" (line 1125-1129). The throw is caught at line 627 and routed to emitListenErrorNextTick, so the server emits 'error' instead of 'listening'. On the base branch the same inputs reached Bun.serve, port was parsed into args.address and then silently overwritten by unix, so the socket bound. Node's net.Server.listen only takes the TCP branch when typeof options.port is 'number' or 'string'; any other port value falls through to the pipe branch which never reads port, so Node…

Verification: nit — real regression this PR introduces, but the triggering inputs (port: true, [8080], new Number(80)) are exotic; unlikely a real caller hits it. Mechanism traced and verified: src/js/node/_http_server.ts with listen({ port: true, path: sock }): - L548 port = arg0.port → true - L552 (port === undefined && "port" in arg0) || port === null → false, skipped - L574 `port ===…


const lastArg = arguments[argc - 1];
if ($isCallable(lastArg)) {
// Before the bind, as in node, so a listen() retried from the 'error' handler still calls it.
Expand Down
7 changes: 7 additions & 0 deletions src/runtime/server/ServerConfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,7 @@ impl ServerConfig {
..ServerConfig::default()
};
let mut has_hostname = false;
let mut has_port = false;

if env.get(b"NODE_ENV").unwrap_or(b"") == b"production" {
args.development = DevelopmentOption::Production;
Expand Down Expand Up @@ -1083,6 +1084,7 @@ impl ServerConfig {
*tp = p;
}
port = p;
has_port = p != 0;
}

if let Some(base_uri) = arg.get_truthy(global, "baseURI")? {
Expand Down Expand Up @@ -1120,6 +1122,11 @@ impl ServerConfig {
"Cannot specify both hostname and unix",
)));
}
if has_port {
return Err(global.throw_invalid_arguments(format_args!(
"Cannot specify both port and unix",
)));
}

args.address = Address::Unix(bun_core::ZBox::from_bytes(unix_str.slice()));
}
Expand Down
29 changes: 28 additions & 1 deletion test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { file, gc, Serve, serve, Server } from "bun";
import { afterAll, afterEach, describe, expect, it, mock } from "bun:test";
import { readFileSync, writeFileSync } from "fs";
import { existsSync, readFileSync, writeFileSync } from "fs";
import {
bunEnv,
bunExe,
Expand Down Expand Up @@ -2493,6 +2493,33 @@ it("unix socket connection throws an error on a bad domain without crashing", as
}).toThrow();
});

it("Bun.serve throws when both port and unix are given", async () => {
using dir = tempDir("port-and-unix", {});
const unix = join(String(dir), "port-and-unix.sock");
expect(() => {
using server = Bun.serve({
port: 1,
unix,
fetch() {
return new Response("hey");
},
});
}).toThrow("Cannot specify both port and unix");
expect(existsSync(unix)).toBe(false);

// port 0 asks for no specific port, so it does not conflict with unix.
using server = Bun.serve({
port: 0,
unix,
fetch() {
return new Response("hey");
},
});
expect(server.port).toBeUndefined();
const res = await fetch("http://localhost/", { unix });
expect(await res.text()).toBe("hey");
});

it("#5859 text", async () => {
using server = Bun.serve({
port: 0,
Expand Down
29 changes: 29 additions & 0 deletions test/js/node/http/node-http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1338,6 +1338,35 @@ describe("node:http", () => {
await promise;
});

// Node binds the TCP port and ignores `path` whenever the options carry a
// `port` key, including `port: undefined`, `port: null` and a string port.
test.each([0, "0", undefined, null])("listen({ port: %p, path }) binds the port and ignores the path", async port => {
const socketPath = `${tmpdir()}/bun-server-${Math.random().toString(32)}.sock`;
await using server = createServer((req, res) => {
res.end("ok");
});
server.listen({ port, path: socketPath });
await once(server, "listening");
const address = server.address() as AddressInfo;
expect(typeof address).toBe("object");
expect(address.port).toBeGreaterThan(0);
expect(nodefs.existsSync(socketPath)).toBe(false);
const response = await fetch(`http://127.0.0.1:${address.port}/`);
expect(await response.text()).toBe("ok");
});

test("listen({ path, host }) binds the path and ignores the host, as in node", async () => {
const socketPath = `${tmpdir()}/bun-server-${Math.random().toString(32)}.sock`;
await using server = createServer((req, res) => {
res.end("ok");
});
server.listen({ path: socketPath, host: "localhost" });
await once(server, "listening");
expect(server.address()).toBe(socketPath);
const response = await fetch("http://localhost/", { unix: socketPath });
expect(await response.text()).toBe("ok");
});

test("should not decompress gzip, issue#4397", async () => {
using server = Bun.serve({
port: 0,
Expand Down
Loading