1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00

fix(ext/node): fix dns.lookup result ordering (#26264)

partially unblocks #25470

This PR aligns the resolution of `localhost` hostname to Node.js
behavior.

In Node.js `dns.lookup("localhost", (_, addr) => console.log(addr))`
prints ipv6 address `::1`, but it prints ipv4 address `127.0.0.1` in
Deno. That difference causes some errors in the work of enabling
`createConnection` option in `http.request` (#25470). This PR fixes the
issue by aligning `dns.lookup` behavior to Node.js.

This PR also changes the following behaviors (resolving TODOs):
- `http.createServer` now listens on ipv6 address `[::]` by default on
linux/mac
- `net.createServer` now listens on ipv6 address `[::]` by default on
linux/mac

These changes are also alignments to Node.js behaviors.
This commit is contained in:
Yoshiya Hinosawa 2024-10-16 20:58:44 +09:00 committed by GitHub
parent ea9c3ffaa2
commit d59599fc18
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 38 additions and 41 deletions

View file

@ -50,6 +50,7 @@ import { urlToHttpOptions } from "ext:deno_node/internal/url.ts";
import { kEmptyObject } from "ext:deno_node/internal/util.mjs";
import { constants, TCP } from "ext:deno_node/internal_binding/tcp_wrap.ts";
import { notImplemented, warnNotImplemented } from "ext:deno_node/_utils.ts";
import { isWindows } from "ext:deno_node/_util/os.ts";
import {
connResetException,
ERR_HTTP_HEADERS_SENT,
@ -1586,9 +1587,8 @@ export class ServerImpl extends EventEmitter {
port = options.port | 0;
}
// TODO(bnoordhuis) Node prefers [::] when host is omitted,
// we on the other hand default to 0.0.0.0.
let hostname = options.host ?? "0.0.0.0";
// Use 0.0.0.0 for Windows, and [::] for other platforms.
let hostname = options.host ?? (isWindows ? "0.0.0.0" : "[::]");
if (hostname == "localhost") {
hostname = "127.0.0.1";
}

View file

@ -416,20 +416,10 @@ export function emitInvalidHostnameWarning(hostname: string) {
);
}
let dnsOrder = getOptionValue("--dns-result-order") || "ipv4first";
let dnsOrder = getOptionValue("--dns-result-order") || "verbatim";
export function getDefaultVerbatim() {
switch (dnsOrder) {
case "verbatim": {
return true;
}
case "ipv4first": {
return false;
}
default: {
return false;
}
}
return dnsOrder !== "ipv4first";
}
/**

View file

@ -75,11 +75,18 @@ export function getaddrinfo(
const recordTypes: ("A" | "AAAA")[] = [];
if (family === 0 || family === 4) {
recordTypes.push("A");
}
if (family === 0 || family === 6) {
if (family === 6) {
recordTypes.push("AAAA");
} else if (family === 4) {
recordTypes.push("A");
} else if (family === 0 && hostname === "localhost") {
// Ipv6 is preferred over Ipv4 for localhost
recordTypes.push("AAAA");
recordTypes.push("A");
} else if (family === 0) {
// Only get Ipv4 addresses for the other hostnames
// This simulates what `getaddrinfo` does when the family is not specified
recordTypes.push("A");
}
(async () => {

View file

@ -1871,23 +1871,13 @@ function _setupListenHandle(
// Try to bind to the unspecified IPv6 address, see if IPv6 is available
if (!address && typeof fd !== "number") {
// TODO(@bartlomieju): differs from Node which tries to bind to IPv6 first
// when no address is provided.
//
// Forcing IPv4 as a workaround for Deno not aligning with Node on
// implicit binding on Windows.
//
// REF: https://github.com/denoland/deno/issues/10762
// rval = _createServerHandle(DEFAULT_IPV6_ADDR, port, 6, fd, flags);
// if (typeof rval === "number") {
// rval = null;
address = DEFAULT_IPV4_ADDR;
addressType = 4;
// } else {
// address = DEFAULT_IPV6_ADDR;
// addressType = 6;
// }
if (isWindows) {
address = DEFAULT_IPV4_ADDR;
addressType = 4;
} else {
address = DEFAULT_IPV6_ADDR;
addressType = 6;
}
}
if (rval === null) {

View file

@ -318,10 +318,14 @@ Deno.test("[node/http] IncomingRequest socket has remoteAddress + remotePort", a
// deno-lint-ignore no-explicit-any
const port = (server.address() as any).port;
const res = await fetch(
`http://127.0.0.1:${port}/`,
`http://localhost:${port}/`,
);
await res.arrayBuffer();
assertEquals(remoteAddress, "127.0.0.1");
if (Deno.build.os === "windows") {
assertEquals(remoteAddress, "127.0.0.1");
} else {
assertEquals(remoteAddress, "::1");
}
assertEquals(typeof remotePort, "number");
server.close(() => resolve());
});

View file

@ -32,13 +32,15 @@ for (
) {
Deno.test(`tls.connect sends correct ALPN: '${alpnServer}' + '${alpnClient}' = '${expected}'`, async () => {
const listener = Deno.listenTls({
hostname: "localhost",
port: 0,
key,
cert,
alpnProtocols: alpnServer,
});
const outgoing = tls.connect({
host: "localhost",
host: "::1",
servername: "localhost",
port: listener.addr.port,
ALPNProtocols: alpnClient,
secureContext: {
@ -61,6 +63,7 @@ Deno.test("tls.connect makes tls connection", async () => {
const ctl = new AbortController();
let port;
const serve = Deno.serve({
hostname: "localhost",
port: 0,
key,
cert,
@ -71,7 +74,8 @@ Deno.test("tls.connect makes tls connection", async () => {
await delay(200);
const conn = tls.connect({
host: "localhost",
host: "::1",
servername: "localhost",
port,
secureContext: {
ca: rootCaCert,
@ -102,6 +106,7 @@ Deno.test("tls.connect mid-read tcp->tls upgrade", async () => {
const { promise, resolve } = Promise.withResolvers<void>();
const ctl = new AbortController();
const serve = Deno.serve({
hostname: "localhost",
port: 8443,
key,
cert,
@ -111,7 +116,8 @@ Deno.test("tls.connect mid-read tcp->tls upgrade", async () => {
await delay(200);
const conn = tls.connect({
host: "localhost",
host: "::1",
servername: "localhost",
port: 8443,
secureContext: {
ca: rootCaCert,