Skip to content
Merged
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
6 changes: 6 additions & 0 deletions src/runtime/dns_jsc/cares_jsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,12 @@ pub(crate) fn error_to_js_with_syscall_and_hostname(
Ok(instance)
}

/// Thrown before uSockets' synchronous `getaddrinfo` can block on a name that can never resolve.
pub(crate) fn not_a_hostname_error(global_this: &JSGlobalObject, hostname: &[u8]) -> JSValue {
system_error_with_syscall_and_hostname(c_ares::Error::ENOTFOUND, b"getaddrinfo", hostname)
.to_error_instance(global_this)
}

// ── canonicalizeIP host fn ─────────────────────────────────────────────────
// `#[bun_jsc::host_fn(export = ...)]` emits the C-ABI shim under that link name.
#[bun_jsc::host_fn(export = "Bun__canonicalizeIP")]
Expand Down
40 changes: 33 additions & 7 deletions src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2815,6 +2815,22 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
// not `*this`.
let global = this_ref.global_this();

if let server_config::Address::Tcp {
hostname: Some(hostname),
..
} = &this_ref.config.address
{
let hostname = hostname.as_bytes();
if !bun_dns::is_valid_hostname(strip_ipv6_brackets(hostname)) {
let _ = global.throw_value(crate::dns_jsc::cares_jsc::not_a_hostname_error(
global, hostname,
));
// SAFETY: caller contract — `this` is the live boxed server from `init()`.
Self::deinit(this);
return JSValue::ZERO;
}
}

let app: *mut uws_sys::NewApp<SSL>;
let route_list_value;

Expand Down Expand Up @@ -3057,14 +3073,14 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
let mut host: *const c_char = core::ptr::null();
if let Some(existing) = hostname.as_deref() {
let bytes = existing.as_bytes();
if bytes.len() > 2 && bytes[0] == b'[' {
// strip "[" and "]" from IPv6 literal
host = stripped_hostname
.insert(bun_core::ZBox::from_bytes(&bytes[1..bytes.len() - 1]))
.as_ptr();
let bare = strip_ipv6_brackets(bytes);
host = if bare.len() == bytes.len() {
existing.as_ptr()
} else {
host = existing.as_ptr();
}
stripped_hostname
.insert(bun_core::ZBox::from_bytes(bare))
.as_ptr()
};
}
Addr::Tcp { port: *port, host }
}
Expand Down Expand Up @@ -3566,6 +3582,16 @@ mod ffi {
}
}

/// `Bun.serve({ hostname: "[::1]" })`: uSockets wants the IPv6 literal bare.
fn strip_ipv6_brackets(hostname: &[u8]) -> &[u8] {
if let [b'[', inner @ .., b']'] = hostname {
if bun_core::ip_address::to_ip_address(inner).is_some_and(|ip| ip.is_ipv6()) {
return inner;
}
}
hostname
}

/// Drain the BoringSSL error queue; if non-empty, throw the top error on
/// `global` and return true.
fn throw_ssl_error_if_necessary(global: &JSGlobalObject) -> bool {
Expand Down
10 changes: 10 additions & 0 deletions src/runtime/socket/Listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,16 @@ impl Listener {
let _cell_root = socket_config.handlers.root_cell(global);

let port = socket_config.port;
if port.is_some() {
let hostname = socket_config.hostname_or_unix.slice();
if !bun_dns::is_valid_hostname(hostname) {
return Err(
global.throw_value(crate::dns_jsc::cares_jsc::not_a_hostname_error(
global, hostname,
)),
);
}
}
let ssl_enabled = socket_config.ssl.is_some();
let socket_flags = socket_config.socket_flags();
let pause_on_connect = socket_config.pause_on_connect;
Expand Down
24 changes: 24 additions & 0 deletions src/runtime/socket/udp_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,22 @@ impl UDPSocket {

let config = this.config.get();
let hostname_z = config.hostname.to_owned_slice_z();
if config.fd.is_none() && !bun_dns::is_valid_hostname(hostname_z.as_bytes()) {
return Err(
global_this.throw_value(crate::dns_jsc::cares_jsc::not_a_hostname_error(
global_this,
hostname_z.as_bytes(),
)),
);
}
if let Some(connect) = &config.connect {
let address = connect.address.to_utf8();
if !bun_dns::is_valid_hostname(&address) {
return Err(global_this.throw_value(
crate::dns_jsc::cares_jsc::not_a_hostname_error(global_this, &address),
));
}
}

// Reserve an adopted descriptor before creating the socket so a
// concurrent adoption of the same number fails with EEXIST (like
Expand Down Expand Up @@ -1896,6 +1912,14 @@ impl UDPSocket {

let str = args[0].to_bun_string(global_this)?;
let connect_host = str.to_owned_slice_z();
if !bun_dns::is_valid_hostname(connect_host.as_bytes()) {
return Err(
global_this.throw_value(crate::dns_jsc::cares_jsc::not_a_hostname_error(
global_this,
connect_host.as_bytes(),
)),
);
}

let connect_port_js = args[1];

Expand Down
44 changes: 44 additions & 0 deletions test/js/bun/http/serve-listen.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,50 @@ describe.each([
});
});

// A name that cannot be a host name (a space, a colon, an empty label,
// brackets around anything but an IPv6 literal) is rejected in-process with the
// resolver's ENOTFOUND, the error Bun.connect reports for the same name. The
// system resolver is never asked: some DNS servers never answer a query for
// such a label, and the synchronous getaddrinfo behind listen() then blocks
// startup for its full timeout.
describe.each([
["http", {}],
["https", { tls }],
] as const)("Bun.serve() %s with a hostname that is not a hostname", (_, options) => {
test.each(["this is not a hostname", "localhost:80", "a..b", "[not-an-ipv6]"])(
"%p throws getaddrinfo ENOTFOUND",
hostname => {
let error: any;
try {
serve({ ...options, hostname, port: 0, fetch: () => new Response() }).stop(true);
} catch (e) {
error = e;
}
const { name, code, syscall, hostname: errHostname, message } = error ?? {};
expect({ name, code, syscall, hostname: errHostname, message }).toEqual({
name: "Error",
code: "ENOTFOUND",
syscall: "getaddrinfo",
hostname,
message: `getaddrinfo ENOTFOUND ${hostname}`,
});
},
);

test("a later Bun.serve() still binds", () => {
using server = serve({ ...options, port: 0, fetch: () => new Response() });
expect(server.port).toBeInteger();
});
});

test.skipIf(!hasIPv6)("Bun.serve() binds a bracketed IPv6 literal hostname", () => {
using server = serve({ hostname: "[::1]", port: 0, fetch: () => new Response() });
expect({ hostname: server.hostname, urlHostname: server.url.hostname }).toEqual({
hostname: "[::1]",
urlHostname: "[::1]",
});
});

// Linux-only: uses /proc/self/fd to find the listen socket and close it from
// under the server so getsockname() fails with EBADF.
test.skipIf(!isLinux)("server.address / server.port do not panic when getsockname() fails", async () => {
Expand Down
16 changes: 16 additions & 0 deletions test/js/bun/net/socket-dns-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,22 @@ test.each(["this is not a hostname", "localhost:80", "a..b"])(
},
);

// The listen side binds through a synchronous getaddrinfo, so a name the
// resolver would sit on blocks the whole thread. The same names are rejected
// before the call, with the same error the connect side reports.
test.each(["this is not a hostname", "localhost:80", "a..b"])(
"Bun.listen on %p, which is not a hostname, throws ENOTFOUND without asking the resolver",
hostname => {
let error: any;
try {
Bun.listen({ hostname, port: 0, socket: { data() {} } }).stop(true);
} catch (e) {
error = e;
}
expect(pick(error ?? {})).toEqual({ ...EXPECTED, hostname, message: `getaddrinfo ENOTFOUND ${hostname}` });
},
);

// An answer that is already known when Bun.connect() is called (answered
// in-process, or a cache hit) is delivered from the event loop, not inline.
// When the connect is made from the callback that delivers the previous
Expand Down
30 changes: 30 additions & 0 deletions test/js/bun/udp/dgram.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,36 @@ test.skipIf(isWindows)("connected send() failure reports Node's error shape", as
});
});

