From a69224ea5bd02f08108aac867c492754095f2d34 Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Wed, 30 Oct 2024 02:41:16 +0900 Subject: [PATCH] Revert "fix(ext/node): fix dns.lookup result ordering (#26264)" (#26621) This reverts commit d59599fc187c559ee231882773e1c5a2b932fc3d. Closes #26588 --- ext/node/polyfills/http.ts | 6 ++--- ext/node/polyfills/internal/dns/utils.ts | 14 +++++++++-- .../polyfills/internal_binding/cares_wrap.ts | 15 ++++-------- ext/node/polyfills/net.ts | 24 +++++++++++++------ tests/unit_node/http_test.ts | 8 ++----- tests/unit_node/tls_test.ts | 12 +++------- 6 files changed, 41 insertions(+), 38 deletions(-) diff --git a/ext/node/polyfills/http.ts b/ext/node/polyfills/http.ts index e7a860ff1c..e8a189f70f 100644 --- a/ext/node/polyfills/http.ts +++ b/ext/node/polyfills/http.ts @@ -51,7 +51,6 @@ 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, @@ -1712,8 +1711,9 @@ export class ServerImpl extends EventEmitter { port = options.port | 0; } - // Use 0.0.0.0 for Windows, and [::] for other platforms. - let hostname = options.host ?? (isWindows ? "0.0.0.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"; if (hostname == "localhost") { hostname = "127.0.0.1"; } diff --git a/ext/node/polyfills/internal/dns/utils.ts b/ext/node/polyfills/internal/dns/utils.ts index 1e0c3d9ed6..226fce93dd 100644 --- a/ext/node/polyfills/internal/dns/utils.ts +++ b/ext/node/polyfills/internal/dns/utils.ts @@ -416,10 +416,20 @@ export function emitInvalidHostnameWarning(hostname: string) { ); } -let dnsOrder = getOptionValue("--dns-result-order") || "verbatim"; +let dnsOrder = getOptionValue("--dns-result-order") || "ipv4first"; export function getDefaultVerbatim() { - return dnsOrder !== "ipv4first"; + switch (dnsOrder) { + case "verbatim": { + return true; + } + case "ipv4first": { + return false; + } + default: { + return false; + } + } } /** diff --git a/ext/node/polyfills/internal_binding/cares_wrap.ts b/ext/node/polyfills/internal_binding/cares_wrap.ts index cbfea40b22..6feb7faf0d 100644 --- a/ext/node/polyfills/internal_binding/cares_wrap.ts +++ b/ext/node/polyfills/internal_binding/cares_wrap.ts @@ -75,18 +75,11 @@ export function getaddrinfo( const recordTypes: ("A" | "AAAA")[] = []; - if (family === 6) { + if (family === 0 || family === 4) { + recordTypes.push("A"); + } + if (family === 0 || 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 () => { diff --git a/ext/node/polyfills/net.ts b/ext/node/polyfills/net.ts index 35d273be93..48e1d0de87 100644 --- a/ext/node/polyfills/net.ts +++ b/ext/node/polyfills/net.ts @@ -1871,13 +1871,23 @@ function _setupListenHandle( // Try to bind to the unspecified IPv6 address, see if IPv6 is available if (!address && typeof fd !== "number") { - if (isWindows) { - address = DEFAULT_IPV4_ADDR; - addressType = 4; - } else { - address = DEFAULT_IPV6_ADDR; - addressType = 6; - } + // 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 (rval === null) { diff --git a/tests/unit_node/http_test.ts b/tests/unit_node/http_test.ts index 84d6f57279..2b3b8f509f 100644 --- a/tests/unit_node/http_test.ts +++ b/tests/unit_node/http_test.ts @@ -322,14 +322,10 @@ 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://localhost:${port}/`, + `http://127.0.0.1:${port}/`, ); await res.arrayBuffer(); - if (Deno.build.os === "windows") { - assertEquals(remoteAddress, "127.0.0.1"); - } else { - assertEquals(remoteAddress, "::1"); - } + assertEquals(remoteAddress, "127.0.0.1"); assertEquals(typeof remotePort, "number"); server.close(() => resolve()); }); diff --git a/tests/unit_node/tls_test.ts b/tests/unit_node/tls_test.ts index 236dab2086..43d6205b0b 100644 --- a/tests/unit_node/tls_test.ts +++ b/tests/unit_node/tls_test.ts @@ -32,15 +32,13 @@ 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: "::1", - servername: "localhost", + host: "localhost", port: listener.addr.port, ALPNProtocols: alpnClient, secureContext: { @@ -63,7 +61,6 @@ Deno.test("tls.connect makes tls connection", async () => { const ctl = new AbortController(); let port; const serve = Deno.serve({ - hostname: "localhost", port: 0, key, cert, @@ -74,8 +71,7 @@ Deno.test("tls.connect makes tls connection", async () => { await delay(200); const conn = tls.connect({ - host: "::1", - servername: "localhost", + host: "localhost", port, secureContext: { ca: rootCaCert, @@ -106,7 +102,6 @@ Deno.test("tls.connect mid-read tcp->tls upgrade", async () => { const { promise, resolve } = Promise.withResolvers(); const ctl = new AbortController(); const serve = Deno.serve({ - hostname: "localhost", port: 8443, key, cert, @@ -116,8 +111,7 @@ Deno.test("tls.connect mid-read tcp->tls upgrade", async () => { await delay(200); const conn = tls.connect({ - host: "::1", - servername: "localhost", + host: "localhost", port: 8443, secureContext: { ca: rootCaCert,