Skip to content

Commit

Permalink
Revert "fix(ext/node): fix dns.lookup result ordering (#26264)" (#26621)
Browse files Browse the repository at this point in the history
This reverts commit d59599f.

Closes #26588
  • Loading branch information
kt3k authored and bartlomieju committed Oct 29, 2024
1 parent 6f68793 commit 36b7f9d
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 37 deletions.
6 changes: 3 additions & 3 deletions ext/node/polyfills/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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";
}
Expand Down
14 changes: 12 additions & 2 deletions ext/node/polyfills/internal/dns/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

/**
Expand Down
13 changes: 3 additions & 10 deletions ext/node/polyfills/internal_binding/cares_wrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,11 @@ export function getaddrinfo(

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

if (family === 6) {
recordTypes.push("AAAA");
} else if (family === 4) {
if (family === 0 || family === 4) {
recordTypes.push("A");
} else if (family === 0 && hostname === "localhost") {
// Ipv6 is preferred over Ipv4 for localhost
}
if (family === 0 || family === 6) {
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 () => {
Expand Down
24 changes: 17 additions & 7 deletions ext/node/polyfills/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 2 additions & 6 deletions tests/unit_node/http_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
Expand Down
12 changes: 3 additions & 9 deletions tests/unit_node/tls_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -106,7 +102,6 @@ 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,
Expand All @@ -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,
Expand Down

0 comments on commit 36b7f9d

Please sign in to comment.