// The default lookup is dns.lookup, which already answers such a name with
// ENOTFOUND. A custom lookup can hand the raw string to the native connect,
// which resolves it synchronously: the same check runs there too, and the
// socket does not end up marked connected.
test("connect() with a custom lookup that yields a name that cannot be a hostname reports getaddrinfo ENOTFOUND", async () => {
const hostname = "this is not a hostname";
const socket = createSocket({ type: "udp4", lookup: (address, _family, callback) => callback(null, address, 4) });
const { promise: bound, resolve: onBound } = Promise.withResolvers<void>();
socket.bind(0, "127.0.0.1", onBound);
await bound;

const err: any = await new Promise(resolve => socket.connect(1234, hostname, resolve));
let connected = true;
try {
socket.remoteAddress();
} catch {
connected = false;
}
socket.close();
const { name, code, syscall, hostname: errHostname, message } = err ?? {};
expect({ name, code, syscall, hostname: errHostname, message, connected }).toEqual({
name: "Error",
code: "ENOTFOUND",
syscall: "getaddrinfo",
hostname,
message: `getaddrinfo ENOTFOUND ${hostname}`,
connected: false,
});
});

// Every send callback must fire with (null, byteLength) — never (null, 0) or
// EAGAIN — even if the kernel refuses some writes (they queue and drain like
// libuv's uv_udp_try_send → uv_udp_send fallback).
Expand Down
38 changes: 38 additions & 0 deletions test/js/bun/udp/udp_socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,44 @@ describe("udpSocket()", () => {
).toThrow();
});

// The bind and the connect option both resolve their hostname with a
// synchronous getaddrinfo. A name that cannot be a host name is rejected
// before that call, with the resolver error node:dgram reports for it,
// instead of a stale errno named as the bind failure.
describe.each(["this is not a hostname", "localhost:80", "a..b"])("with %p, which is not a hostname", hostname => {
const resolverError = {
name: "Error",
code: "ENOTFOUND",
syscall: "getaddrinfo",
hostname,
message: `getaddrinfo ENOTFOUND ${hostname}`,
};
const pick = (e: any) => {
const { name, code, syscall, hostname, message } = e ?? {};
return { name, code, syscall, hostname, message };
};

test("bind fails with getaddrinfo ENOTFOUND", async () => {
let error: any;
try {
(await udpSocket({ hostname, port: 0 })).close();
} catch (e) {
error = e;
}
expect(pick(error)).toEqual(resolverError);
});

test("connect fails with getaddrinfo ENOTFOUND", async () => {
let error: any;
try {
(await udpSocket({ port: 0, connect: { hostname, port: 1234 } })).close();
} catch (e) {
error = e;
}
expect(pick(error)).toEqual(resolverError);
});
});

// Out-of-range connect.port used to be silently rewritten to 0, so send()
// returned true while every datagram was dropped. The bind path already
// rejected the same values; connect must too. Values beyond the i32 range
Expand Down
Loading