Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ext/node): better dns.lookup compatibility #27936

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions ext/net/ops_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,14 +268,6 @@ where
n => n.to_string(),
};

{
let mut s = state.borrow_mut();
let permissions = s.borrow_mut::<NP>();
permissions
.check_net(&(&hostname, Some(0)), "Deno.startTls()")
.map_err(NetError::Permission)?;
}

let ca_certs = args
.ca_certs
.into_iter()
Expand Down
1 change: 1 addition & 0 deletions ext/node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ sys_traits = { workspace = true, features = ["real", "winapi", "libc"] }
thiserror.workspace = true
tokio.workspace = true
tokio-eld = "0.2"
tower-service.workspace = true
url.workspace = true
webpki-root-certs.workspace = true
winapi.workspace = true
Expand Down
1 change: 1 addition & 0 deletions ext/node/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ deno_core::extension!(deno_node,
ops::crypto::x509::op_node_x509_get_serial_number,
ops::crypto::x509::op_node_x509_key_usage,
ops::crypto::x509::op_node_x509_public_key,
ops::dns::op_getaddrinfo<P>,
ops::fs::op_node_fs_exists_sync<P>,
ops::fs::op_node_fs_exists<P>,
ops::fs::op_node_cp_sync<P>,
Expand Down
69 changes: 69 additions & 0 deletions ext/node/ops/dns.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2018-2025 the Deno authors. MIT license.

use std::cell::RefCell;
use std::rc::Rc;
use std::str::FromStr;

use deno_core::op2;
use deno_core::OpState;
use deno_error::JsError;
use deno_permissions::PermissionCheckError;
use hyper_util::client::legacy::connect::dns::GaiResolver;
use hyper_util::client::legacy::connect::dns::Name;
use serde::Serialize;
use tower_service::Service;

#[derive(Serialize, Debug)]
#[serde(rename_all = "camelCase")]
struct GetAddrInfoResult {
family: usize,
address: String,
}

#[derive(Debug, thiserror::Error, JsError)]
pub enum GetAddrInfoError {
#[class(inherit)]
#[error(transparent)]
Permission(#[from] PermissionCheckError),
#[class(type)]
#[error("Could not resolve the hostname \"{0}\"")]
Resolution(String),
}

#[op2(async, stack_trace)]
#[serde]
pub async fn op_getaddrinfo<P>(
state: Rc<RefCell<OpState>>,
#[string] hostname: String,
port: Option<u16>,
) -> Result<Vec<GetAddrInfoResult>, GetAddrInfoError>
where
P: crate::NodePermissions + 'static,
{
{
let mut state_ = state.borrow_mut();
let permissions = state_.borrow_mut::<P>();
permissions.check_net((hostname.as_str(), port), "lookup")?;
}
let mut resolver = GaiResolver::new();
let name = Name::from_str(&hostname)
.map_err(|_| GetAddrInfoError::Resolution(hostname.clone()))?;
resolver
.call(name)
.await
.map_err(|_| GetAddrInfoError::Resolution(hostname))
.map(|addrs| {
addrs
.into_iter()
.map(|addr| {
GetAddrInfoResult {
family: match addr {
std::net::SocketAddr::V4(_) => 4,
std::net::SocketAddr::V6(_) => 6,
},
address: addr.ip().to_string(),
}
})
.collect::<Vec<_>>()
})
}
1 change: 1 addition & 0 deletions ext/node/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
pub mod blocklist;
pub mod buffer;
pub mod crypto;
pub mod dns;
pub mod fs;
pub mod http;
pub mod http2;
Expand Down
7 changes: 7 additions & 0 deletions ext/node/polyfills/dns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export function lookup(
let family = 0;
let all = false;
let verbatim = getDefaultVerbatim();
let port = undefined;

// Parse arguments
if (hostname) {
Expand Down Expand Up @@ -230,6 +231,11 @@ export function lookup(
validateBoolean(options.verbatim, "options.verbatim");
verbatim = options.verbatim;
}

if (options?.port != null) {
validateNumber(options.port, "options.port");
port = options.port;
}
}

if (!hostname) {
Expand Down Expand Up @@ -263,6 +269,7 @@ export function lookup(
req.family = family;
req.hostname = hostname;
req.oncomplete = all ? onlookupall : onlookup;
req.port = port;

const err = getaddrinfo(
req,
Expand Down
5 changes: 5 additions & 0 deletions ext/node/polyfills/internal/dns/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ export interface LookupOptions {
hints?: number | undefined;
all?: boolean | undefined;
verbatim?: boolean | undefined;
/**
* Deno specific extension. If port is specified, the required net permission
* for the lookup call will be reduced to single port.
*/
port?: number | undefined;
}

export interface LookupOneOptions extends LookupOptions {
Expand Down
52 changes: 28 additions & 24 deletions ext/node/polyfills/internal_binding/cares_wrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@
// deno-lint-ignore-file prefer-primordials

import type { ErrnoException } from "ext:deno_node/internal/errors.ts";
import { isIPv4 } from "ext:deno_node/internal/net.ts";
import { codeMap } from "ext:deno_node/internal_binding/uv.ts";
import {
AsyncWrap,
providerType,
} from "ext:deno_node/internal_binding/async_wrap.ts";
import { ares_strerror } from "ext:deno_node/internal_binding/ares.ts";
import { notImplemented } from "ext:deno_node/_utils.ts";
import { op_getaddrinfo } from "ext:core/ops";

interface LookupAddress {
address: string;
Expand All @@ -45,6 +45,7 @@ interface LookupAddress {
export class GetAddrInfoReqWrap extends AsyncWrap {
family!: number;
hostname!: string;
port: number | undefined;

callback!: (
err: ErrnoException | null,
Expand All @@ -67,46 +68,49 @@ export function getaddrinfo(
_hints: number,
verbatim: boolean,
): number {
const addresses: string[] = [];
type Addr = {
address: string;
family: number;
};
let addresses: Addr[] = [];

// TODO(cmorten): use hints
// REF: https://nodejs.org/api/dns.html#dns_supported_getaddrinfo_flags

const recordTypes: ("A" | "AAAA")[] = [];

if (family === 0 || family === 4) {
recordTypes.push("A");
}
if (family === 0 || family === 6) {
recordTypes.push("AAAA");
}

(async () => {
await Promise.allSettled(
recordTypes.map((recordType) =>
Deno.resolveDns(hostname, recordType).then((records) => {
records.forEach((record) => addresses.push(record));
})
),
);

const error = addresses.length ? 0 : codeMap.get("EAI_NODATA")!;
let error = 0;
try {
addresses.push(...await op_getaddrinfo(hostname, req.port || undefined));
if (addresses.length === 0) {
error = codeMap.get("EAI_NODATA")!;
}
} catch (e) {
if (e instanceof Deno.errors.NotCapable) {
error = codeMap.get("EPERM")!;
} else {
error = codeMap.get("EAI_NODATA")!;
}
}

// TODO(cmorten): needs work
// REF: https://github.com/nodejs/node/blob/master/src/cares_wrap.cc#L1444
if (!verbatim) {
addresses.sort((a: string, b: string): number => {
if (isIPv4(a)) {
addresses.sort((a: Addr, b: Addr): number => {
if (a.family === 4 && b.family === 6) {
return -1;
} else if (isIPv4(b)) {
} else if (a.family === 6 && b.family === 4) {
return 1;
}

return 0;
});
}

req.oncomplete(error, addresses);
if (family === 4 || family === 6) {
addresses = addresses.filter((addr) => addr.family === family);
}

req.oncomplete(error, addresses.map((addr) => addr.address));
})();

return 0;
Expand Down
3 changes: 2 additions & 1 deletion ext/node/polyfills/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,7 @@ function _lookupAndConnect(
family: options.family,
hints: options.hints || 0,
all: false,
port,
};

if (
Expand Down Expand Up @@ -2179,7 +2180,7 @@ function _lookupAndListen(
exclusive: boolean,
flags: number,
) {
dnsLookup(address, function doListen(err, ip, addressType) {
dnsLookup(address, { port }, function doListen(err, ip, addressType) {
if (err) {
server.emit("error", err);
} else {
Expand Down
50 changes: 46 additions & 4 deletions tests/unit_node/http_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import net, { Socket } from "node:net";
import fs from "node:fs";
import { text } from "node:stream/consumers";

import { assert, assertEquals, fail } from "@std/assert";
import { assert, assertEquals, assertStringIncludes, fail } from "@std/assert";
import { assertSpyCalls, spy } from "@std/testing/mock";
import { fromFileUrl, relative } from "@std/path";
import { retry } from "@std/async/retry";
Expand Down Expand Up @@ -1038,7 +1038,7 @@ Deno.test(
);
const { promise, resolve, reject } = Promise.withResolvers<void>();

const request = http.request("http://localhost:5929/");
const request = http.request("http://127.0.0.1:5929/");
request.on("error", reject);
request.on("close", () => {});
request.end();
Expand All @@ -1058,7 +1058,7 @@ Deno.test(
"[node/http] client destroy before sending request should not error",
async () => {
const { resolve, promise } = Promise.withResolvers<void>();
const request = http.request("http://localhost:5929/");
const request = http.request("http://127.0.0.1:5929/");
// Calling this would throw
request.destroy();
request.on("error", (e) => {
Expand Down Expand Up @@ -1087,7 +1087,7 @@ Deno.test(
receivedRequest = true;
return new Response(null);
});
const request = http.request(`http://localhost:${server.addr.port}/`);
const request = http.request(`http://127.0.0.1:${server.addr.port}/`);
request.destroy();
request.end("hello");
request.on("error", (err) => {
Expand Down Expand Up @@ -1945,3 +1945,45 @@ Deno.test("[node/http] supports proxy http request", async () => {
await promise;
await server.finished;
});

Deno.test("[node/http] `request` requires net permission to host and port", {
permissions: { net: ["localhost:4545"] },
}, async () => {
const { promise, resolve } = Promise.withResolvers<void>();
http.request("http://localhost:4545/echo.ts", async (res) => {
assertEquals(res.statusCode, 200);
assertStringIncludes(await text(res), "function echo(");
resolve();
}).end();
await promise;
});

const ca = await Deno.readTextFile("tests/testdata/tls/RootCA.pem");

Deno.test("[node/https] `request` requires net permission to host and port", {
permissions: { net: ["localhost:5545"] },
}, async () => {
const { promise, resolve } = Promise.withResolvers<void>();
https.request("https://localhost:5545/echo.ts", { ca }, async (res) => {
assertEquals(res.statusCode, 200);
assertStringIncludes(await text(res), "function echo(");
resolve();
}).end();
await promise;
});

Deno.test(
"[node/http] `request` errors with EPERM error when permission is not granted",
{ permissions: { net: ["localhost:4321"] } }, // wrong permission
async () => {
const { promise, resolve } = Promise.withResolvers<void>();
http.request("http://localhost:4545/echo.ts", async () => {})
.on("error", (e) => {
assertEquals(e.message, "getaddrinfo EPERM localhost");
// deno-lint-ignore no-explicit-any
assertEquals((e as any).code, "EPERM");
resolve();
}).end();
await promise;
},
);
Loading