mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-09 10:28:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix crash in dns.lookup, ensure getaddrinfo() only returns IPv4-only or IPv6-only results when it should, normalize node:dns errors (#12223)

Browse Source
This commit is contained in:
Jarred Sumner
2024-06-28 18:45:10 -07:00
committed by GitHub
parent e22383dff9
commit 5f34387bea
7 changed files with 133 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files

123
src/bun.js/api/bun/dns_resolver.zig
View File

@@ -271,6 +271,10 @@ pub fn normalizeDNSName(name: []const u8, backend: *GetAddrInfo.Backend) []const
} else if (strings.isIPV6Address(name)) {
backend.* = .system;
}
// getaddrinfo() is inconsistent with ares_getaddrinfo() when using localhost
else if (strings.eqlComptime(name, "localhost")) {
backend.* = .system;
}
}
return name;
@@ -488,29 +492,15 @@ pub const CAresNameInfo = struct {
pub fn processResolve(this: *@This(), err_: ?c_ares.Error, _: i32, result: ?c_ares.struct_nameinfo) void {
if (err_) |err| {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("lookupService failed: {s}", .{err.label()});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init(err.code()).toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, err.toJS(globalThis));
this.deinit();
return;
}
if (result == null) {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("lookupService failed: No results", .{});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init("EUNREACHABLE").toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, c_ares.Error.ENOTFOUND.toJS(globalThis));
this.deinit();
return;
}
@@ -906,29 +896,15 @@ pub const CAresReverse = struct {
pub fn processResolve(this: *@This(), err_: ?c_ares.Error, _: i32, result: ?*c_ares.struct_hostent) void {
if (err_) |err| {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("reverse failed: {s}", .{err.label()});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init(err.code()).toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, err.toJS(globalThis));
this.deinit();
return;
}
if (result == null) {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("reverse failed: No results", .{});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init("EUNREACHABLE").toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, c_ares.Error.ENOTFOUND.toJS(globalThis));
this.deinit();
return;
}
@@ -985,28 +961,15 @@ pub fn CAresLookup(comptime cares_type: type, comptime type_name: []const u8) ty
pub fn processResolve(this: *@This(), err_: ?c_ares.Error, _: i32, result: ?*cares_type) void {
if (err_) |err| {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("{s} lookup failed: {s}", .{ type_name, err.label() });
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init(err.code()).toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, err.toJS(globalThis));
this.deinit();
return;
}
if (result == null) {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("{s} lookup failed: {s}", .{ type_name, "No results" });
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init("EUNREACHABLE").toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, c_ares.Error.ENOTFOUND.toJS(globalThis));
this.deinit();
return;
}
@@ -1070,19 +1033,14 @@ pub const DNSLookup = struct {
log("processGetAddrInfoNative: status={d}", .{status});
if (c_ares.Error.initEAI(status)) |err| {
var promise = this.promise;
var globalThis = this.globalThis;
const globalThis = this.globalThis;
const error_value = brk: {
if (err == .ESERVFAIL) {
break :brk bun.sys.Error.fromCode(bun.C.getErrno(@as(c_int, -1)), .getaddrinfo).toJSC(globalThis);
}
const error_value = globalThis.createErrorInstance("DNS lookup failed: {s}", .{err.label()});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init(err.code()).toValueGC(globalThis),
);
break :brk error_value;
break :brk err.toJS(globalThis);
};
this.deinit();
@@ -1096,28 +1054,17 @@ pub const DNSLookup = struct {
log("processGetAddrInfo", .{});
if (err_) |err| {
var promise = this.promise;
var globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("DNS lookup failed: {s}", .{err.label()});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init(err.code()).toValueGC(globalThis),
);
promise.rejectTask(globalThis, error_value);
const globalThis = this.globalThis;
promise.rejectTask(globalThis, err.toJS(globalThis));
this.deinit();
return;
}
if (result == null or result.?.node == null) {
var promise = this.promise;
var globalThis = this.globalThis;
const globalThis = this.globalThis;
const error_value = globalThis.createErrorInstance("DNS lookup failed: {s}", .{"No results"});
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init("EUNREACHABLE").toValueGC(globalThis),
);
const error_value = c_ares.Error.ENOTFOUND.toJS(globalThis);
promise.rejectTask(globalThis, error_value);
this.deinit();
return;
@@ -1396,7 +1343,14 @@ pub const InternalDNS = struct {
.addrlen = 0,
.canonname = null,
.family = std.c.AF.UNSPEC,
.flags = 0,
// If the system is IPv4-only or IPv6-only, then only return the corresponding address family.
// https://github.com/nodejs/node/commit/54dd7c38e507b35ee0ffadc41a716f1782b0d32f
// https://bugzilla.mozilla.org/show_bug.cgi?id=467497
// https://github.com/adobe/chromium/blob/cfe5bf0b51b1f6b9fe239c2a3c2f2364da9967d7/net/base/host_resolver_proc.cc#L122-L241
// https://github.com/nodejs/node/issues/33816
// https://github.com/aio-libs/aiohttp/issues/5357
// https://github.com/libuv/libuv/issues/2225
.flags = if (Environment.isPosix) bun.C.netdb.AI_ADDRCONFIG else 0,
.next = null,
.protocol = 0,
.socktype = std.c.SOCK.STREAM,
@@ -2411,13 +2365,8 @@ pub const DNSResolver = struct {
var channel: *c_ares.Channel = switch (resolver.getChannel()) {
.result => |res| res,
.err => |err| {
const system_error = JSC.SystemError{
.errno = -1,
.code = bun.String.static(err.code()),
.message = bun.String.static(err.label()),
};
defer ip_slice.deinit();
globalThis.throwValue(system_error.toErrorInstance(globalThis));
globalThis.throwValue(err.toJS(globalThis));
return .zero;
},
};
@@ -2796,13 +2745,7 @@ pub const DNSResolver = struct {
var channel: *c_ares.Channel = switch (this.getChannel()) {
.result => |res| res,
.err => |err| {
const system_error = JSC.SystemError{
.errno = -1,
.code = bun.String.static(err.code()),
.message = bun.String.static(err.label()),
};
globalThis.throwValue(system_error.toErrorInstance(globalThis));
globalThis.throwValue(err.toJS(globalThis));
return .zero;
},
};

4
src/darwin_c.zig
View File

@@ -816,3 +816,7 @@ pub const CLOCK_UPTIME_RAW = 8;
pub const CLOCK_UPTIME_RAW_APPROX = 9;
pub const CLOCK_PROCESS_CPUTIME_ID = 12;
pub const CLOCK_THREAD_CPUTIME_ID = 1;
pub const netdb = @cImport({
@cInclude("netdb.h");
});

57
src/deps/c_ares.zig
View File

@@ -388,7 +388,7 @@ pub const AddrInfo = extern struct {
addr_info: *AddrInfo,
globalThis: *JSC.JSGlobalObject,
) JSC.JSValue {
var node = addr_info.node.?;
var node = addr_info.node orelse return JSC.JSValue.createEmptyArray(globalThis, 0);
const array = JSC.JSValue.createEmptyArray(
globalThis,
node.count(),
@@ -1316,8 +1316,33 @@ pub const Error = enum(i32) {
ECANCELLED = ARES_ECANCELLED,
ESERVICE = ARES_ESERVICE,
pub fn toJS(this: Error, globalThis: *JSC.JSGlobalObject) JSC.JSValue {
const error_value = globalThis.createErrorInstance("{s}", .{this.label()});
error_value.put(
globalThis,
JSC.ZigString.static("name"),
JSC.ZigString.init("DNSException").toValueGC(globalThis),
);
error_value.put(
globalThis,
JSC.ZigString.static("code"),
JSC.ZigString.init(this.code()).toValueGC(globalThis),
);
error_value.put(
globalThis,
JSC.ZigString.static("errno"),
JSC.jsNumber(@intFromEnum(this)),
);
return error_value;
}
pub fn initEAI(rc: i32) ?Error {
if (comptime bun.Environment.isWindows) {
// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/lib/internal/errors.js#L807-L815
if (rc == libuv.UV_EAI_NODATA or rc == libuv.UV_EAI_NONAME) {
return Error.ENOTFOUND;
}
// TODO: revisit this
return switch (rc) {
0 => null,
@@ -1339,18 +1364,35 @@ pub const Error = enum(i32) {
};
}
return switch (@as(std.posix.system.EAI, @enumFromInt(rc))) {
const eai: std.posix.system.EAI = @enumFromInt(rc);
// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/lib/internal/errors.js#L807-L815
if (eai == .NODATA or eai == .NONAME) {
return Error.ENOTFOUND;
}
if (comptime bun.Environment.isLinux) {
switch (eai) {
.SOCKTYPE => return Error.ECONNREFUSED,
.IDN_ENCODE => return Error.EBADSTR,
.ALLDONE => return Error.ENOTFOUND,
.INPROGRESS => return Error.ETIMEOUT,
.CANCELED => return Error.ECANCELLED,
.NOTCANCELED => return Error.ECANCELLED,
else => {},
}
}
return switch (eai) {
@as(std.posix.system.EAI, @enumFromInt(0)) => return null,
.ADDRFAMILY => Error.EBADFAMILY,
.BADFLAGS => Error.EBADFLAGS, // Invalid hints
.FAIL => Error.EBADRESP,
.FAMILY => Error.EBADFAMILY,
.MEMORY => Error.ENOMEM,
.NODATA => Error.ENODATA,
.NONAME => Error.ENONAME,
.SERVICE => Error.ESERVICE,
.SYSTEM => Error.ESERVFAIL,
else => unreachable,
else => bun.todo(@src(), Error.ENOTIMP),
};
}
@@ -1411,6 +1453,11 @@ pub const Error = enum(i32) {
});
pub fn get(rc: i32) ?Error {
// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/lib/internal/errors.js#L807-L815
if (rc == ARES_ENODATA or rc == ARES_ENONAME) {
return get(ARES_ENOTFOUND);
}
return switch (rc) {
0 => null,
1...ARES_ESERVICE => @as(Error, @enumFromInt(rc)),

10
src/dns.zig
View File

@@ -42,7 +42,12 @@ pub const GetAddrInfo = struct {
pub const Options = packed struct {
family: Family = .unspecified,
socktype: SocketType = .unspecified,
/// Leaving this unset leads to many duplicate addresses returned.
/// Node hardcodes to `SOCK_STREAM`.
/// There don't seem to be any issues in Node's repo about this
/// So I think it's likely that nobody actually needs `SOCK_DGRAM` as a flag
/// https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/src/cares_wrap.cc#L1609
socktype: SocketType = .stream,
protocol: Protocol = .unspecified,
backend: Backend = Backend.default,
flags: i32 = 0,
@@ -171,7 +176,8 @@ pub const GetAddrInfo = struct {
pub fn fromJS(value: JSC.JSValue, globalObject: *JSC.JSGlobalObject) !SocketType {
if (value.isEmptyOrUndefinedOrNull())
return .unspecified;
// Default to .stream
return .stream;
if (value.isNumber()) {
return switch (value.to(i32)) {

28
src/js/node/dns.ts
View File

@@ -47,6 +47,7 @@ function lookup(domain, options, callback) {
}
dns.lookup(domain, options).then(res => {
throwIfEmpty(res);
res.sort((a, b) => a.family - b.family);
if (options?.all) {
@@ -333,18 +334,33 @@ var {
function setDefaultResultOrder() {}
function setServers() {}
const promisifyLookup = res => {
res.sort((a, b) => a.family - b.family);
const [{ address, family }] = res;
return { address, family };
};
const mapLookupAll = res => {
const { address, family } = res;
return { address, family };
};
function throwIfEmpty(res) {
if (res.length === 0) {
const err = new Error("No records found");
err.name = "DNSException";
err.code = "ENODATA";
// Hardcoded errno
err.errno = 1;
err.syscall = "getaddrinfo";
throw err;
}
}
Object.defineProperty(throwIfEmpty, "name", { value: "::bunternal::" });
const promisifyLookup = res => {
throwIfEmpty(res);
res.sort((a, b) => a.family - b.family);
const [{ address, family }] = res;
return { address, family };
};
const promisifyLookupAll = res => {
throwIfEmpty(res);
res.sort((a, b) => a.family - b.family);
return res.map(mapLookupAll);
};

4
src/linux_c.zig
View File

@@ -698,3 +698,7 @@ pub const RENAME_WHITEOUT = 1 << 2;
pub extern "C" fn quick_exit(code: c_int) noreturn;
pub extern "C" fn memrchr(ptr: [*]const u8, val: c_int, len: usize) ?[*]const u8;
pub const netdb = @cImport({
@cInclude("netdb.h");
});

18
test/js/bun/dns/resolve-dns.test.ts
View File

@@ -6,7 +6,7 @@ import { isIP, isIPv4, isIPv6 } from "node:net";
const backends = ["system", "libc", "c-ares"];
const validHostnames = ["localhost", "example.com"];
const invalidHostnames = ["adsfa.asdfasdf.asdf.com"]; // known invalid
const malformedHostnames = ["", " ", ".", " .", "localhost:80", "this is not a hostname"];
const malformedHostnames = [" ", ".", " .", "localhost:80", "this is not a hostname"];
const isWindows = process.platform === "win32";
describe("dns", () => {
describe.each(backends)("lookup() [backend: %s]", backend => {
@@ -100,14 +100,16 @@ describe("dns", () => {
// @ts-expect-error
expect(dns.lookup(hostname, { backend })).rejects.toMatchObject({
code: "DNS_ENOTFOUND",
name: "DNSException",
});
});
test.each(malformedHostnames)("'%s'", hostname => {
// @ts-expect-error
expect(dns.lookup(hostname, { backend })).rejects.toMatchObject({
code: "DNS_ENOTFOUND",
name: "DNSException",
});
});
// TODO: causes segfaults
// test.each(malformedHostnames)("%s", (hostname) => {
// // @ts-expect-error
// expect(dns.lookup(hostname, { backend })).rejects.toMatchObject({
// code: "DNS_ENOTFOUND",
// });
// });
});
});