From 9413807338b29d4af8fa7d69ae2bd0a470ebc864 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 10:56:00 +0000 Subject: [PATCH 01/19] feat: add dns cache #2440 --- lib/core/connect.js | 7 +- lib/core/lookup.js | 154 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 lib/core/lookup.js diff --git a/lib/core/connect.js b/lib/core/connect.js index 2d2521dcfbd..4d02a54a992 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -4,6 +4,7 @@ const net = require('net') const assert = require('assert') const util = require('./util') const { InvalidArgumentError, ConnectTimeoutError } = require('./errors') +const lookup = require('./lookup') let tls // include tls conditionally since it is not always available @@ -105,7 +106,8 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, ...o ALPNProtocols: allowH2 ? ['http/1.1', 'h2'] : ['http/1.1'], socket: httpSocket, // upgrade socket connection port: port || 443, - host: hostname + host: hostname, + lookup: process.env.LOOKUP ? lookup : undefined, }) socket @@ -120,7 +122,8 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, ...o ...options, localAddress, port: port || 80, - host: hostname + host: hostname, + lookup: process.env.LOOKUP ? lookup : undefined }) } diff --git a/lib/core/lookup.js b/lib/core/lookup.js new file mode 100644 index 00000000000..a6085e805a4 --- /dev/null +++ b/lib/core/lookup.js @@ -0,0 +1,154 @@ +const { promises: dnsPromises } = require('node:dns') +const os = require('node:os') + +const { Resolver: AsyncResolver } = dnsPromises + +const cache = new Map() + +const resolver = new AsyncResolver() +const maxTtl = Infinity +let _nextRemovalTime = false +let _removalTimeout = undefined + +const getIfaceInfo = () => { + let has4 = false + let has6 = false + + for (const device of Object.values(os.networkInterfaces())) { + for (const iface of device) { + if (iface.internal) { + continue + } + + if (iface.family === 'IPv6') { + has6 = true + } else { + has4 = true + } + + if (has4 && has6) { + return { has4, has6 } + } + } + } + return { has4, has6 } +} + +const ttl = { ttl: true } + +function lookup(hostname, options, callback) { + if (typeof options === 'function') { + callback = options + options = {} + } else if (typeof options === 'number') { + options = { family: options } + } + + if (!callback) { + throw new Error('Function needs callback') + } + + lookupAsync(hostname).then((result) => { + callback(null, result.address, result.family, result.expires, result.ttl) + }, callback) +} + +async function lookupAsync(hostname) { + const cached = cache.get(hostname) + if (cached) { + return cached + } + + const { has6 } = getIfaceInfo() + + const [A, AAAA] = await Promise.all([ + resolver.resolve4(hostname, ttl), + resolver.resolve6(hostname, ttl) + ]) + + let cacheTtl = 0 + let aTtl = 0 + let aaaaTtl = 0 + + for (const entry of AAAA) { + entry.family = 6 + entry.expires = Date.now + entry.ttl * 1000 + aaaaTtl = Math.max(aaaaTtl, entry.ttl) + } + + for (const entry of A) { + entry.family = 4 + entry.expires = Date.now + entry.ttl * 1000 + aTtl = Math.max(aTtl, entry.ttl) + } + + if (A.length > 0) { + if (AAAA.length > 0) { + cacheTtl = Math.min(aTtl, aaaaTtl) + } else { + cacheTtl = aTtl + } + } else { + cacheTtl = aaaaTtl + } + + // favourite ipv6 if available + let result + if (has6 && AAAA.length) { + result = AAAA[0] + } else { + result = A[0] + } + + set(hostname, result, cacheTtl) + + return result +} + +function set(hostname, data, cacheTtl) { + if (maxTtl > 0 && cacheTtl > 0) { + cacheTtl = Math.min(cacheTtl, maxTtl) * 1000 + data.expires = Date.now() + cacheTtl + cache.set(hostname, data) + tick(cacheTtl) + } +} + +function tick(ms) { + const nextRemovalTime = _nextRemovalTime + + if (!nextRemovalTime || ms < nextRemovalTime) { + clearTimeout(_removalTimeout) + + _nextRemovalTime = ms + + _removalTimeout = setTimeout(() => { + _nextRemovalTime = false + + let nextExpiry = Infinity + + const now = Date.now() + + for (const [hostname, entries] of this._cache) { + const expires = entries.expires + + if (now >= expires) { + cache.delete(hostname) + } else if (expires < nextExpiry) { + nextExpiry = expires + } + } + + if (nextExpiry !== Infinity) { + tick(nextExpiry - now) + } + }, ms) + + /* istanbul ignore next: There is no `timeout.unref()` when running inside an Electron renderer */ + if (_removalTimeout.unref) { + _removalTimeout.unref() + } + } +} + +module.exports = lookup From 51707630df83b4045224f704136253096744163f Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 13:13:36 +0000 Subject: [PATCH 02/19] refactor --- index.js | 3 + lib/core/connect.js | 6 +- lib/core/dns-cache.js | 269 ++++++++++++++++++++++++++++++++++++++++++ lib/core/lookup.js | 154 ------------------------ 4 files changed, 275 insertions(+), 157 deletions(-) create mode 100644 lib/core/dns-cache.js delete mode 100644 lib/core/lookup.js diff --git a/index.js b/index.js index 5fc0d9727ef..93b551b1219 100644 --- a/index.js +++ b/index.js @@ -165,3 +165,6 @@ module.exports.MockClient = MockClient module.exports.MockPool = MockPool module.exports.MockAgent = MockAgent module.exports.mockErrors = mockErrors + +const DnsCache = require('./lib/core/dns-cache') +module.exports.DnsCache = DnsCache diff --git a/lib/core/connect.js b/lib/core/connect.js index 4d02a54a992..ce90428c609 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -4,7 +4,7 @@ const net = require('net') const assert = require('assert') const util = require('./util') const { InvalidArgumentError, ConnectTimeoutError } = require('./errors') -const lookup = require('./lookup') +const { lookup, dnsCacheEnabled } = require('./dns-cache') let tls // include tls conditionally since it is not always available @@ -107,7 +107,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, ...o socket: httpSocket, // upgrade socket connection port: port || 443, host: hostname, - lookup: process.env.LOOKUP ? lookup : undefined, + lookup: dnsCacheEnabled() ? lookup : undefined }) socket @@ -123,7 +123,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, ...o localAddress, port: port || 80, host: hostname, - lookup: process.env.LOOKUP ? lookup : undefined + lookup: dnsCacheEnabled() ? lookup : undefined }) } diff --git a/lib/core/dns-cache.js b/lib/core/dns-cache.js new file mode 100644 index 00000000000..07227d3c052 --- /dev/null +++ b/lib/core/dns-cache.js @@ -0,0 +1,269 @@ +const { + promises: dnsPromises, + lookup: _dnsLookup +} = require('node:dns') +const os = require('node:os') +const { promisify } = require('node:util') + +const { Resolver: AsyncResolver } = dnsPromises +const dnsLookup = promisify(_dnsLookup) + +const cache = new Map() +const _pending = new Map() +const hostnamesToFallback = new Set() + +const resolver = new AsyncResolver() +const maxTtl = Infinity +const fallbackDuration = 3600 +const errorTtl = 0.15 +let _nextRemovalTime = false +let _removalTimeout = null + +let cacheEnabled = true + +function enableCacheLookup () { + cacheEnabled = true +} + +function disableCacheLookup () { + cacheEnabled = false +} + +const getIfaceInfo = () => { + let has4 = false + let has6 = false + + for (const device of Object.values(os.networkInterfaces())) { + for (const iface of device) { + if (iface.internal) { + continue + } + + if (iface.family === 'IPv6') { + has6 = true + } else { + has4 = true + } + + if (has4 && has6) { + return { has4, has6 } + } + } + } + return { has4, has6 } +} + +function ignoreNoResultErrors (dnsPromise) { + return dnsPromise.catch(error => { + switch (error.code) { + case 'ENODATA': + case 'ENOTFOUND': + case 'ENOENT': + return [] + default: + throw error + } + }) +} + +const ttl = { ttl: true } +const all = { all: true } +const all4 = { all: true, family: 4 } +const all6 = { all: true, family: 6 } +const { has6 } = getIfaceInfo() + +function lookup (hostname, options, callback) { + if (typeof options === 'function') { + callback = options + options = {} + } else if (typeof options === 'number') { + options = { family: options } + } + + if (!callback) { + throw new Error('Function needs callback') + } + + lookupAsync(hostname).then((result) => { + // @TODO remove + console.log(result.address) + callback(null, result.address, result.family, result.expires, result.ttl) + }, callback) +} + +async function lookupAsync (hostname) { + const cached = await query(hostname) + let filtered = [] + + // IMOH (antoinegomez) we should always prioritize ipv6 if iface available and dns resolved + // @TODO better filter options to allow to pool through ipv6 + v4 + if (has6) { + filtered = cached.filter(entry => entry.family === 6) + + // if not v6 returned reset to the results, should be v4 only or empty + if (filtered.length === 0) { + filtered = cached + } + } else { + filtered = cached.filter(entry => entry.family === 4) + } + + if (filtered.length === 0) { + const error = new Error(`dnsLookup ENOTFOUND ${hostname}`) + error.code = 'ENOTFOUND' + error.hostname = hostname + throw error + } + + // return random result, better for balancing + // @TODO: keep track of usage or loop through filtered results instead? + return filtered[Math.floor(Math.random() * filtered.length)] +} + +async function query (hostname) { + let cached = cache.get(hostname) + + if (!cached || cacheEnabled === false) { + const pending = _pending.get(hostname) + if (pending) { + cached = await pending + } else { + const newPromise = queryAndCache(hostname) + _pending.set(hostname, newPromise) + try { + cached = await newPromise + } finally { + _pending.delete(hostname) + } + } + } + + return cached +} + +async function queryAndCache (hostname) { + if (hostnamesToFallback.has(hostname)) { + return dnsLookup(hostname, all) + } + + let query = await resolve(hostname) + + if (query.length === 0) { + query = await _lookup(hostname) + + if (query.entries.length !== 0 && fallbackDuration > 0) { + hostnamesToFallback(hostname) + } + } + + const cacheTtl = query.entries.length === 0 ? errorTtl : query.cacheTtl + set(hostname, query.entries, cacheTtl) + return query.entries +} + +async function _lookup (hostname) { + try { + const [A, AAAA] = await Promise.all([ + ignoreNoResultErrors(dnsLookup(hostname, all4)), + ignoreNoResultErrors(dnsLookup(hostname, all6)) + ]) + + return { + entries: [...AAAA, ...A], + cacheTtl: 0 + } + } catch { + return { + entries: [], + cacheTtl: 0 + } + } +} + +async function resolve (hostname) { + const [A, AAAA] = await Promise.all([ + ignoreNoResultErrors(resolver.resolve4(hostname, ttl)), + ignoreNoResultErrors(resolver.resolve6(hostname, ttl)) + ]) + + let cacheTtl = 0 + let aTtl = 0 + let aaaaTtl = 0 + + for (const entry of AAAA) { + entry.family = 6 + entry.expires = Date.now + entry.ttl * 1000 + aaaaTtl = Math.max(aaaaTtl, entry.ttl) + } + + for (const entry of A) { + entry.family = 4 + entry.expires = Date.now + entry.ttl * 1000 + aTtl = Math.max(aTtl, entry.ttl) + } + + if (A.length > 0) { + if (AAAA.length > 0) { + cacheTtl = Math.min(aTtl, aaaaTtl) + } else { + cacheTtl = aTtl + } + } else { + cacheTtl = aaaaTtl + } + + // favourite ipv6 if available + const result = [...AAAA, ...A] + return { entries: result, cacheTtl } +} + +function set (hostname, data, cacheTtl) { + if (maxTtl > 0 && cacheTtl > 0) { + cacheTtl = Math.min(cacheTtl, maxTtl) * 1000 + data.expires = Date.now() + cacheTtl + cache.set(hostname, data) + tick(cacheTtl) + } +} + +function tick (ms) { + const nextRemovalTime = _nextRemovalTime + + if (!nextRemovalTime || ms < nextRemovalTime) { + clearTimeout(_removalTimeout) + + _nextRemovalTime = ms + + _removalTimeout = setTimeout(() => { + _nextRemovalTime = false + + let nextExpiry = Infinity + + const now = Date.now() + + for (const [hostname, entries] of cache) { + const expires = entries.expires + + if (now >= expires) { + cache.delete(hostname) + } else if (expires < nextExpiry) { + nextExpiry = expires + } + } + + if (nextExpiry !== Infinity) { + tick(nextExpiry - now) + } + }, ms) + + /* istanbul ignore next: There is no `timeout.unref()` when running inside an Electron renderer */ + if (_removalTimeout.unref) { + _removalTimeout.unref() + } + } +} + +module.exports.lookup = lookup +module.exports.disableCacheLookup = disableCacheLookup +module.exports.enaleCacheLookup = enableCacheLookup +module.exports.dnsCacheEnabled = () => cacheEnabled diff --git a/lib/core/lookup.js b/lib/core/lookup.js deleted file mode 100644 index a6085e805a4..00000000000 --- a/lib/core/lookup.js +++ /dev/null @@ -1,154 +0,0 @@ -const { promises: dnsPromises } = require('node:dns') -const os = require('node:os') - -const { Resolver: AsyncResolver } = dnsPromises - -const cache = new Map() - -const resolver = new AsyncResolver() -const maxTtl = Infinity -let _nextRemovalTime = false -let _removalTimeout = undefined - -const getIfaceInfo = () => { - let has4 = false - let has6 = false - - for (const device of Object.values(os.networkInterfaces())) { - for (const iface of device) { - if (iface.internal) { - continue - } - - if (iface.family === 'IPv6') { - has6 = true - } else { - has4 = true - } - - if (has4 && has6) { - return { has4, has6 } - } - } - } - return { has4, has6 } -} - -const ttl = { ttl: true } - -function lookup(hostname, options, callback) { - if (typeof options === 'function') { - callback = options - options = {} - } else if (typeof options === 'number') { - options = { family: options } - } - - if (!callback) { - throw new Error('Function needs callback') - } - - lookupAsync(hostname).then((result) => { - callback(null, result.address, result.family, result.expires, result.ttl) - }, callback) -} - -async function lookupAsync(hostname) { - const cached = cache.get(hostname) - if (cached) { - return cached - } - - const { has6 } = getIfaceInfo() - - const [A, AAAA] = await Promise.all([ - resolver.resolve4(hostname, ttl), - resolver.resolve6(hostname, ttl) - ]) - - let cacheTtl = 0 - let aTtl = 0 - let aaaaTtl = 0 - - for (const entry of AAAA) { - entry.family = 6 - entry.expires = Date.now + entry.ttl * 1000 - aaaaTtl = Math.max(aaaaTtl, entry.ttl) - } - - for (const entry of A) { - entry.family = 4 - entry.expires = Date.now + entry.ttl * 1000 - aTtl = Math.max(aTtl, entry.ttl) - } - - if (A.length > 0) { - if (AAAA.length > 0) { - cacheTtl = Math.min(aTtl, aaaaTtl) - } else { - cacheTtl = aTtl - } - } else { - cacheTtl = aaaaTtl - } - - // favourite ipv6 if available - let result - if (has6 && AAAA.length) { - result = AAAA[0] - } else { - result = A[0] - } - - set(hostname, result, cacheTtl) - - return result -} - -function set(hostname, data, cacheTtl) { - if (maxTtl > 0 && cacheTtl > 0) { - cacheTtl = Math.min(cacheTtl, maxTtl) * 1000 - data.expires = Date.now() + cacheTtl - cache.set(hostname, data) - tick(cacheTtl) - } -} - -function tick(ms) { - const nextRemovalTime = _nextRemovalTime - - if (!nextRemovalTime || ms < nextRemovalTime) { - clearTimeout(_removalTimeout) - - _nextRemovalTime = ms - - _removalTimeout = setTimeout(() => { - _nextRemovalTime = false - - let nextExpiry = Infinity - - const now = Date.now() - - for (const [hostname, entries] of this._cache) { - const expires = entries.expires - - if (now >= expires) { - cache.delete(hostname) - } else if (expires < nextExpiry) { - nextExpiry = expires - } - } - - if (nextExpiry !== Infinity) { - tick(nextExpiry - now) - } - }, ms) - - /* istanbul ignore next: There is no `timeout.unref()` when running inside an Electron renderer */ - if (_removalTimeout.unref) { - _removalTimeout.unref() - } - } -} - -module.exports = lookup From 65cc4fc10c7cad5244ec7ba7e07727ab20ec5e39 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 15:28:51 +0000 Subject: [PATCH 03/19] use whole cacheable-dns module instead, use it in Agent and Pool --- index.js | 3 - lib/agent.js | 2 + lib/core/connect.js | 7 +- lib/core/dns-cache.js | 269 ------------------------ lib/core/dns-resolver.js | 437 +++++++++++++++++++++++++++++++++++++++ lib/pool.js | 2 + 6 files changed, 443 insertions(+), 277 deletions(-) delete mode 100644 lib/core/dns-cache.js create mode 100644 lib/core/dns-resolver.js diff --git a/index.js b/index.js index 93b551b1219..5fc0d9727ef 100644 --- a/index.js +++ b/index.js @@ -165,6 +165,3 @@ module.exports.MockClient = MockClient module.exports.MockPool = MockPool module.exports.MockAgent = MockAgent module.exports.mockErrors = mockErrors - -const DnsCache = require('./lib/core/dns-cache') -module.exports.DnsCache = DnsCache diff --git a/lib/agent.js b/lib/agent.js index 320d7c473d8..f5cf0bda060 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -7,6 +7,7 @@ const Pool = require('./pool') const Client = require('./client') const util = require('./core/util') const createRedirectInterceptor = require('./interceptor/redirectInterceptor') +const DNSResolver = require('./core/dns-resolver') const kOnConnect = Symbol('onConnect') const kOnDisconnect = Symbol('onDisconnect') @@ -50,6 +51,7 @@ class Agent extends DispatcherBase { this[kOptions].interceptors = options.interceptors ? { ...options.interceptors } : undefined + this[kOptions].dnsResolver = options.dnsResolver?.disable ? undefined : new DNSResolver({ lookupOptions: options?.dnsResolver?.lookupOptions }) this[kMaxRedirections] = maxRedirections this[kFactory] = factory this[kClients] = new Map() diff --git a/lib/core/connect.js b/lib/core/connect.js index ce90428c609..2d2521dcfbd 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -4,7 +4,6 @@ const net = require('net') const assert = require('assert') const util = require('./util') const { InvalidArgumentError, ConnectTimeoutError } = require('./errors') -const { lookup, dnsCacheEnabled } = require('./dns-cache') let tls // include tls conditionally since it is not always available @@ -106,8 +105,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, ...o ALPNProtocols: allowH2 ? ['http/1.1', 'h2'] : ['http/1.1'], socket: httpSocket, // upgrade socket connection port: port || 443, - host: hostname, - lookup: dnsCacheEnabled() ? lookup : undefined + host: hostname }) socket @@ -122,8 +120,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, ...o ...options, localAddress, port: port || 80, - host: hostname, - lookup: dnsCacheEnabled() ? lookup : undefined + host: hostname }) } diff --git a/lib/core/dns-cache.js b/lib/core/dns-cache.js deleted file mode 100644 index 07227d3c052..00000000000 --- a/lib/core/dns-cache.js +++ /dev/null @@ -1,269 +0,0 @@ -const { - promises: dnsPromises, - lookup: _dnsLookup -} = require('node:dns') -const os = require('node:os') -const { promisify } = require('node:util') - -const { Resolver: AsyncResolver } = dnsPromises -const dnsLookup = promisify(_dnsLookup) - -const cache = new Map() -const _pending = new Map() -const hostnamesToFallback = new Set() - -const resolver = new AsyncResolver() -const maxTtl = Infinity -const fallbackDuration = 3600 -const errorTtl = 0.15 -let _nextRemovalTime = false -let _removalTimeout = null - -let cacheEnabled = true - -function enableCacheLookup () { - cacheEnabled = true -} - -function disableCacheLookup () { - cacheEnabled = false -} - -const getIfaceInfo = () => { - let has4 = false - let has6 = false - - for (const device of Object.values(os.networkInterfaces())) { - for (const iface of device) { - if (iface.internal) { - continue - } - - if (iface.family === 'IPv6') { - has6 = true - } else { - has4 = true - } - - if (has4 && has6) { - return { has4, has6 } - } - } - } - return { has4, has6 } -} - -function ignoreNoResultErrors (dnsPromise) { - return dnsPromise.catch(error => { - switch (error.code) { - case 'ENODATA': - case 'ENOTFOUND': - case 'ENOENT': - return [] - default: - throw error - } - }) -} - -const ttl = { ttl: true } -const all = { all: true } -const all4 = { all: true, family: 4 } -const all6 = { all: true, family: 6 } -const { has6 } = getIfaceInfo() - -function lookup (hostname, options, callback) { - if (typeof options === 'function') { - callback = options - options = {} - } else if (typeof options === 'number') { - options = { family: options } - } - - if (!callback) { - throw new Error('Function needs callback') - } - - lookupAsync(hostname).then((result) => { - // @TODO remove - console.log(result.address) - callback(null, result.address, result.family, result.expires, result.ttl) - }, callback) -} - -async function lookupAsync (hostname) { - const cached = await query(hostname) - let filtered = [] - - // IMOH (antoinegomez) we should always prioritize ipv6 if iface available and dns resolved - // @TODO better filter options to allow to pool through ipv6 + v4 - if (has6) { - filtered = cached.filter(entry => entry.family === 6) - - // if not v6 returned reset to the results, should be v4 only or empty - if (filtered.length === 0) { - filtered = cached - } - } else { - filtered = cached.filter(entry => entry.family === 4) - } - - if (filtered.length === 0) { - const error = new Error(`dnsLookup ENOTFOUND ${hostname}`) - error.code = 'ENOTFOUND' - error.hostname = hostname - throw error - } - - // return random result, better for balancing - // @TODO: keep track of usage or loop through filtered results instead? - return filtered[Math.floor(Math.random() * filtered.length)] -} - -async function query (hostname) { - let cached = cache.get(hostname) - - if (!cached || cacheEnabled === false) { - const pending = _pending.get(hostname) - if (pending) { - cached = await pending - } else { - const newPromise = queryAndCache(hostname) - _pending.set(hostname, newPromise) - try { - cached = await newPromise - } finally { - _pending.delete(hostname) - } - } - } - - return cached -} - -async function queryAndCache (hostname) { - if (hostnamesToFallback.has(hostname)) { - return dnsLookup(hostname, all) - } - - let query = await resolve(hostname) - - if (query.length === 0) { - query = await _lookup(hostname) - - if (query.entries.length !== 0 && fallbackDuration > 0) { - hostnamesToFallback(hostname) - } - } - - const cacheTtl = query.entries.length === 0 ? errorTtl : query.cacheTtl - set(hostname, query.entries, cacheTtl) - return query.entries -} - -async function _lookup (hostname) { - try { - const [A, AAAA] = await Promise.all([ - ignoreNoResultErrors(dnsLookup(hostname, all4)), - ignoreNoResultErrors(dnsLookup(hostname, all6)) - ]) - - return { - entries: [...AAAA, ...A], - cacheTtl: 0 - } - } catch { - return { - entries: [], - cacheTtl: 0 - } - } -} - -async function resolve (hostname) { - const [A, AAAA] = await Promise.all([ - ignoreNoResultErrors(resolver.resolve4(hostname, ttl)), - ignoreNoResultErrors(resolver.resolve6(hostname, ttl)) - ]) - - let cacheTtl = 0 - let aTtl = 0 - let aaaaTtl = 0 - - for (const entry of AAAA) { - entry.family = 6 - entry.expires = Date.now + entry.ttl * 1000 - aaaaTtl = Math.max(aaaaTtl, entry.ttl) - } - - for (const entry of A) { - entry.family = 4 - entry.expires = Date.now + entry.ttl * 1000 - aTtl = Math.max(aTtl, entry.ttl) - } - - if (A.length > 0) { - if (AAAA.length > 0) { - cacheTtl = Math.min(aTtl, aaaaTtl) - } else { - cacheTtl = aTtl - } - } else { - cacheTtl = aaaaTtl - } - - // favourite ipv6 if available - const result = [...AAAA, ...A] - return { entries: result, cacheTtl } -} - -function set (hostname, data, cacheTtl) { - if (maxTtl > 0 && cacheTtl > 0) { - cacheTtl = Math.min(cacheTtl, maxTtl) * 1000 - data.expires = Date.now() + cacheTtl - cache.set(hostname, data) - tick(cacheTtl) - } -} - -function tick (ms) { - const nextRemovalTime = _nextRemovalTime - - if (!nextRemovalTime || ms < nextRemovalTime) { - clearTimeout(_removalTimeout) - - _nextRemovalTime = ms - - _removalTimeout = setTimeout(() => { - _nextRemovalTime = false - - let nextExpiry = Infinity - - const now = Date.now() - - for (const [hostname, entries] of cache) { - const expires = entries.expires - - if (now >= expires) { - cache.delete(hostname) - } else if (expires < nextExpiry) { - nextExpiry = expires - } - } - - if (nextExpiry !== Infinity) { - tick(nextExpiry - now) - } - }, ms) - - /* istanbul ignore next: There is no `timeout.unref()` when running inside an Electron renderer */ - if (_removalTimeout.unref) { - _removalTimeout.unref() - } - } -} - -module.exports.lookup = lookup -module.exports.disableCacheLookup = disableCacheLookup -module.exports.enaleCacheLookup = enableCacheLookup -module.exports.dnsCacheEnabled = () => cacheEnabled diff --git a/lib/core/dns-resolver.js b/lib/core/dns-resolver.js new file mode 100644 index 00000000000..ee075325d19 --- /dev/null +++ b/lib/core/dns-resolver.js @@ -0,0 +1,437 @@ +// source: https://raw.githubusercontent.com/szmarczak/cacheable-lookup/9e60c9f6e74a003692aec68f3ddad93afe613b8f/source/index.mjs +const { + V4MAPPED, + ADDRCONFIG, + ALL, + promises: dnsPromises, + lookup: dnsLookup +} = require('node:dns') +const { promisify } = require('node:util') +const os = require('node:os') + +const { Resolver: AsyncResolver } = dnsPromises + +const kExpires = Symbol('expires') + +const supportsALL = typeof ALL === 'number' + +const map4to6 = (entries) => { + for (const entry of entries) { + if (entry.family === 6) { + continue + } + + entry.address = `::ffff:${entry.address}` + entry.family = 6 + } +} + +const getIfaceInfo = () => { + let has4 = false + let has6 = false + + for (const device of Object.values(os.networkInterfaces())) { + for (const iface of device) { + if (iface.internal) { + continue + } + + if (iface.family === 'IPv6') { + has6 = true + } else { + has4 = true + } + + if (has4 && has6) { + return { has4, has6 } + } + } + } + + return { has4, has6 } +} + +const isIterable = (map) => { + return Symbol.iterator in map +} + +const ignoreNoResultErrors = (dnsPromise) => { + return dnsPromise.catch((error) => { + if ( + error.code === 'ENODATA' || + error.code === 'ENOTFOUND' || + error.code === 'ENOENT' // Windows: name exists, but not this record type + ) { + return [] + } + + throw error + }) +} + +const ttl = { ttl: true } +const all = { all: true } +const all4 = { all: true, family: 4 } +const all6 = { all: true, family: 6 } + +class DNSResolver { + constructor ({ + cache = new Map(), + maxTtl = Infinity, + fallbackDuration = 3600, + errorTtl = 0.15, + resolver = new AsyncResolver(), + lookup = dnsLookup, + lookupOptions = undefined + } = {}) { + this.maxTtl = maxTtl + this.errorTtl = errorTtl + + this._cache = cache + this._resolver = resolver + this._dnsLookup = lookup && promisify(lookup) + this.stats = { + cache: 0, + query: 0 + } + + if (this._resolver instanceof AsyncResolver) { + this._resolve4 = this._resolver.resolve4.bind(this._resolver) + this._resolve6 = this._resolver.resolve6.bind(this._resolver) + } else { + this._resolve4 = promisify(this._resolver.resolve4.bind(this._resolver)) + this._resolve6 = promisify(this._resolver.resolve6.bind(this._resolver)) + } + + this._iface = getIfaceInfo() + + this._pending = {} + this._nextRemovalTime = false + this._hostnamesToFallback = new Set() + + this.fallbackDuration = fallbackDuration + + if (fallbackDuration > 0) { + const interval = setInterval(() => { + this._hostnamesToFallback.clear() + }, fallbackDuration * 1000) + + /* istanbul ignore next: There is no `interval.unref()` when running inside an Electron renderer */ + if (interval.unref) { + interval.unref() + } + + this._fallbackInterval = interval + } + + if (lookupOptions) { + this.lookup = (hostname, _options, callback) => this.__lookup(this, hostname, lookupOptions, callback) + } else { + this.lookup = this.__lookup.bind(this) + } + this.lookupAsync = this.lookupAsync.bind(this) + } + + set servers (servers) { + this.clear() + + this._resolver.setServers(servers) + } + + get servers () { + return this._resolver.getServers() + } + + __lookup (hostname, options, callback) { + if (typeof options === 'function') { + callback = options + options = {} + } else if (typeof options === 'number') { + options = { + family: options + } + } + + if (!callback) { + throw new Error('Callback must be a function.') + } + + // eslint-disable-next-line promise/prefer-await-to-then + this.lookupAsync(hostname, options).then((result) => { + if (options.all) { + callback(null, result) + } else { + callback( + null, + result.address, + result.family, + result.expires, + result.ttl, + result.source + ) + } + }, callback) + } + + async lookupAsync (hostname, options = {}) { + if (typeof options === 'number') { + options = { + family: options + } + } + + let cached = await this.query(hostname) + + /* + * @TODO not a huge fan of this, would need to know more on how to select + * the right range. + * need to read code source of dns lookup from nodejs to compare + * + * We should favour ipv6 when available + * But this will fail for localhost if server is listening on v4 only + * Curl is handling this well + */ + if (this._iface.has6 || options.family === 6) { + const filtered = cached.filter((entry) => entry.family === 6) + + if (options.hints & V4MAPPED) { + if ((supportsALL && options.hints & ALL) || filtered.length === 0) { + map4to6(cached) + } else { + cached = filtered + } + } else { + cached = filtered + } + } else if (options.family === 4) { + cached = cached.filter((entry) => entry.family === 4) + } + + if (options.hints & ADDRCONFIG) { + const { _iface } = this + cached = cached.filter((entry) => + entry.family === 6 ? _iface.has6 : _iface.has4 + ) + } + + if (cached.length === 0) { + const error = new Error(`cacheableLookup ENOTFOUND ${hostname}`) + error.code = 'ENOTFOUND' + error.hostname = hostname + + throw error + } + + if (options.all) { + return cached + } + + // @TODO go through each result in sequence to allow good load balancing + return cached[Math.floor(Math.random() * cached.length)] + } + + async query (hostname) { + let source = 'cache' + let cached = await this._cache.get(hostname) + + if (cached) { + this.stats.cache++ + } + + if (!cached) { + const pending = this._pending[hostname] + if (pending) { + this.stats.cache++ + cached = await pending + } else { + source = 'query' + const newPromise = this.queryAndCache(hostname) + this._pending[hostname] = newPromise + this.stats.query++ + try { + cached = await newPromise + } finally { + delete this._pending[hostname] + } + } + } + + cached = cached.map((entry) => { + return { ...entry, source } + }) + + return cached + } + + async _resolve (hostname) { + // ANY is unsafe as it doesn't trigger new queries in the underlying server. + const [A, AAAA] = await Promise.all([ + ignoreNoResultErrors(this._resolve4(hostname, ttl)), + ignoreNoResultErrors(this._resolve6(hostname, ttl)) + ]) + + let aTtl = 0 + let aaaaTtl = 0 + let cacheTtl = 0 + + const now = Date.now() + + for (const entry of A) { + entry.family = 4 + entry.expires = now + entry.ttl * 1000 + + aTtl = Math.max(aTtl, entry.ttl) + } + + for (const entry of AAAA) { + entry.family = 6 + entry.expires = now + entry.ttl * 1000 + + aaaaTtl = Math.max(aaaaTtl, entry.ttl) + } + + if (A.length > 0) { + if (AAAA.length > 0) { + cacheTtl = Math.min(aTtl, aaaaTtl) + } else { + cacheTtl = aTtl + } + } else { + cacheTtl = aaaaTtl + } + + return { + entries: [...A, ...AAAA], + cacheTtl + } + } + + async _lookup (hostname) { + try { + const [A, AAAA] = await Promise.all([ + // Passing {all: true} doesn't return all IPv4 and IPv6 entries. + // See https://github.com/szmarczak/cacheable-lookup/issues/42 + ignoreNoResultErrors(this._dnsLookup(hostname, all4)), + ignoreNoResultErrors(this._dnsLookup(hostname, all6)) + ]) + + return { + entries: [...A, ...AAAA], + cacheTtl: 0 + } + } catch { + return { + entries: [], + cacheTtl: 0 + } + } + } + + async _set (hostname, data, cacheTtl) { + if (this.maxTtl > 0 && cacheTtl > 0) { + cacheTtl = Math.min(cacheTtl, this.maxTtl) * 1000 + data[kExpires] = Date.now() + cacheTtl + + try { + await this._cache.set(hostname, data, cacheTtl) + } catch (error) { + this.lookupAsync = async () => { + const cacheError = new Error( + 'Cache Error. Please recreate the CacheableLookup instance.' + ) + cacheError.cause = error + + throw cacheError + } + } + + if (isIterable(this._cache)) { + this._tick(cacheTtl) + } + } + } + + async queryAndCache (hostname) { + if (this._hostnamesToFallback.has(hostname)) { + return this._dnsLookup(hostname, all) + } + + let query = await this._resolve(hostname) + + if (query.entries.length === 0 && this._dnsLookup) { + query = await this._lookup(hostname) + + if (query.entries.length !== 0 && this.fallbackDuration > 0) { + // Use `dns.lookup(...)` for that particular hostname + this._hostnamesToFallback.add(hostname) + } + } + + const cacheTtl = query.entries.length === 0 ? this.errorTtl : query.cacheTtl + await this._set(hostname, query.entries, cacheTtl) + + return query.entries + } + + _tick (ms) { + const nextRemovalTime = this._nextRemovalTime + + if (!nextRemovalTime || ms < nextRemovalTime) { + clearTimeout(this._removalTimeout) + + this._nextRemovalTime = ms + + this._removalTimeout = setTimeout(() => { + this._nextRemovalTime = false + + let nextExpiry = Infinity + + const now = Date.now() + + for (const [hostname, entries] of this._cache) { + const expires = entries[kExpires] + + if (now >= expires) { + this._cache.delete(hostname) + } else if (expires < nextExpiry) { + nextExpiry = expires + } + } + + if (nextExpiry !== Infinity) { + this._tick(nextExpiry - now) + } + }, ms) + + /* istanbul ignore next: There is no `timeout.unref()` when running inside an Electron renderer */ + if (this._removalTimeout.unref) { + this._removalTimeout.unref() + } + } + } + + // @TODO useless for now as its not exposed + updateInterfaceInfo () { + const { _iface } = this + + this._iface = getIfaceInfo() + + if ( + (_iface.has4 && !this._iface.has4) || + (_iface.has6 && !this._iface.has6) + ) { + this._cache.clear() + } + } + + clear (hostname) { + if (hostname) { + this._cache.delete(hostname) + return + } + + this._cache.clear() + } +} + +module.exports = DNSResolver diff --git a/lib/pool.js b/lib/pool.js index e3cd3399e6b..9a04178347b 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -35,6 +35,7 @@ class Pool extends PoolBase { autoSelectFamily, autoSelectFamilyAttemptTimeout, allowH2, + dnsResolver, ...options } = {}) { super() @@ -57,6 +58,7 @@ class Pool extends PoolBase { maxCachedSessions, allowH2, socketPath, + lookup: dnsResolver ? dnsResolver.lookup : undefined, timeout: connectTimeout, ...(util.nodeHasAutoSelectFamily && autoSelectFamily ? { autoSelectFamily, autoSelectFamilyAttemptTimeout } : undefined), ...connect From dffb51026780512fa8612352ba19ba46296634b8 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 15:49:36 +0000 Subject: [PATCH 04/19] use Promise.allSettled --- lib/core/dns-resolver.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/core/dns-resolver.js b/lib/core/dns-resolver.js index ee075325d19..20910034e4e 100644 --- a/lib/core/dns-resolver.js +++ b/lib/core/dns-resolver.js @@ -125,7 +125,7 @@ class DNSResolver { } if (lookupOptions) { - this.lookup = (hostname, _options, callback) => this.__lookup(this, hostname, lookupOptions, callback) + this.lookup = (hostname, _options, callback) => this.__lookup(hostname, lookupOptions, callback) } else { this.lookup = this.__lookup.bind(this) } @@ -265,11 +265,14 @@ class DNSResolver { async _resolve (hostname) { // ANY is unsafe as it doesn't trigger new queries in the underlying server. - const [A, AAAA] = await Promise.all([ + const entries = await Promise.allSettled([ ignoreNoResultErrors(this._resolve4(hostname, ttl)), ignoreNoResultErrors(this._resolve6(hostname, ttl)) ]) + const A = entries[0].status === 'fulfilled' ? entries[0].value : [] + const AAAA = entries[1].status === 'fulfilled' ? entries[1].value : [] + let aTtl = 0 let aaaaTtl = 0 let cacheTtl = 0 @@ -301,22 +304,25 @@ class DNSResolver { } return { - entries: [...A, ...AAAA], + entries: [...AAAA, ...A], cacheTtl } } async _lookup (hostname) { try { - const [A, AAAA] = await Promise.all([ + const entries = await Promise.allSettled([ // Passing {all: true} doesn't return all IPv4 and IPv6 entries. // See https://github.com/szmarczak/cacheable-lookup/issues/42 ignoreNoResultErrors(this._dnsLookup(hostname, all4)), ignoreNoResultErrors(this._dnsLookup(hostname, all6)) ]) + const A = entries[0].status === 'fulfilled' ? entries[0].value : [] + const AAAA = entries[1].status === 'fulfilled' ? entries[1].value : [] + return { - entries: [...A, ...AAAA], + entries: [...AAAA, ...A], cacheTtl: 0 } } catch { From 0b0ca320a48f7bbcf37b202bcc22fdfb838aa2cf Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 16:55:18 +0000 Subject: [PATCH 05/19] fix ipv6 --- lib/core/dns-resolver.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/lib/core/dns-resolver.js b/lib/core/dns-resolver.js index 20910034e4e..b7255a7f02d 100644 --- a/lib/core/dns-resolver.js +++ b/lib/core/dns-resolver.js @@ -182,16 +182,7 @@ class DNSResolver { let cached = await this.query(hostname) - /* - * @TODO not a huge fan of this, would need to know more on how to select - * the right range. - * need to read code source of dns lookup from nodejs to compare - * - * We should favour ipv6 when available - * But this will fail for localhost if server is listening on v4 only - * Curl is handling this well - */ - if (this._iface.has6 || options.family === 6) { + if (options.family === 6) { const filtered = cached.filter((entry) => entry.family === 6) if (options.hints & V4MAPPED) { From 9a8ea6ee0b119fd47c574430052e4ae7fbb5c507 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 17:43:17 +0000 Subject: [PATCH 06/19] slight refactor --- index.js | 3 +++ lib/agent.js | 10 +++++++++- lib/core/dns-resolver.js | 27 ++++++++++++++++----------- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index 5fc0d9727ef..4c634ccc36d 100644 --- a/index.js +++ b/index.js @@ -165,3 +165,6 @@ module.exports.MockClient = MockClient module.exports.MockPool = MockPool module.exports.MockAgent = MockAgent module.exports.mockErrors = mockErrors + +const DNSResolver = require('./lib/core/dns-resolver') +module.exports.DNSResolver = DNSResolver diff --git a/lib/agent.js b/lib/agent.js index f5cf0bda060..a8252db5be4 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -23,6 +23,14 @@ function defaultFactory (origin, opts) { : new Pool(origin, opts) } +function getDnsResolver (options = {}) { + if (options.DNSResolver instanceof DNSResolver) { + return options.DNSResolver + } + + return options?.dnsResolver?.disable ? undefined : new DNSResolver(options?.dnsResolver) +} + class Agent extends DispatcherBase { constructor ({ factory = defaultFactory, maxRedirections = 0, connect, ...options } = {}) { super() @@ -51,7 +59,7 @@ class Agent extends DispatcherBase { this[kOptions].interceptors = options.interceptors ? { ...options.interceptors } : undefined - this[kOptions].dnsResolver = options.dnsResolver?.disable ? undefined : new DNSResolver({ lookupOptions: options?.dnsResolver?.lookupOptions }) + this[kOptions].dnsResolver = getDnsResolver(options) this[kMaxRedirections] = maxRedirections this[kFactory] = factory this[kClients] = new Map() diff --git a/lib/core/dns-resolver.js b/lib/core/dns-resolver.js index b7255a7f02d..9f0a70e649d 100644 --- a/lib/core/dns-resolver.js +++ b/lib/core/dns-resolver.js @@ -12,6 +12,7 @@ const os = require('node:os') const { Resolver: AsyncResolver } = dnsPromises const kExpires = Symbol('expires') +const roundRobinStrategies = ['first', 'random'] const supportsALL = typeof ALL === 'number' @@ -82,11 +83,18 @@ class DNSResolver { errorTtl = 0.15, resolver = new AsyncResolver(), lookup = dnsLookup, - lookupOptions = undefined + lookupOptions = undefined, + roundRobinStrategy = 'first' } = {}) { this.maxTtl = maxTtl this.errorTtl = errorTtl + if (roundRobinStrategies.includes(roundRobinStrategy) === false) { + throw new Error(`roundRobinStrategy must be one of: ${roundRobinStrategies.join(', ')}`) + } + + this.roundRobinStrategy = roundRobinStrategy + this._cache = cache this._resolver = resolver this._dnsLookup = lookup && promisify(lookup) @@ -166,8 +174,7 @@ class DNSResolver { result.address, result.family, result.expires, - result.ttl, - result.source + result.ttl ) } }, callback) @@ -217,12 +224,15 @@ class DNSResolver { return cached } - // @TODO go through each result in sequence to allow good load balancing - return cached[Math.floor(Math.random() * cached.length)] + if (this.roundRobinStrategy === 'first') { + return cached[0] + } else { + // random + return cached[Math.floor(Math.random() * cached.length)] + } } async query (hostname) { - let source = 'cache' let cached = await this._cache.get(hostname) if (cached) { @@ -235,7 +245,6 @@ class DNSResolver { this.stats.cache++ cached = await pending } else { - source = 'query' const newPromise = this.queryAndCache(hostname) this._pending[hostname] = newPromise this.stats.query++ @@ -247,10 +256,6 @@ class DNSResolver { } } - cached = cached.map((entry) => { - return { ...entry, source } - }) - return cached } From 14e986b5eef7b700afc7213f9111a21d9bd97915 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Thu, 28 Dec 2023 21:18:19 +0000 Subject: [PATCH 07/19] add tests --- lib/core/dns-resolver.js | 8 +- test/dns-resolver.js | 935 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 941 insertions(+), 2 deletions(-) create mode 100644 test/dns-resolver.js diff --git a/lib/core/dns-resolver.js b/lib/core/dns-resolver.js index 9f0a70e649d..e497b7856df 100644 --- a/lib/core/dns-resolver.js +++ b/lib/core/dns-resolver.js @@ -213,7 +213,7 @@ class DNSResolver { } if (cached.length === 0) { - const error = new Error(`cacheableLookup ENOTFOUND ${hostname}`) + const error = new Error(`DNSResolver ENOTFOUND ${hostname}`) error.code = 'ENOTFOUND' error.hostname = hostname @@ -266,6 +266,10 @@ class DNSResolver { ignoreNoResultErrors(this._resolve6(hostname, ttl)) ]) + if (entries[0].status === 'rejected' && entries[1].status === 'rejected') { + throw entries[0].reason + } + const A = entries[0].status === 'fulfilled' ? entries[0].value : [] const AAAA = entries[1].status === 'fulfilled' ? entries[1].value : [] @@ -339,7 +343,7 @@ class DNSResolver { } catch (error) { this.lookupAsync = async () => { const cacheError = new Error( - 'Cache Error. Please recreate the CacheableLookup instance.' + 'Cache Error. Please recreate the DNSResolver instance.' ) cacheError.cause = error diff --git a/test/dns-resolver.js b/test/dns-resolver.js new file mode 100644 index 00000000000..8bedf3bd7bf --- /dev/null +++ b/test/dns-resolver.js @@ -0,0 +1,935 @@ +const { promises: dnsPromises, V4MAPPED, ADDRCONFIG, ALL } = require('node:dns') +const { promisify } = require('node:util') +const http = require('node:http') +const { test } = require('tap') +const originalDns = require('node:dns') +const proxyquire = require('proxyquire') +const osStub = {} +const dnsStub = { + ...originalDns +} + +const { Resolver: AsyncResolver } = dnsPromises + +const makeRequest = (options) => + new Promise((resolve, reject) => { + http.get(options, resolve).once('error', reject) + }) + +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)) + +const mockedInterfaces = async (options) => { + const createInterfaces = (options = {}) => { + const interfaces = { + lo: [ + { + internal: true + } + ], + eth0: [] + } + + if (options.has4) { + interfaces.eth0.push({ + address: '192.168.0.111', + netmask: '255.255.255.0', + family: 'IPv4', + mac: '00:00:00:00:00:00', + internal: false, + cidr: '192.168.0.111/24' + }) + } + + if (options.has6) { + interfaces.eth0.push({ + address: 'fe80::c962:2946:a4e2:9f05', + netmask: 'ffff:ffff:ffff:ffff::', + family: 'IPv6', + mac: '00:00:00:00:00:00', + scopeid: 8, + internal: false, + cidr: 'fe80::c962:2946:a4e2:9f05/64' + }) + } + + return interfaces + } + + let interfaces = createInterfaces(options) + + const _updateInterfaces = (options = {}) => { + interfaces = createInterfaces(options) + } + + osStub.networkInterfaces = function () { + return interfaces + } + + DNSResolver._updateInterfaces = _updateInterfaces + return DNSResolver +} + +const createResolver = () => { + let counter = { + 4: 0, + 6: 0, + lookup: 0 + } + + const resolver = { + servers: ['127.0.0.1'], + getServers () { + return [...resolver.servers] + }, + setServers (servers) { + resolver.servers = [...servers] + }, + resolve: (hostname, options, callback) => { + let data + for (const server of resolver.servers) { + if (resolver.data[server][hostname]) { + data = resolver.data[server][hostname] + break + } + } + + if (hostname === 'econnrefused') { + const error = new Error(`ECONNREFUSED ${hostname}`) + error.code = 'ECONNREFUSED' + + callback(error) + return + } + + if (!data) { + const error = new Error(`ENOTFOUND ${hostname}`) + error.code = 'ENOTFOUND' + + callback(error) + return + } + + if (data.length === 0) { + const error = new Error(`ENODATA ${hostname}`) + error.code = 'ENODATA' + + callback(error) + return + } + + if (options.family === 4 || options.family === 6) { + data = data.filter((entry) => entry.family === options.family) + } + + callback(null, JSON.parse(JSON.stringify(data))) + }, + resolve4: (hostname, options, callback) => { + counter[4]++ + + return resolver.resolve(hostname, { ...options, family: 4 }, callback) + }, + resolve6: (hostname, options, callback) => { + counter[6]++ + + return resolver.resolve(hostname, { ...options, family: 6 }, callback) + }, + lookup: (hostname, options, callback) => { + // No need to implement hints yet + + counter.lookup++ + + if (!resolver.lookupData[hostname]) { + const error = new Error(`ENOTFOUND ${hostname}`) + error.code = 'ENOTFOUND' + error.hostname = hostname + + callback(error) + return + } + + let entries = resolver.lookupData[hostname] + + if (options.family === 4 || options.family === 6) { + entries = entries.filter((entry) => entry.family === options.family) + } + + if (options.all) { + callback(null, entries) + return + } + + callback(null, entries[0]) + }, + data: { + '127.0.0.1': { + localhost: [ + { address: '127.0.0.1', family: 4, ttl: 60 }, + { address: '::ffff:127.0.0.2', family: 6, ttl: 60 } + ], + example: [{ address: '127.0.0.127', family: 4, ttl: 60 }], + temporary: [{ address: '127.0.0.1', family: 4, ttl: 1 }], + twoSeconds: [{ address: '127.0.0.1', family: 4, ttl: 2 }], + ttl: [{ address: '127.0.0.1', family: 4, ttl: 1 }], + maxTtl: [{ address: '127.0.0.1', family: 4, ttl: 60 }], + static4: [{ address: '127.0.0.1', family: 4, ttl: 1 }], + zeroTtl: [{ address: '127.0.0.127', family: 4, ttl: 0 }], + multiple: [ + { address: '127.0.0.127', family: 4, ttl: 0 }, + { address: '127.0.0.128', family: 4, ttl: 0 } + ], + outdated: [{ address: '127.0.0.1', family: 4, ttl: 1 }] + }, + '192.168.0.100': { + unique: [{ address: '127.0.0.1', family: 4, ttl: 60 }] + } + }, + lookupData: { + osHostname: [ + { address: '127.0.0.1', family: 4 }, + { address: '127.0.0.2', family: 4 } + ], + outdated: [{ address: '127.0.0.127', family: 4 }] + }, + get counter () { + return counter + }, + resetCounter () { + counter = { + 4: 0, + 6: 0, + lookup: 0 + } + } + } + + return resolver +} + +const resolver = createResolver() +dnsStub.lookup = resolver.lookup +const DNSResolver = proxyquire('../lib/core/dns-resolver', { + 'node:os': osStub, + 'node:dns': dnsStub +}) +// await quibble.esm('node:dns', dnsExports, dnsExports) +// +// const DNSResolver = require('../lib/core/dns-resolver') + +const verify = (t, entry, value) => { + if (Array.isArray(value)) { + // eslint-disable-next-line guard-for-in + for (const key in value) { + t.equal( + typeof entry[key].expires === 'number' && + entry[key].expires >= Date.now() - 1000, + true + ) + t.equal(typeof entry[key].ttl === 'number' && entry[key].ttl >= 0, true) + + if (!('ttl' in value[key]) && 'ttl' in entry[key]) { + value[key].ttl = entry[key].ttl + } + + if (!('expires' in value[key]) && 'expires' in entry[key]) { + value[key].expires = entry[key].expires + } + } + } else { + t.equal( + typeof entry.expires === 'number' && entry.expires >= Date.now() - 1000, + true + ) + t.equal(typeof entry.ttl === 'number' && entry.ttl >= 0, true) + + if (!('ttl' in value)) { + value.ttl = entry.ttl + } + + if (!('expires' in value)) { + value.expires = entry.expires + } + } + + t.same(entry, value) +} + +test('options.family', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // IPv4 + let entry = await cacheable.lookupAsync('localhost', { family: 4 }) + verify(t, entry, { + address: '127.0.0.1', + family: 4 + }) + + // IPv6 + entry = await cacheable.lookupAsync('localhost', { family: 6 }) + verify(t, entry, { + address: '::ffff:127.0.0.2', + family: 6 + }) +}) + +test('options.all', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + const entries = await cacheable.lookupAsync('localhost', { all: true }) + verify(t, entries, [ + { address: '::ffff:127.0.0.2', family: 6 }, + { address: '127.0.0.1', family: 4 } + ]) +}) + +test('options.all mixed with options.family', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // IPv4 + let entries = await cacheable.lookupAsync('localhost', { + all: true, + family: 4 + }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + + // IPv6 + entries = await cacheable.lookupAsync('localhost', { all: true, family: 6 }) + verify(t, entries, [{ address: '::ffff:127.0.0.2', family: 6 }]) +}) + +test('V4MAPPED hint', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // Make sure default behavior is right + await t.rejects(cacheable.lookupAsync('static4', { family: 6 }), { + code: 'ENOTFOUND' + }) + + // V4MAPPED + { + const entries = await cacheable.lookupAsync('static4', { + family: 6, + hints: V4MAPPED + }) + verify(t, entries, { address: '::ffff:127.0.0.1', family: 6 }) + } + + { + const entries = await cacheable.lookupAsync('localhost', { + family: 6, + hints: V4MAPPED + }) + verify(t, entries, { address: '::ffff:127.0.0.2', family: 6 }) + } +}) + +if (process.versions.node.split('.')[0] >= 14) { + test('ALL hint', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // ALL + const entries = await cacheable.lookupAsync('localhost', { + family: 6, + hints: V4MAPPED | ALL, + all: true + }) + + verify(t, entries, [ + { address: '::ffff:127.0.0.2', family: 6, ttl: 60 }, + { address: '::ffff:127.0.0.1', family: 6, ttl: 60 } + ]) + }) +} + +test('ADDRCONFIG hint', async (t) => { + // => has6 = false, family = 6 + { + const DNSResolver = await mockedInterfaces({ has4: true, has6: false }) + const cacheable = new DNSResolver({ resolver }) + + await t.rejects( + cacheable.lookupAsync('localhost', { family: 6, hints: ADDRCONFIG }), + { code: 'ENOTFOUND' } + ) + } + + // => has6 = true, family = 6 + { + const DNSResolver = await mockedInterfaces({ has4: true, has6: true }) + const cacheable = new DNSResolver({ resolver }) + + verify( + t, + await cacheable.lookupAsync('localhost', { + family: 6, + hints: ADDRCONFIG + }), + { + address: '::ffff:127.0.0.2', + family: 6 + } + ) + } + + // => has4 = false, family = 4 + { + const DNSResolver = await mockedInterfaces({ has4: false, has6: true }) + const cacheable = new DNSResolver({ resolver }) + + await t.rejects( + cacheable.lookupAsync('localhost', { family: 4, hints: ADDRCONFIG }), + { code: 'ENOTFOUND' } + ) + } + + // => has4 = true, family = 4 + { + const DNSResolver = await mockedInterfaces({ has4: true, has6: true }) + const cacheable = new DNSResolver({ resolver }) + + verify( + t, + await cacheable.lookupAsync('localhost', { + family: 4, + hints: ADDRCONFIG + }), + { + address: '127.0.0.1', + family: 4 + } + ) + } + + // Update interface info + { + const DNSResolver = await mockedInterfaces({ has4: false, has6: true }) + const cacheable = new DNSResolver({ resolver }) + + await t.rejects( + cacheable.lookupAsync('localhost', { family: 4, hints: ADDRCONFIG }), + { code: 'ENOTFOUND' } + ) + + // => has4 = true, family = 4 + DNSResolver._updateInterfaces({ has4: true, has6: true }) // Override os.networkInterfaces() + cacheable.updateInterfaceInfo() + + verify( + t, + await cacheable.lookupAsync('localhost', { + family: 4, + hints: ADDRCONFIG + }), + { + address: '127.0.0.1', + family: 4 + } + ) + } +}) + +test('caching works', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // Make sure default behavior is right + let entries = await cacheable.lookupAsync('temporary', { + all: true, + family: 4 + }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + + // Update DNS data + const resovlerEntry = resolver.data['127.0.0.1'].temporary[0] + const { address: resolverAddress } = resovlerEntry + resovlerEntry.address = '127.0.0.2' + + // Lookup again returns cached data + entries = await cacheable.lookupAsync('temporary', { all: true, family: 4 }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + + // Restore back + resovlerEntry.address = resolverAddress +}) + +test('respects ttl', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // Make sure default behavior is right + let entries = await cacheable.lookupAsync('ttl', { all: true, family: 4 }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + + // Update DNS data + const resolverEntry = resolver.data['127.0.0.1'].ttl[0] + const { address: resolverAddress } = resolverEntry + resolverEntry.address = '127.0.0.2' + + // Wait until it expires + await sleep(resolverEntry.ttl * 1000 + 1) + + // Lookup again + entries = await cacheable.lookupAsync('ttl', { all: true, family: 4 }) + verify(t, entries, [{ address: '127.0.0.2', family: 4 }]) + + // Restore back + resolverEntry.address = resolverAddress +}) + +test('throw when there are entries available but not for the requested family', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + await t.rejects(cacheable.lookupAsync('static4', { family: 6 }), { + code: 'ENOTFOUND' + }) +}) + +test('custom servers', async (t) => { + const cacheable = new DNSResolver({ resolver: createResolver() }) + + // .servers (get) + t.same(cacheable.servers, ['127.0.0.1']) + await t.rejects(cacheable.lookupAsync('unique'), { code: 'ENOTFOUND' }) + + // .servers (set) + cacheable.servers = ['127.0.0.1', '192.168.0.100'] + verify(t, await cacheable.lookupAsync('unique'), { + address: '127.0.0.1', + family: 4 + }) + + // Verify + t.same(cacheable.servers, ['127.0.0.1', '192.168.0.100']) +}) + +test('callback style', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // Custom promise for this particular test + const lookup = (...args) => + new Promise((resolve, reject) => { + cacheable.lookup(...args, (error, ...data) => { + if (error) { + reject(error) + } else { + resolve(data) + } + }) + }) + + // Without options + let result = await lookup('localhost') + t.equal(result.length, 4) + t.equal(result[0], '::ffff:127.0.0.2') + t.equal(result[1], 6) + t.equal(typeof result[2] === 'number' && result[2] >= Date.now() - 1000, true) + t.equal(typeof result[3] === 'number' && result[3] >= 0, true) + + // With options + result = await lookup('localhost', { family: 4, all: true }) + t.equal(result.length, 1) + verify(t, result[0], [{ address: '127.0.0.1', family: 4 }]) +}) + +test('works', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + verify(t, await cacheable.lookupAsync('localhost'), { + address: '::ffff:127.0.0.2', + family: 6 + }) +}) + +test('options.maxTtl', async (t) => { + // => maxTtl = 1 + { + const cacheable = new DNSResolver({ resolver, maxTtl: 1 }) + + // Make sure default behavior is right + verify(t, await cacheable.lookupAsync('maxTtl'), { + address: '127.0.0.1', + family: 4 + }) + + // Update DNS data + const resolverEntry = resolver.data['127.0.0.1'].maxTtl[0] + resolverEntry.address = '127.0.0.2' + + // Wait until it expires + await sleep(cacheable.maxTtl * 1000 + 1) + + // Lookup again + verify(t, await cacheable.lookupAsync('maxTtl'), { + address: '127.0.0.2', + family: 4 + }) + + // Reset + resolverEntry.address = '127.0.0.1' + } + + // => maxTtl = 0 + { + const cacheable = new DNSResolver({ resolver, maxTtl: 0 }) + + // Make sure default behavior is right + verify(t, await cacheable.lookupAsync('maxTtl'), { + address: '127.0.0.1', + family: 4 + }) + + // Update DNS data + const resolverEntry = resolver.data['127.0.0.1'].maxTtl[0] + resolverEntry.address = '127.0.0.2' + + // Wait until it expires + await sleep(cacheable.maxTtl * 1000 + 1) + + // Lookup again + verify(t, await cacheable.lookupAsync('maxTtl'), { + address: '127.0.0.2', + family: 4 + }) + + // Reset + resolverEntry.address = '127.0.0.1' + } +}) + +test('entry with 0 ttl', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + // Make sure default behavior is right + verify(t, await cacheable.lookupAsync('zeroTtl'), { + address: '127.0.0.127', + family: 4 + }) + + // Update DNS data + resolver.data['127.0.0.1'].zeroTtl[0].address = '127.0.0.1' + + // Lookup again + verify(t, await cacheable.lookupAsync('zeroTtl'), { + address: '127.0.0.1', + family: 4 + }) +}) + +test('http example', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + const options = { + hostname: 'example', + port: 9999, + lookup: cacheable.lookup + } + + await t.rejects(makeRequest(options), { + message: 'connect ECONNREFUSED 127.0.0.127:9999' + }) +}) + +test('.lookup() and .lookupAsync() are automatically bounded', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + await t.resolves(cacheable.lookupAsync('localhost')) + await t.resolves(promisify(cacheable.lookup)('localhost')) + + t.throws(() => cacheable.lookup('localhost'), { + message: 'Callback must be a function.' + }) +}) + +test('works (Internet connection)', async (t) => { + const cacheable = new DNSResolver() + + const { address, family } = await cacheable.lookupAsync( + '1dot1dot1dot1.cloudflare-dns.com', + { family: 4 } + ) + t.equal(address === '1.1.1.1' || address === '1.0.0.1', true) + t.equal(family, 4) +}) + +test('async resolver (Internet connection)', async (t) => { + const cacheable = new DNSResolver({ resolver: new AsyncResolver() }) + + t.equal(typeof cacheable._resolve4, 'function') + t.equal(typeof cacheable._resolve6, 'function') + + const { address } = await cacheable.lookupAsync( + '1dot1dot1dot1.cloudflare-dns.com', + { family: 4 } + ) + t.equal(address === '1.1.1.1' || address === '1.0.0.1', true) +}) + +test('clear() works', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + await cacheable.lookupAsync('localhost') + t.equal(cacheable._cache.size, 1) + + cacheable.clear() + + t.equal(cacheable._cache.size, 0) +}) + +test('ttl works', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + await Promise.all([ + cacheable.lookupAsync('temporary'), + cacheable.lookupAsync('ttl') + ]) + t.equal(cacheable._cache.size, 2) + + await sleep(2001) + + t.equal(cacheable._cache.size, 0) +}) + +test('fallback works', async (t) => { + const cacheable = new DNSResolver({ resolver, fallbackDuration: 0.1 }) + resolver.resetCounter() + + const entries = await cacheable.lookupAsync('osHostname', { all: true }) + t.equal(entries.length, 2) + + t.equal(entries[0].address, '127.0.0.1') + t.equal(entries[0].family, 4) + + t.equal(entries[1].address, '127.0.0.2') + t.equal(entries[1].family, 4) + + t.equal(cacheable._cache.size, 0) + + await cacheable.lookupAsync('osHostname', { all: true }) + + t.same(resolver.counter, { + 6: 1, + 4: 1, + lookup: 3 + }) + + await sleep(100) + + t.equal(cacheable._hostnamesToFallback.size, 0) +}) + +test('fallback works if ip change', async (t) => { + const cacheable = new DNSResolver({ resolver, fallbackDuration: 3600 }) + resolver.resetCounter() + resolver.lookupData.osHostnameChange = [ + { address: '127.0.0.1', family: 4 }, + { address: '127.0.0.2', family: 4 } + ] + + // First call: do not enter in `if (this._hostnamesToFallback.has(hostname)) {` + const entries = await cacheable.query('osHostnameChange', { all: true }) + t.equal(entries.length, 2) + + t.equal(entries[0].address, '127.0.0.1') + t.equal(entries[0].family, 4) + + t.equal(entries[1].address, '127.0.0.2') + t.equal(entries[1].family, 4) + + t.equal(cacheable._cache.size, 0) + + // Second call: enter in `if (this._hostnamesToFallback.has(hostname)) {` + // And use _dnsLookup + // This call is used to ensure that this._pending is cleaned up when the promise is resolved + await cacheable.query('osHostnameChange', { all: true }) + + // Third call: enter in `if (this._hostnamesToFallback.has(hostname)) {` + // And use _dnsLookup + // Address should be different + resolver.lookupData.osHostnameChange = [ + { address: '127.0.0.3', family: 4 }, + { address: '127.0.0.4', family: 4 } + ] + const entries2 = await cacheable.query('osHostnameChange', { all: true }) + + t.equal(entries2.length, 2) + + t.equal(entries2[0].address, '127.0.0.3') + t.equal(entries2[0].family, 4) + + t.equal(entries2[1].address, '127.0.0.4') + t.equal(entries2[1].family, 4) + + t.equal(cacheable._cache.size, 0) + + delete resolver.lookupData.osHostnameChange +}) + +test('real DNS queries first', async (t) => { + const resolver = createResolver({ delay: 0 }) + const cacheable = new DNSResolver({ + resolver, + fallbackDuration: 3600, + lookup: resolver.lookup + }) + + { + const entries = await cacheable.lookupAsync('outdated', { all: true }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + } + + await new Promise((resolve) => setTimeout(resolve, 100)) + + { + const entries = await cacheable.lookupAsync('outdated', { all: true }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + } +}) + +test('fallback can be turned off', async (t) => { + const cacheable = new DNSResolver({ resolver, lookup: false }) + + await t.rejects(cacheable.lookupAsync('osHostname', { all: true }), { + message: 'DNSResolver ENOTFOUND osHostname' + }) +}) + +test('errors are cached', async (t) => { + const cacheable = new DNSResolver({ resolver, errorTtl: 0.1 }) + + await t.rejects(cacheable.lookupAsync('doesNotExist'), { + code: 'ENOTFOUND' + }) + + t.equal(cacheable._cache.size, 1) + + await sleep(cacheable.errorTtl * 1000 + 1) + + t.equal(cacheable._cache.size, 0) +}) + +test('passing family as options', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + const promisified = promisify(cacheable.lookup) + + const entry = await cacheable.lookupAsync('localhost', 6) + t.equal(entry.address, '::ffff:127.0.0.2') + t.equal(entry.family, 6) + + const address = await promisified('localhost', 6) + t.equal(address, '::ffff:127.0.0.2') +}) + +test('clear(hostname) works', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + await cacheable.lookupAsync('localhost') + await cacheable.lookupAsync('temporary') + + cacheable.clear('localhost') + + t.equal(cacheable._cache.size, 1) +}) + +test('prevents overloading DNS', async (t) => { + const resolver = createResolver() + const { lookupAsync } = new DNSResolver({ + resolver, + lookup: resolver.lookup + }) + + await Promise.all([lookupAsync('localhost'), lookupAsync('localhost')]) + + t.same(resolver.counter, { + 4: 1, + 6: 1, + lookup: 0 + }) +}) + +test('returns IPv6 if no other entries available', async (t) => { + const DNSResolver = await mockedInterfaces({ has4: false, has6: true }) + const cacheable = new DNSResolver({ resolver }) + + verify(t, await cacheable.lookupAsync('localhost', { hints: ADDRCONFIG }), { + address: '::ffff:127.0.0.2', + family: 6 + }) +}) + +test('throws when no internet connection', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + await t.rejects(cacheable.lookupAsync('econnrefused'), { + code: 'ECONNREFUSED' + }) +}) + +test('throws when the cache instance is broken', async (t) => { + const cacheable = new DNSResolver({ + resolver, + cache: { + get: () => {}, + set: () => { + throw new Error('Something broke.') + } + } + }) + + await t.resolves(cacheable.lookupAsync('localhost')) + + await t.rejects(cacheable.lookupAsync('localhost'), { + message: 'Cache Error. Please recreate the DNSResolver instance.' + }) + + // not supported by this tap version + // t.equal(error.cause.message, 'Something broke.') +}) + +test('slow dns.lookup', async (t) => { + const cacheable = new DNSResolver({ + resolver, + lookup: (hostname, options, callback) => { + t.equal(hostname, 'osHostname') + t.equal(options.all, true) + t.equal(options.family === 4 || options.family === 6, true) + + setTimeout(() => { + if (options.family === 4) { + callback(null, [{ address: '127.0.0.1', family: 4 }]) + } + + if (options.family === 6) { + callback(null, [{ address: '::1', family: 6 }]) + } + }, 10) + } + }) + + const entry = await cacheable.lookupAsync('osHostname', 4) + + t.same(entry, { + address: '127.0.0.1', + family: 4 + }) +}) + +test('cache and query stats', async (t) => { + const cacheable = new DNSResolver({ resolver }) + + t.equal(cacheable.stats.query, 0) + t.equal(cacheable.stats.cache, 0) + + let entries = await cacheable.lookupAsync('temporary', { + all: true, + family: 4 + }) + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + + t.equal(cacheable.stats.query, 1) + t.equal(cacheable.stats.cache, 0) + + entries = await cacheable.lookupAsync('temporary', { all: true, family: 4 }) + + verify(t, entries, [{ address: '127.0.0.1', family: 4 }]) + + t.equal(cacheable.stats.query, 1) + t.equal(cacheable.stats.cache, 1) +}) From 007872f83ae5f926618b97ae00f28631da04d986 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Fri, 29 Dec 2023 21:11:20 +0000 Subject: [PATCH 08/19] apply comments --- lib/core/dns-resolver.js | 74 +++++++++++++++++++++++++++++++++++----- test/dns-resolver.js | 36 +++++++++++++++++-- 2 files changed, 100 insertions(+), 10 deletions(-) diff --git a/lib/core/dns-resolver.js b/lib/core/dns-resolver.js index e497b7856df..7e7c02b3551 100644 --- a/lib/core/dns-resolver.js +++ b/lib/core/dns-resolver.js @@ -1,4 +1,33 @@ +'use strict' + // source: https://raw.githubusercontent.com/szmarczak/cacheable-lookup/9e60c9f6e74a003692aec68f3ddad93afe613b8f/source/index.mjs + +/** + +MIT License + +Copyright (c) 2019 Szymon Marczak + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +*/ + const { V4MAPPED, ADDRCONFIG, @@ -190,8 +219,12 @@ class DNSResolver { let cached = await this.query(hostname) if (options.family === 6) { - const filtered = cached.filter((entry) => entry.family === 6) - + const filtered = [] + for (const entry of cached) { + if (entry.family === 6) { + filtered.push(entry) + } + } if (options.hints & V4MAPPED) { if ((supportsALL && options.hints & ALL) || filtered.length === 0) { map4to6(cached) @@ -202,14 +235,26 @@ class DNSResolver { cached = filtered } } else if (options.family === 4) { - cached = cached.filter((entry) => entry.family === 4) + const filtered = [] + for (const entry of cached) { + if (entry.family === 4) { + filtered.push(entry) + } + } + cached = filtered } if (options.hints & ADDRCONFIG) { const { _iface } = this - cached = cached.filter((entry) => - entry.family === 6 ? _iface.has6 : _iface.has4 - ) + const filtered = [] + for (const entry of cached) { + if (entry.family === 6 && _iface.has6) { + filtered.push(entry) + } else if (entry.family === 4 && _iface.has4) { + filtered.push(entry) + } + } + cached = filtered } if (cached.length === 0) { @@ -267,7 +312,12 @@ class DNSResolver { ]) if (entries[0].status === 'rejected' && entries[1].status === 'rejected') { - throw entries[0].reason + const error = new AggregateError(`All resolvers failed for hostname: ${hostname}`) + error.errors = [ + entries[0].reason, + entries[1].reason + ] + throw error } const A = entries[0].status === 'fulfilled' ? entries[0].value : [] @@ -318,6 +368,15 @@ class DNSResolver { ignoreNoResultErrors(this._dnsLookup(hostname, all6)) ]) + if (entries[0].status === 'rejected' && entries[1].status === 'rejected') { + const error = new AggregateError(`All resolvers failed for hostname: ${hostname}`) + error.errors = [ + entries[0].reason, + entries[1].reason + ] + throw error + } + const A = entries[0].status === 'fulfilled' ? entries[0].value : [] const AAAA = entries[1].status === 'fulfilled' ? entries[1].value : [] @@ -416,7 +475,6 @@ class DNSResolver { } } - // @TODO useless for now as its not exposed updateInterfaceInfo () { const { _iface } = this diff --git a/test/dns-resolver.js b/test/dns-resolver.js index 8bedf3bd7bf..1cd8a57590d 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -1,3 +1,33 @@ +'use strict' + +// source: https://raw.githubusercontent.com/szmarczak/cacheable-lookup/9e60c9f6e74a003692aec68f3ddad93afe613b8f/tests/test.mjs + +/** + +MIT License + +Copyright (c) 2019 Szymon Marczak + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +*/ + const { promises: dnsPromises, V4MAPPED, ADDRCONFIG, ALL } = require('node:dns') const { promisify } = require('node:util') const http = require('node:http') @@ -856,9 +886,11 @@ test('returns IPv6 if no other entries available', async (t) => { test('throws when no internet connection', async (t) => { const cacheable = new DNSResolver({ resolver }) - await t.rejects(cacheable.lookupAsync('econnrefused'), { - code: 'ECONNREFUSED' + errors: [ + { code: 'ECONNREFUSED' }, + { code: 'ECONNREFUSED' } + ] }) }) From 7940b9a017251208711e47a6f20811a0e58db954 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Fri, 29 Dec 2023 23:05:53 +0000 Subject: [PATCH 09/19] add documentation and global --- README.md | 60 ++++++++++++++++++++++++++++++++-- index.js | 5 ++- lib/agent.js | 19 ++++++++--- lib/{core => }/dns-resolver.js | 0 lib/global-dns-resolver.js | 32 ++++++++++++++++++ test/dns-resolver.js | 5 +-- 6 files changed, 110 insertions(+), 11 deletions(-) rename lib/{core => }/dns-resolver.js (100%) create mode 100644 lib/global-dns-resolver.js diff --git a/README.md b/README.md index e1234a3b0c6..90438723a99 100644 --- a/README.md +++ b/README.md @@ -365,6 +365,62 @@ Returns: `URL` * **protocol** `string` (optional) * **search** `string` (optional) +## DNS caching + +Undici provides DNS caching via the `DNSResolver` class. + +This functionality is coming from [cacheable-lookup](https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f) package. + +By default a global DNSResolver will be created and passed down by the agent to the pool. +Here is an example of how to configure a custom DNSResolver: + +```js +new Agent({ + dnsResolver: { + lookupOptions: { family: 6, hints: ALL } + } + }) +``` + +To disable DNS caching for an agent: + +```js +new Agent({ + dnsResolver: { + disable: true + } +}) +``` + +Provide your custom DNS resolver, it must implement the method `lookup`: + +```js +new Agent({ + DNSResolver: new MyCustomDNSResolver() +}) +``` +You can get and set the global DNSResolver: + +```js +const { fetch, setGlobalDNSResolver, getGlobalDNSResolver, DNSResolver } = require('undici') + +setGlobalDNSResolver(new DNSResolver({ resolver: custom.dnsResolver })) + +await fetch('http://example.com') + +const dnsResolver = getGlobalDNSResolver() +const entry = await dnsResolver.lookup('example.com') +``` + +dnsResolver arguments: + +* **lookupOptions** + * **family** `4 | 6 | 0` - Default: `0` + * **hints** [`getaddrinfo flags`](https://nodejs.org/api/dns.html#supported-getaddrinfo-flags) + * **all** `Boolean` - Default: `false` + +For other arguments see: https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f + ## Specification Compliance This section documents parts of the HTTP/1.1 specification that Undici does @@ -413,9 +469,9 @@ Refs: https://fetch.spec.whatwg.org/#atomic-http-redirect-handling If you experience problem when connecting to a remote server that is resolved by your DNS servers to a IPv6 (AAAA record) first, there are chances that your local router or ISP might have problem connecting to IPv6 networks. In that case -undici will throw an error with code `UND_ERR_CONNECT_TIMEOUT`. +undici will throw an error with code `UND_ERR_CONNECT_TIMEOUT`. -If the target server resolves to both a IPv6 and IPv4 (A records) address and you are using a compatible Node version +If the target server resolves to both a IPv6 and IPv4 (A records) address and you are using a compatible Node version (18.3.0 and above), you can fix the problem by providing the `autoSelectFamily` option (support by both `undici.request` and `undici.Agent`) which will enable the family autoselection algorithm when establishing the connection. diff --git a/index.js b/index.js index 4c634ccc36d..057d93c0173 100644 --- a/index.js +++ b/index.js @@ -166,5 +166,8 @@ module.exports.MockPool = MockPool module.exports.MockAgent = MockAgent module.exports.mockErrors = mockErrors -const DNSResolver = require('./lib/core/dns-resolver') +const DNSResolver = require('./lib/dns-resolver') +const { getGlobalDNSResolver, setGlobalDNSResolver } = require('./lib/global-dns-resolver') module.exports.DNSResolver = DNSResolver +module.exports.getGlobalDNSResolver = getGlobalDNSResolver +module.exports.setGlobalDNSResolver = setGlobalDNSResolver diff --git a/lib/agent.js b/lib/agent.js index a8252db5be4..4e19d030655 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -7,7 +7,8 @@ const Pool = require('./pool') const Client = require('./client') const util = require('./core/util') const createRedirectInterceptor = require('./interceptor/redirectInterceptor') -const DNSResolver = require('./core/dns-resolver') +const DNSResolver = require('./dns-resolver') +const { getGlobalDNSResolver } = require('./global-dns-resolver') const kOnConnect = Symbol('onConnect') const kOnDisconnect = Symbol('onDisconnect') @@ -23,12 +24,22 @@ function defaultFactory (origin, opts) { : new Pool(origin, opts) } -function getDnsResolver (options = {}) { +function getDNSResolver (options) { if (options.DNSResolver instanceof DNSResolver) { return options.DNSResolver } - return options?.dnsResolver?.disable ? undefined : new DNSResolver(options?.dnsResolver) + if (options?.dnsResolver?.disable) { + return undefined + } + + // if options are provided create a new instance + if (options.dnsResolver) { + return new DNSResolver(options.dnsResolver) + } + + // without any options fallback to the global one + return getGlobalDNSResolver() } class Agent extends DispatcherBase { @@ -59,7 +70,7 @@ class Agent extends DispatcherBase { this[kOptions].interceptors = options.interceptors ? { ...options.interceptors } : undefined - this[kOptions].dnsResolver = getDnsResolver(options) + this[kOptions].dnsResolver = getDNSResolver(options) this[kMaxRedirections] = maxRedirections this[kFactory] = factory this[kClients] = new Map() diff --git a/lib/core/dns-resolver.js b/lib/dns-resolver.js similarity index 100% rename from lib/core/dns-resolver.js rename to lib/dns-resolver.js diff --git a/lib/global-dns-resolver.js b/lib/global-dns-resolver.js new file mode 100644 index 00000000000..584436ac251 --- /dev/null +++ b/lib/global-dns-resolver.js @@ -0,0 +1,32 @@ +'use strict' + +// We include a version number for the DNSResolver. In case of breaking changes, +// this version number must be increased to avoid conflicts. +const globalDNSResolver = Symbol.for('undici.globalDNSResolver.1') +const { InvalidArgumentError } = require('./core/errors') +const DNSResolver = require('./dns-resolver') + +if (getGlobalDNSResolver() === undefined) { + setGlobalDNSResolver(new DNSResolver()) +} + +function getGlobalDNSResolver () { + return globalThis[globalDNSResolver] +} + +function setGlobalDNSResolver (dnsResolver) { + if (!dnsResolver || typeof dnsResolver.lookup !== 'function') { + throw new InvalidArgumentError('Argument dnsResolver must implement DNSResolver') + } + Object.defineProperty(globalThis, globalDNSResolver, { + value: dnsResolver, + writable: true, + enumerable: false, + configurable: false + }) +} + +module.exports = { + setGlobalDNSResolver, + getGlobalDNSResolver +} diff --git a/test/dns-resolver.js b/test/dns-resolver.js index 1cd8a57590d..e47cc30ca5b 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -237,13 +237,10 @@ const createResolver = () => { const resolver = createResolver() dnsStub.lookup = resolver.lookup -const DNSResolver = proxyquire('../lib/core/dns-resolver', { +const DNSResolver = proxyquire('../lib/dns-resolver', { 'node:os': osStub, 'node:dns': dnsStub }) -// await quibble.esm('node:dns', dnsExports, dnsExports) -// -// const DNSResolver = require('../lib/core/dns-resolver') const verify = (t, entry, value) => { if (Array.isArray(value)) { From 3db955010c480614357872cac7e9a3fadfa167fb Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Mon, 8 Jan 2024 22:16:36 +0000 Subject: [PATCH 10/19] remove global dns resolver --- README.md | 16 +++------------- index.js | 3 --- lib/agent.js | 9 +-------- lib/global-dns-resolver.js | 32 -------------------------------- 4 files changed, 4 insertions(+), 56 deletions(-) delete mode 100644 lib/global-dns-resolver.js diff --git a/README.md b/README.md index 90438723a99..75b04b29907 100644 --- a/README.md +++ b/README.md @@ -399,27 +399,17 @@ new Agent({ DNSResolver: new MyCustomDNSResolver() }) ``` -You can get and set the global DNSResolver: -```js -const { fetch, setGlobalDNSResolver, getGlobalDNSResolver, DNSResolver } = require('undici') - -setGlobalDNSResolver(new DNSResolver({ resolver: custom.dnsResolver })) - -await fetch('http://example.com') - -const dnsResolver = getGlobalDNSResolver() -const entry = await dnsResolver.lookup('example.com') -``` +All arguments match [`cacheable-lookup`](https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f) package -dnsResolver arguments: +Extra arguments available in Undici: * **lookupOptions** * **family** `4 | 6 | 0` - Default: `0` * **hints** [`getaddrinfo flags`](https://nodejs.org/api/dns.html#supported-getaddrinfo-flags) * **all** `Boolean` - Default: `false` -For other arguments see: https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f +* **roundRobinStrategy** `'first' | 'random'` - Default: `'first'` ## Specification Compliance diff --git a/index.js b/index.js index 057d93c0173..3ba08d64721 100644 --- a/index.js +++ b/index.js @@ -167,7 +167,4 @@ module.exports.MockAgent = MockAgent module.exports.mockErrors = mockErrors const DNSResolver = require('./lib/dns-resolver') -const { getGlobalDNSResolver, setGlobalDNSResolver } = require('./lib/global-dns-resolver') module.exports.DNSResolver = DNSResolver -module.exports.getGlobalDNSResolver = getGlobalDNSResolver -module.exports.setGlobalDNSResolver = setGlobalDNSResolver diff --git a/lib/agent.js b/lib/agent.js index 4e19d030655..8960b40b049 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -8,7 +8,6 @@ const Client = require('./client') const util = require('./core/util') const createRedirectInterceptor = require('./interceptor/redirectInterceptor') const DNSResolver = require('./dns-resolver') -const { getGlobalDNSResolver } = require('./global-dns-resolver') const kOnConnect = Symbol('onConnect') const kOnDisconnect = Symbol('onDisconnect') @@ -33,13 +32,7 @@ function getDNSResolver (options) { return undefined } - // if options are provided create a new instance - if (options.dnsResolver) { - return new DNSResolver(options.dnsResolver) - } - - // without any options fallback to the global one - return getGlobalDNSResolver() + return new DNSResolver(options.dnsResolver) } class Agent extends DispatcherBase { diff --git a/lib/global-dns-resolver.js b/lib/global-dns-resolver.js deleted file mode 100644 index 584436ac251..00000000000 --- a/lib/global-dns-resolver.js +++ /dev/null @@ -1,32 +0,0 @@ -'use strict' - -// We include a version number for the DNSResolver. In case of breaking changes, -// this version number must be increased to avoid conflicts. -const globalDNSResolver = Symbol.for('undici.globalDNSResolver.1') -const { InvalidArgumentError } = require('./core/errors') -const DNSResolver = require('./dns-resolver') - -if (getGlobalDNSResolver() === undefined) { - setGlobalDNSResolver(new DNSResolver()) -} - -function getGlobalDNSResolver () { - return globalThis[globalDNSResolver] -} - -function setGlobalDNSResolver (dnsResolver) { - if (!dnsResolver || typeof dnsResolver.lookup !== 'function') { - throw new InvalidArgumentError('Argument dnsResolver must implement DNSResolver') - } - Object.defineProperty(globalThis, globalDNSResolver, { - value: dnsResolver, - writable: true, - enumerable: false, - configurable: false - }) -} - -module.exports = { - setGlobalDNSResolver, - getGlobalDNSResolver -} From afd92c8b6a3d32589105870b51841e37db4625cf Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Tue, 9 Jan 2024 22:17:04 +0000 Subject: [PATCH 11/19] resolve comments --- lib/agent.js | 10 +-- lib/dns-resolver.js | 155 ++++++++++++++++++++++++------------------- test/dns-resolver.js | 35 ++++++++-- 3 files changed, 120 insertions(+), 80 deletions(-) diff --git a/lib/agent.js b/lib/agent.js index 8960b40b049..3091fffe87f 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -24,15 +24,15 @@ function defaultFactory (origin, opts) { } function getDNSResolver (options) { - if (options.DNSResolver instanceof DNSResolver) { - return options.DNSResolver + if (options?.dnsResolverOptions?.disable) { + return undefined } - if (options?.dnsResolver?.disable) { - return undefined + if (options.DNSResolver && typeof options.DNSResolver.lookup === 'function') { + return options.DNSResolver } - return new DNSResolver(options.dnsResolver) + return new DNSResolver(options.dnsResolverOptions) } class Agent extends DispatcherBase { diff --git a/lib/dns-resolver.js b/lib/dns-resolver.js index 7e7c02b3551..d7225a64a8c 100644 --- a/lib/dns-resolver.js +++ b/lib/dns-resolver.js @@ -43,8 +43,6 @@ const { Resolver: AsyncResolver } = dnsPromises const kExpires = Symbol('expires') const roundRobinStrategies = ['first', 'random'] -const supportsALL = typeof ALL === 'number' - const map4to6 = (entries) => { for (const entry of entries) { if (entry.family === 6) { @@ -105,6 +103,18 @@ const all4 = { all: true, family: 4 } const all6 = { all: true, family: 6 } class DNSResolver { + #resolver + #cache + #dnsLookup + #iface + #pending = {} + #nextRemovalTime = false + #removalTimeout + #hostnamesToFallback = new Set() + #fallbackInterval + #resolve4 + #resolve6 + constructor ({ cache = new Map(), maxTtl = Infinity, @@ -124,33 +134,33 @@ class DNSResolver { this.roundRobinStrategy = roundRobinStrategy - this._cache = cache - this._resolver = resolver - this._dnsLookup = lookup && promisify(lookup) + this.#cache = cache + this.#resolver = resolver + this.#dnsLookup = lookup && promisify(lookup) this.stats = { cache: 0, query: 0 } - if (this._resolver instanceof AsyncResolver) { - this._resolve4 = this._resolver.resolve4.bind(this._resolver) - this._resolve6 = this._resolver.resolve6.bind(this._resolver) + if (this.#resolver instanceof AsyncResolver) { + this.#resolve4 = this.#resolver.resolve4.bind(this.#resolver) + this.#resolve6 = this.#resolver.resolve6.bind(this.#resolver) } else { - this._resolve4 = promisify(this._resolver.resolve4.bind(this._resolver)) - this._resolve6 = promisify(this._resolver.resolve6.bind(this._resolver)) + this.#resolve4 = promisify(this.#resolver.resolve4.bind(this.#resolver)) + this.#resolve6 = promisify(this.#resolver.resolve6.bind(this.#resolver)) } - this._iface = getIfaceInfo() + this.#iface = getIfaceInfo() - this._pending = {} - this._nextRemovalTime = false - this._hostnamesToFallback = new Set() + this.#pending = {} + this.#nextRemovalTime = false + this.#hostnamesToFallback = new Set() this.fallbackDuration = fallbackDuration if (fallbackDuration > 0) { const interval = setInterval(() => { - this._hostnamesToFallback.clear() + this.#hostnamesToFallback.clear() }, fallbackDuration * 1000) /* istanbul ignore next: There is no `interval.unref()` when running inside an Electron renderer */ @@ -158,28 +168,36 @@ class DNSResolver { interval.unref() } - this._fallbackInterval = interval + this.#fallbackInterval = interval } if (lookupOptions) { - this.lookup = (hostname, _options, callback) => this.__lookup(hostname, lookupOptions, callback) + this.lookup = (hostname, _options, callback) => this.#lookup(hostname, lookupOptions, callback) } else { - this.lookup = this.__lookup.bind(this) + this.lookup = this.#lookup.bind(this) } this.lookupAsync = this.lookupAsync.bind(this) } + get _cache () { + return this.#cache + } + + get _hostnamesToFallback () { + return this.#hostnamesToFallback + } + set servers (servers) { this.clear() - this._resolver.setServers(servers) + this.#resolver.setServers(servers) } get servers () { - return this._resolver.getServers() + return this.#resolver.getServers() } - __lookup (hostname, options, callback) { + #lookup (hostname, options, callback) { if (typeof options === 'function') { callback = options options = {} @@ -226,7 +244,7 @@ class DNSResolver { } } if (options.hints & V4MAPPED) { - if ((supportsALL && options.hints & ALL) || filtered.length === 0) { + if ((options.hints & ALL) || filtered.length === 0) { map4to6(cached) } else { cached = filtered @@ -245,12 +263,11 @@ class DNSResolver { } if (options.hints & ADDRCONFIG) { - const { _iface } = this const filtered = [] for (const entry of cached) { - if (entry.family === 6 && _iface.has6) { + if (entry.family === 6 && this.#iface.has6) { filtered.push(entry) - } else if (entry.family === 4 && _iface.has4) { + } else if (entry.family === 4 && this.#iface.has4) { filtered.push(entry) } } @@ -278,25 +295,25 @@ class DNSResolver { } async query (hostname) { - let cached = await this._cache.get(hostname) + let cached = await this.#cache.get(hostname) if (cached) { this.stats.cache++ } if (!cached) { - const pending = this._pending[hostname] + const pending = this.#pending[hostname] if (pending) { this.stats.cache++ cached = await pending } else { const newPromise = this.queryAndCache(hostname) - this._pending[hostname] = newPromise + this.#pending[hostname] = newPromise this.stats.query++ try { cached = await newPromise } finally { - delete this._pending[hostname] + delete this.#pending[hostname] } } } @@ -304,19 +321,18 @@ class DNSResolver { return cached } - async _resolve (hostname) { + async #resolve (hostname) { // ANY is unsafe as it doesn't trigger new queries in the underlying server. const entries = await Promise.allSettled([ - ignoreNoResultErrors(this._resolve4(hostname, ttl)), - ignoreNoResultErrors(this._resolve6(hostname, ttl)) + ignoreNoResultErrors(this.#resolve4(hostname, ttl)), + ignoreNoResultErrors(this.#resolve6(hostname, ttl)) ]) if (entries[0].status === 'rejected' && entries[1].status === 'rejected') { - const error = new AggregateError(`All resolvers failed for hostname: ${hostname}`) - error.errors = [ + const error = new AggregateError([ entries[0].reason, entries[1].reason - ] + ], `All resolvers failed for hostname: ${hostname}`) throw error } @@ -359,21 +375,20 @@ class DNSResolver { } } - async _lookup (hostname) { + async #lookupViaDns (hostname) { try { const entries = await Promise.allSettled([ // Passing {all: true} doesn't return all IPv4 and IPv6 entries. // See https://github.com/szmarczak/cacheable-lookup/issues/42 - ignoreNoResultErrors(this._dnsLookup(hostname, all4)), - ignoreNoResultErrors(this._dnsLookup(hostname, all6)) + ignoreNoResultErrors(this.#dnsLookup(hostname, all4)), + ignoreNoResultErrors(this.#dnsLookup(hostname, all6)) ]) if (entries[0].status === 'rejected' && entries[1].status === 'rejected') { - const error = new AggregateError(`All resolvers failed for hostname: ${hostname}`) - error.errors = [ + const error = new AggregateError([ entries[0].reason, entries[1].reason - ] + ], `All resolvers failed for hostname: ${hostname}`) throw error } @@ -392,13 +407,13 @@ class DNSResolver { } } - async _set (hostname, data, cacheTtl) { + async #set (hostname, data, cacheTtl) { if (this.maxTtl > 0 && cacheTtl > 0) { cacheTtl = Math.min(cacheTtl, this.maxTtl) * 1000 data[kExpires] = Date.now() + cacheTtl try { - await this._cache.set(hostname, data, cacheTtl) + await this.#cache.set(hostname, data, cacheTtl) } catch (error) { this.lookupAsync = async () => { const cacheError = new Error( @@ -410,91 +425,91 @@ class DNSResolver { } } - if (isIterable(this._cache)) { - this._tick(cacheTtl) + if (isIterable(this.#cache)) { + this.#tick(cacheTtl) } } } async queryAndCache (hostname) { - if (this._hostnamesToFallback.has(hostname)) { - return this._dnsLookup(hostname, all) + if (this.#hostnamesToFallback.has(hostname)) { + return this.#dnsLookup(hostname, all) } - let query = await this._resolve(hostname) + let query = await this.#resolve(hostname) - if (query.entries.length === 0 && this._dnsLookup) { - query = await this._lookup(hostname) + if (query.entries.length === 0 && this.#dnsLookup) { + query = await this.#lookupViaDns(hostname) if (query.entries.length !== 0 && this.fallbackDuration > 0) { // Use `dns.lookup(...)` for that particular hostname - this._hostnamesToFallback.add(hostname) + this.#hostnamesToFallback.add(hostname) } } const cacheTtl = query.entries.length === 0 ? this.errorTtl : query.cacheTtl - await this._set(hostname, query.entries, cacheTtl) + await this.#set(hostname, query.entries, cacheTtl) return query.entries } - _tick (ms) { - const nextRemovalTime = this._nextRemovalTime + #tick (ms) { + const nextRemovalTime = this.#nextRemovalTime if (!nextRemovalTime || ms < nextRemovalTime) { - clearTimeout(this._removalTimeout) + clearTimeout(this.#removalTimeout) - this._nextRemovalTime = ms + this.#nextRemovalTime = ms - this._removalTimeout = setTimeout(() => { - this._nextRemovalTime = false + this.#removalTimeout = setTimeout(() => { + this.#nextRemovalTime = false let nextExpiry = Infinity const now = Date.now() - for (const [hostname, entries] of this._cache) { + for (const [hostname, entries] of this.#cache) { const expires = entries[kExpires] if (now >= expires) { - this._cache.delete(hostname) + this.#cache.delete(hostname) } else if (expires < nextExpiry) { nextExpiry = expires } } if (nextExpiry !== Infinity) { - this._tick(nextExpiry - now) + this.#tick(nextExpiry - now) } }, ms) /* istanbul ignore next: There is no `timeout.unref()` when running inside an Electron renderer */ - if (this._removalTimeout.unref) { - this._removalTimeout.unref() + if (this.#removalTimeout.unref) { + this.#removalTimeout.unref() } } } updateInterfaceInfo () { - const { _iface } = this + const iface = this.#iface - this._iface = getIfaceInfo() + this.#iface = getIfaceInfo() if ( - (_iface.has4 && !this._iface.has4) || - (_iface.has6 && !this._iface.has6) + (iface.has4 && !this.#iface.has4) || + (iface.has6 && !this.#iface.has6) ) { - this._cache.clear() + this.#cache.clear() } } clear (hostname) { if (hostname) { - this._cache.delete(hostname) + this.#cache.delete(hostname) return } - this._cache.clear() + this.#cache.clear() } } diff --git a/test/dns-resolver.js b/test/dns-resolver.js index e47cc30ca5b..93dce5c7b9f 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -192,6 +192,9 @@ const createResolver = () => { }, data: { '127.0.0.1': { + agentdns: [ + { address: '127.0.0.1', family: 4, ttl: 60 } + ], localhost: [ { address: '127.0.0.1', family: 4, ttl: 60 }, { address: '::ffff:127.0.0.2', family: 6, ttl: 60 } @@ -580,7 +583,7 @@ test('options.maxTtl', async (t) => { resolverEntry.address = '127.0.0.2' // Wait until it expires - await sleep(cacheable.maxTtl * 1000 + 1) + await sleep(cacheable.maxTtl * 1000 + 10) // Lookup again verify(t, await cacheable.lookupAsync('maxTtl'), { @@ -678,9 +681,6 @@ test('works (Internet connection)', async (t) => { test('async resolver (Internet connection)', async (t) => { const cacheable = new DNSResolver({ resolver: new AsyncResolver() }) - t.equal(typeof cacheable._resolve4, 'function') - t.equal(typeof cacheable._resolve6, 'function') - const { address } = await cacheable.lookupAsync( '1dot1dot1dot1.cloudflare-dns.com', { family: 4 } @@ -826,7 +826,7 @@ test('errors are cached', async (t) => { t.equal(cacheable._cache.size, 1) - await sleep(cacheable.errorTtl * 1000 + 1) + await sleep(cacheable.errorTtl * 1000 + 10) t.equal(cacheable._cache.size, 0) }) @@ -879,6 +879,7 @@ test('returns IPv6 if no other entries available', async (t) => { address: '::ffff:127.0.0.2', family: 6 }) + await mockedInterfaces({ has4: true, has6: true }) }) test('throws when no internet connection', async (t) => { @@ -962,3 +963,27 @@ test('cache and query stats', async (t) => { t.equal(cacheable.stats.query, 1) t.equal(cacheable.stats.cache, 1) }) + +test('verify DNSResolver is working caching requests', t => { + t.plan(2) + const { Agent, request } = require('../index') + const dnsResolver = new DNSResolver({ resolver: createResolver() }) + dnsResolver.clear() + const agent = new Agent({ + DNSResolver: dnsResolver + }) + t.equal(dnsResolver._cache.size, 0) + + const server = http.createServer((req, res) => { + req.pipe(res) + }) + + t.teardown(server.close.bind(server)) + + server.listen(0, async () => { + const origin = `http://agentdns:${server.address().port}` + await request(origin, { dispatcher: agent }) + t.equal(dnsResolver._cache.size, 1) + t.end() + }) +}) From 30ef32bd738e4bac30cdb62eafbc29b2844c7c57 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Tue, 9 Jan 2024 22:35:30 +0000 Subject: [PATCH 12/19] test agent DNSResolver disabled --- test/dns-resolver.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/dns-resolver.js b/test/dns-resolver.js index 93dce5c7b9f..c240db05c6e 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -964,7 +964,7 @@ test('cache and query stats', async (t) => { t.equal(cacheable.stats.cache, 1) }) -test('verify DNSResolver is working caching requests', t => { +test('agent: verify DNSResolver is working caching requests', t => { t.plan(2) const { Agent, request } = require('../index') const dnsResolver = new DNSResolver({ resolver: createResolver() }) @@ -987,3 +987,27 @@ test('verify DNSResolver is working caching requests', t => { t.end() }) }) + +test('agent verify DNSResolver is disabled', t => { + t.plan(2) + const { Agent, request } = require('../index') + const dnsResolver = new DNSResolver({ resolver: createResolver() }) + dnsResolver.clear() + const agent = new Agent({ + dnsResolverOptions: { disable: true } + }) + t.equal(dnsResolver._cache.size, 0) + + const server = http.createServer((req, res) => { + req.pipe(res) + }) + + t.teardown(server.close.bind(server)) + + server.listen(0, async () => { + const origin = `http://localhost:${server.address().port}` + await request(origin, { dispatcher: agent }) + t.equal(dnsResolver._cache.size, 0) + t.end() + }) +}) From 67c132f83a314dccacb7de025c1169545043dece Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Wed, 10 Jan 2024 09:54:50 +0000 Subject: [PATCH 13/19] Update lib/agent.js Co-authored-by: Carlos Fuentes --- lib/agent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/agent.js b/lib/agent.js index 3091fffe87f..cbf46d6e25f 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -25,7 +25,7 @@ function defaultFactory (origin, opts) { function getDNSResolver (options) { if (options?.dnsResolverOptions?.disable) { - return undefined + return } if (options.DNSResolver && typeof options.DNSResolver.lookup === 'function') { From 4f603a6cf3e9738b112f947489887d2532c95e8f Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Wed, 10 Jan 2024 09:55:02 +0000 Subject: [PATCH 14/19] Update lib/dns-resolver.js Co-authored-by: Carlos Fuentes --- lib/dns-resolver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns-resolver.js b/lib/dns-resolver.js index d7225a64a8c..4f48801b6fd 100644 --- a/lib/dns-resolver.js +++ b/lib/dns-resolver.js @@ -122,7 +122,7 @@ class DNSResolver { errorTtl = 0.15, resolver = new AsyncResolver(), lookup = dnsLookup, - lookupOptions = undefined, + lookupOptions, roundRobinStrategy = 'first' } = {}) { this.maxTtl = maxTtl From 3c4487b69ca40ba5c7ddcf558d7d803e62222769 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Wed, 10 Jan 2024 09:55:18 +0000 Subject: [PATCH 15/19] Update lib/pool.js Co-authored-by: Carlos Fuentes --- lib/pool.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pool.js b/lib/pool.js index 9a04178347b..ff3ac9c0bd5 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -58,7 +58,7 @@ class Pool extends PoolBase { maxCachedSessions, allowH2, socketPath, - lookup: dnsResolver ? dnsResolver.lookup : undefined, + lookup: dnsResolver?.lookup timeout: connectTimeout, ...(util.nodeHasAutoSelectFamily && autoSelectFamily ? { autoSelectFamily, autoSelectFamilyAttemptTimeout } : undefined), ...connect From b192a0ee2acbc5e85d9b1a2c9c60d96460b7272f Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Wed, 10 Jan 2024 22:41:05 +0000 Subject: [PATCH 16/19] update comments --- README.md | 6 +++--- lib/core/symbols.js | 4 +++- lib/dns-resolver.js | 26 +++++++++++++------------- lib/pool.js | 2 +- test/dns-resolver.js | 35 ++++++++++++++++++----------------- 5 files changed, 38 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 75b04b29907..92ba00232ec 100644 --- a/README.md +++ b/README.md @@ -376,7 +376,7 @@ Here is an example of how to configure a custom DNSResolver: ```js new Agent({ - dnsResolver: { + dnsResolverOptions: { lookupOptions: { family: 6, hints: ALL } } }) @@ -386,7 +386,7 @@ To disable DNS caching for an agent: ```js new Agent({ - dnsResolver: { + dnsResolverOptions: { disable: true } }) @@ -409,7 +409,7 @@ Extra arguments available in Undici: * **hints** [`getaddrinfo flags`](https://nodejs.org/api/dns.html#supported-getaddrinfo-flags) * **all** `Boolean` - Default: `false` -* **roundRobinStrategy** `'first' | 'random'` - Default: `'first'` +* **scheduling** `'first' | 'random'` - Default: `'first'` ## Specification Compliance diff --git a/lib/core/symbols.js b/lib/core/symbols.js index 68d8566fac0..df03ba0e161 100644 --- a/lib/core/symbols.js +++ b/lib/core/symbols.js @@ -59,5 +59,7 @@ module.exports = { kHTTP2CopyHeaders: Symbol('http2 copy headers'), kHTTPConnVersion: Symbol('http connection version'), kRetryHandlerDefaultRetry: Symbol('retry agent default retry'), - kConstruct: Symbol('constructable') + kConstruct: Symbol('constructable'), + kDnsCacheSize: Symbol('dns cache size'), + kDnsHostnamesToFallback: Symbol('dns hostnames to fallback') } diff --git a/lib/dns-resolver.js b/lib/dns-resolver.js index 4f48801b6fd..4ff2893c90b 100644 --- a/lib/dns-resolver.js +++ b/lib/dns-resolver.js @@ -37,6 +37,7 @@ const { } = require('node:dns') const { promisify } = require('node:util') const os = require('node:os') +const { kDnsCacheSize, kDnsHostnamesToFallback } = require('./core/symbols') const { Resolver: AsyncResolver } = dnsPromises @@ -109,11 +110,12 @@ class DNSResolver { #iface #pending = {} #nextRemovalTime = false + #fallbackDuration #removalTimeout #hostnamesToFallback = new Set() - #fallbackInterval #resolve4 #resolve6 + #scheduling constructor ({ cache = new Map(), @@ -123,16 +125,16 @@ class DNSResolver { resolver = new AsyncResolver(), lookup = dnsLookup, lookupOptions, - roundRobinStrategy = 'first' + scheduling = 'first' } = {}) { this.maxTtl = maxTtl this.errorTtl = errorTtl - if (roundRobinStrategies.includes(roundRobinStrategy) === false) { + if (roundRobinStrategies.includes(scheduling) === false) { throw new Error(`roundRobinStrategy must be one of: ${roundRobinStrategies.join(', ')}`) } - this.roundRobinStrategy = roundRobinStrategy + this.#scheduling = scheduling this.#cache = cache this.#resolver = resolver @@ -156,7 +158,7 @@ class DNSResolver { this.#nextRemovalTime = false this.#hostnamesToFallback = new Set() - this.fallbackDuration = fallbackDuration + this.#fallbackDuration = fallbackDuration if (fallbackDuration > 0) { const interval = setInterval(() => { @@ -167,8 +169,6 @@ class DNSResolver { if (interval.unref) { interval.unref() } - - this.#fallbackInterval = interval } if (lookupOptions) { @@ -179,12 +179,12 @@ class DNSResolver { this.lookupAsync = this.lookupAsync.bind(this) } - get _cache () { - return this.#cache + get [kDnsCacheSize] () { + return this.#cache.size ?? 0 } - get _hostnamesToFallback () { - return this.#hostnamesToFallback + get [kDnsHostnamesToFallback] () { + return this.#hostnamesToFallback.size ?? 0 } set servers (servers) { @@ -286,7 +286,7 @@ class DNSResolver { return cached } - if (this.roundRobinStrategy === 'first') { + if (this.#scheduling === 'first') { return cached[0] } else { // random @@ -441,7 +441,7 @@ class DNSResolver { if (query.entries.length === 0 && this.#dnsLookup) { query = await this.#lookupViaDns(hostname) - if (query.entries.length !== 0 && this.fallbackDuration > 0) { + if (query.entries.length !== 0 && this.#fallbackDuration > 0) { // Use `dns.lookup(...)` for that particular hostname this.#hostnamesToFallback.add(hostname) } diff --git a/lib/pool.js b/lib/pool.js index ff3ac9c0bd5..a582259187f 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -58,7 +58,7 @@ class Pool extends PoolBase { maxCachedSessions, allowH2, socketPath, - lookup: dnsResolver?.lookup + lookup: dnsResolver?.lookup, timeout: connectTimeout, ...(util.nodeHasAutoSelectFamily && autoSelectFamily ? { autoSelectFamily, autoSelectFamilyAttemptTimeout } : undefined), ...connect diff --git a/test/dns-resolver.js b/test/dns-resolver.js index c240db05c6e..320682bee8f 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -34,6 +34,7 @@ const http = require('node:http') const { test } = require('tap') const originalDns = require('node:dns') const proxyquire = require('proxyquire') +const { kDnsCacheSize, kDnsHostnamesToFallback } = require('../lib/core/symbols') const osStub = {} const dnsStub = { ...originalDns @@ -692,11 +693,11 @@ test('clear() works', async (t) => { const cacheable = new DNSResolver({ resolver }) await cacheable.lookupAsync('localhost') - t.equal(cacheable._cache.size, 1) + t.equal(cacheable[kDnsCacheSize], 1) cacheable.clear() - t.equal(cacheable._cache.size, 0) + t.equal(cacheable[kDnsCacheSize], 0) }) test('ttl works', async (t) => { @@ -706,11 +707,11 @@ test('ttl works', async (t) => { cacheable.lookupAsync('temporary'), cacheable.lookupAsync('ttl') ]) - t.equal(cacheable._cache.size, 2) + t.equal(cacheable[kDnsCacheSize], 2) await sleep(2001) - t.equal(cacheable._cache.size, 0) + t.equal(cacheable[kDnsCacheSize], 0) }) test('fallback works', async (t) => { @@ -726,7 +727,7 @@ test('fallback works', async (t) => { t.equal(entries[1].address, '127.0.0.2') t.equal(entries[1].family, 4) - t.equal(cacheable._cache.size, 0) + t.equal(cacheable[kDnsCacheSize], 0) await cacheable.lookupAsync('osHostname', { all: true }) @@ -738,7 +739,7 @@ test('fallback works', async (t) => { await sleep(100) - t.equal(cacheable._hostnamesToFallback.size, 0) + t.equal(cacheable[kDnsHostnamesToFallback], 0) }) test('fallback works if ip change', async (t) => { @@ -759,7 +760,7 @@ test('fallback works if ip change', async (t) => { t.equal(entries[1].address, '127.0.0.2') t.equal(entries[1].family, 4) - t.equal(cacheable._cache.size, 0) + t.equal(cacheable[kDnsCacheSize], 0) // Second call: enter in `if (this._hostnamesToFallback.has(hostname)) {` // And use _dnsLookup @@ -783,7 +784,7 @@ test('fallback works if ip change', async (t) => { t.equal(entries2[1].address, '127.0.0.4') t.equal(entries2[1].family, 4) - t.equal(cacheable._cache.size, 0) + t.equal(cacheable[kDnsCacheSize], 0) delete resolver.lookupData.osHostnameChange }) @@ -824,11 +825,11 @@ test('errors are cached', async (t) => { code: 'ENOTFOUND' }) - t.equal(cacheable._cache.size, 1) + t.equal(cacheable[kDnsCacheSize], 1) await sleep(cacheable.errorTtl * 1000 + 10) - t.equal(cacheable._cache.size, 0) + t.equal(cacheable[kDnsCacheSize], 0) }) test('passing family as options', async (t) => { @@ -852,7 +853,7 @@ test('clear(hostname) works', async (t) => { cacheable.clear('localhost') - t.equal(cacheable._cache.size, 1) + t.equal(cacheable[kDnsCacheSize], 1) }) test('prevents overloading DNS', async (t) => { @@ -967,12 +968,12 @@ test('cache and query stats', async (t) => { test('agent: verify DNSResolver is working caching requests', t => { t.plan(2) const { Agent, request } = require('../index') - const dnsResolver = new DNSResolver({ resolver: createResolver() }) + const dnsResolver = new DNSResolver({ resolver }) dnsResolver.clear() const agent = new Agent({ DNSResolver: dnsResolver }) - t.equal(dnsResolver._cache.size, 0) + t.equal(dnsResolver[kDnsCacheSize], 0) const server = http.createServer((req, res) => { req.pipe(res) @@ -983,7 +984,7 @@ test('agent: verify DNSResolver is working caching requests', t => { server.listen(0, async () => { const origin = `http://agentdns:${server.address().port}` await request(origin, { dispatcher: agent }) - t.equal(dnsResolver._cache.size, 1) + t.equal(dnsResolver[kDnsCacheSize], 1) t.end() }) }) @@ -991,12 +992,12 @@ test('agent: verify DNSResolver is working caching requests', t => { test('agent verify DNSResolver is disabled', t => { t.plan(2) const { Agent, request } = require('../index') - const dnsResolver = new DNSResolver({ resolver: createResolver() }) + const dnsResolver = new DNSResolver({ resolver }) dnsResolver.clear() const agent = new Agent({ dnsResolverOptions: { disable: true } }) - t.equal(dnsResolver._cache.size, 0) + t.equal(dnsResolver[kDnsCacheSize], 0) const server = http.createServer((req, res) => { req.pipe(res) @@ -1007,7 +1008,7 @@ test('agent verify DNSResolver is disabled', t => { server.listen(0, async () => { const origin = `http://localhost:${server.address().port}` await request(origin, { dispatcher: agent }) - t.equal(dnsResolver._cache.size, 0) + t.equal(dnsResolver[kDnsCacheSize], 0) t.end() }) }) From bbff8e2699ecd16450c197558802517bcf7c686b Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Wed, 10 Jan 2024 22:51:11 +0000 Subject: [PATCH 17/19] add types --- README.md | 2 +- lib/agent.js | 4 +-- test/dns-resolver.js | 2 +- types/agent.d.ts | 3 ++ types/dns-resolver.d.ts | 71 +++++++++++++++++++++++++++++++++++++++++ types/pool.d.ts | 5 ++- 6 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 types/dns-resolver.d.ts diff --git a/README.md b/README.md index 92ba00232ec..e9b8f45153d 100644 --- a/README.md +++ b/README.md @@ -396,7 +396,7 @@ Provide your custom DNS resolver, it must implement the method `lookup`: ```js new Agent({ - DNSResolver: new MyCustomDNSResolver() + dnsResolver: new MyCustomDNSResolver() }) ``` diff --git a/lib/agent.js b/lib/agent.js index cbf46d6e25f..fde9634a862 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -28,8 +28,8 @@ function getDNSResolver (options) { return } - if (options.DNSResolver && typeof options.DNSResolver.lookup === 'function') { - return options.DNSResolver + if (options.dnsResolver && typeof options.dnsResolver.lookup === 'function') { + return options.dnsResolver } return new DNSResolver(options.dnsResolverOptions) diff --git a/test/dns-resolver.js b/test/dns-resolver.js index 320682bee8f..f567731ea92 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -971,7 +971,7 @@ test('agent: verify DNSResolver is working caching requests', t => { const dnsResolver = new DNSResolver({ resolver }) dnsResolver.clear() const agent = new Agent({ - DNSResolver: dnsResolver + dnsResolver }) t.equal(dnsResolver[kDnsCacheSize], 0) diff --git a/types/agent.d.ts b/types/agent.d.ts index 58081ce9372..9d915367c0b 100644 --- a/types/agent.d.ts +++ b/types/agent.d.ts @@ -1,6 +1,7 @@ import { URL } from 'url' import Pool from './pool' import Dispatcher from "./dispatcher"; +import DNSResolver from './dns-resolver'; export default Agent @@ -22,6 +23,8 @@ declare namespace Agent { maxRedirections?: number; interceptors?: { Agent?: readonly Dispatcher.DispatchInterceptor[] } & Pool.Options["interceptors"] + + dnsResolverOptions: DNSResolver.Options; } export interface DispatchOptions extends Dispatcher.DispatchOptions { diff --git a/types/dns-resolver.d.ts b/types/dns-resolver.d.ts new file mode 100644 index 00000000000..a39c5a4d5e7 --- /dev/null +++ b/types/dns-resolver.d.ts @@ -0,0 +1,71 @@ +import { lookup, promises, LookupAddress, LookupOneOptions, LookupAllOptions, LookupOptions } from 'node:dns' + + +export default DNSResolver + +declare class DNSResolver { + constructor(opts?: DNSResolver.Options) + lookup( + hostname: string, + family: number, + callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void, + ): void; + lookup( + hostname: string, + options: LookupOneOptions, + callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void, + ): void; + lookup( + hostname: string, + options: LookupAllOptions, + callback: (err: NodeJS.ErrnoException | null, addresses: LookupAddress[]) => void, + ): void; + lookup( + hostname: string, + options: LookupOptions, + callback: (err: NodeJS.ErrnoException | null, address: string | LookupAddress[], family: number) => void, + ): void; + lookup( + hostname: string, + callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void, + ): void; + server: { + get: () => string[]; + set: (servers: string[]) => void; + } + lookupAsync: (hostname: string, options: LookupOptions) => Promise + queryAndCache: (hostname: string) => Promise; + query: (hostname: string) => Promise; + updateInterfaceInfo: () => void; + clear: (hostname?: string) => void; +} + +interface CacheInterface { + set: (key:string, value: LookupAddress[]) => Promise; + get: (key: string) => Promise; + delete: (key?: string) => void; + clear: () => void; +} + +declare namespace DNSResolver { + export interface LookupAddress { + address: string; + family: 4 | 6; + ttl: number; + } + + export interface Options { + lookupOptions: { + family?: 4 | 6 | 0; + hints?: number; + all?: boolean; + }; + maxTtl?: number; + cache?: Map | CacheInterface; + fallbackDuration?: number; + errorTtl?: number; + resolver: typeof promises.Resolver; + lookup: typeof lookup; + roundRobinStrategy: 'first'; + } +} diff --git a/types/pool.d.ts b/types/pool.d.ts index bad5ba0308e..1753343bbb3 100644 --- a/types/pool.d.ts +++ b/types/pool.d.ts @@ -2,6 +2,7 @@ import Client from './client' import TPoolStats from './pool-stats' import { URL } from 'url' import Dispatcher from "./dispatcher"; +import DNSResolver from "./dns-resolver"; export default Pool @@ -34,6 +35,8 @@ declare namespace Pool { /** The max number of clients to create. `null` if no limit. Default `null`. */ connections?: number | null; - interceptors?: { Pool?: readonly Dispatcher.DispatchInterceptor[] } & Client.Options["interceptors"] + interceptors?: { Pool?: readonly Dispatcher.DispatchInterceptor[] } & Client.Options["interceptors"]; + + dnsResolver?: typeof DNSResolver; } } From 13616d3431b97325e27d5708979a22ad1c9e6653 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Tue, 16 Jan 2024 23:57:07 +0000 Subject: [PATCH 18/19] fix comments --- README.md | 25 +++++++++++++++++-------- index.js | 2 +- lib/agent.js | 5 +++-- test/dns-resolver.js | 24 +++++++++++++++++++++++- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index e9b8f45153d..7b609db98c7 100644 --- a/README.md +++ b/README.md @@ -371,27 +371,36 @@ Undici provides DNS caching via the `DNSResolver` class. This functionality is coming from [cacheable-lookup](https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f) package. -By default a global DNSResolver will be created and passed down by the agent to the pool. -Here is an example of how to configure a custom DNSResolver: +By default this functionality is disabled. + +You can enable the default DNSResolver and pass it down by the agent to the pool by setting `dnsResolver` to `true`. ```js new Agent({ - dnsResolverOptions: { - lookupOptions: { family: 6, hints: ALL } - } - }) + dnsResolver: true +}) ``` -To disable DNS caching for an agent: + +Here is an example of how to configure a custom DNSResolver: ```js new Agent({ + dnsResolver: true, dnsResolverOptions: { - disable: true + lookupOptions: { family: 6, hints: ALL } } }) ``` +To disable DNS caching for an agent: + +```js +new Agent({ + dnsResolver: false +}) +``` + Provide your custom DNS resolver, it must implement the method `lookup`: ```js diff --git a/index.js b/index.js index 3ba08d64721..c0bd351ef56 100644 --- a/index.js +++ b/index.js @@ -6,6 +6,7 @@ const errors = require('./lib/core/errors') const Pool = require('./lib/pool') const BalancedPool = require('./lib/balanced-pool') const Agent = require('./lib/agent') +const DNSResolver = require('./lib/dns-resolver') const util = require('./lib/core/util') const { InvalidArgumentError } = errors const api = require('./lib/api') @@ -166,5 +167,4 @@ module.exports.MockPool = MockPool module.exports.MockAgent = MockAgent module.exports.mockErrors = mockErrors -const DNSResolver = require('./lib/dns-resolver') module.exports.DNSResolver = DNSResolver diff --git a/lib/agent.js b/lib/agent.js index fde9634a862..2ae97a61f75 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -24,11 +24,12 @@ function defaultFactory (origin, opts) { } function getDNSResolver (options) { - if (options?.dnsResolverOptions?.disable) { + // by default disable DNSResolver + if (options?.dnsResolver === false || options?.dnsResolver === undefined) { return } - if (options.dnsResolver && typeof options.dnsResolver.lookup === 'function') { + if (typeof options?.dnsResolver?.lookup === 'function') { return options.dnsResolver } diff --git a/test/dns-resolver.js b/test/dns-resolver.js index f567731ea92..8c301c675ea 100644 --- a/test/dns-resolver.js +++ b/test/dns-resolver.js @@ -989,13 +989,35 @@ test('agent: verify DNSResolver is working caching requests', t => { }) }) +test('agent verify DNSResolver is disabled by default', t => { + t.plan(2) + const { Agent, request } = require('../index') + const dnsResolver = new DNSResolver({ resolver }) + dnsResolver.clear() + const agent = new Agent() + t.equal(dnsResolver[kDnsCacheSize], 0) + + const server = http.createServer((req, res) => { + req.pipe(res) + }) + + t.teardown(server.close.bind(server)) + + server.listen(0, async () => { + const origin = `http://localhost:${server.address().port}` + await request(origin, { dispatcher: agent }) + t.equal(dnsResolver[kDnsCacheSize], 0) + t.end() + }) +}) + test('agent verify DNSResolver is disabled', t => { t.plan(2) const { Agent, request } = require('../index') const dnsResolver = new DNSResolver({ resolver }) dnsResolver.clear() const agent = new Agent({ - dnsResolverOptions: { disable: true } + dnsResolver: false }) t.equal(dnsResolver[kDnsCacheSize], 0) From 76e34bb52a58a5d0b173350d1f1218796cdae325 Mon Sep 17 00:00:00 2001 From: Antoine Gomez Date: Sun, 21 Jan 2024 23:04:01 +0000 Subject: [PATCH 19/19] add types --- test/types/agent.test-d.ts | 4 +- test/types/dns-resolver.test-d.ts | 74 +++++++++++++++++++++++++++++++ types/agent.d.ts | 2 +- types/dns-resolver.d.ts | 26 +++++------ types/index.d.ts | 4 +- types/pool.d.ts | 2 +- 6 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 test/types/dns-resolver.test-d.ts diff --git a/test/types/agent.test-d.ts b/test/types/agent.test-d.ts index 5c12c480018..55d02e23b4b 100644 --- a/test/types/agent.test-d.ts +++ b/test/types/agent.test-d.ts @@ -1,12 +1,14 @@ import { Duplex, Readable, Writable } from 'stream' import { expectAssignable } from 'tsd' -import { Agent, Dispatcher } from '../..' +import { Agent, DNSResolver, Dispatcher } from '../..' import { URL } from 'url' expectAssignable(new Agent()) expectAssignable(new Agent({})) expectAssignable(new Agent({ maxRedirections: 1 })) expectAssignable(new Agent({ factory: () => new Dispatcher() })) +expectAssignable(new Agent({ dnsResolver: new DNSResolver() })) +expectAssignable(new Agent({ dnsResolverOptions: { cache: new Map() } })) { const agent = new Agent() diff --git a/test/types/dns-resolver.test-d.ts b/test/types/dns-resolver.test-d.ts new file mode 100644 index 00000000000..2de28d4ce1f --- /dev/null +++ b/test/types/dns-resolver.test-d.ts @@ -0,0 +1,74 @@ +import { expectAssignable, expectType } from 'tsd' +import { DNSResolver } from '../..' +import { LookupAddress } from 'dns' + +type LookupResponse = Promise + +// constructor +expectAssignable(new DNSResolver()) +expectAssignable(new DNSResolver({ + maxTtl: 0, + cache: new Map(), + fallbackDuration: 100, + errorTtl: 10, + scheduling: 'random', + lookupOptions: { + family: 4, + } +})) +expectAssignable(new DNSResolver({ + maxTtl: 0, + fallbackDuration: 100, + errorTtl: 10, +})) +expectAssignable(new DNSResolver({ + maxTtl: 0, + cache: new Map(), + fallbackDuration: 100, + errorTtl: 10, + scheduling: 'random', +})) + +{ + const dnsResolver = new DNSResolver() + + // lookup + expectAssignable(dnsResolver.lookup('localhost', (err, address, family, ttl, expires) => { + expectAssignable(err) + expectAssignable(address) + expectAssignable(family) + expectAssignable(ttl) + expectAssignable(expires) + })) + expectAssignable(dnsResolver.lookup('localhost', { family: 6 }, (err, address, family, ttl, expires) => { + expectAssignable(err) + expectAssignable(address) + expectAssignable(family) + expectAssignable(ttl) + expectAssignable(expires) + })) + expectAssignable(dnsResolver.lookup('localhost', 4, (err, address, family, ttl, expires) => { + expectAssignable(err) + expectAssignable(address) + expectAssignable(family) + expectAssignable(ttl) + expectAssignable(expires) + })) + expectAssignable(dnsResolver.lookup('localhost', { all: true }, (err, entries) => { + expectAssignable(err) + expectAssignable(entries) + })) + + // lookupAsync + expectAssignable(dnsResolver.lookupAsync('localhost', 4)) + expectAssignable>(dnsResolver.lookupAsync('localhost', { family: 6 })) + + // query and cache + expectAssignable(dnsResolver.query('localhost')) + expectAssignable(dnsResolver.queryAndCache('localhost')) + + // other utils + expectAssignable(dnsResolver.servers = ['0.0.0.0']) + expectAssignable(dnsResolver.updateInterfaceInfo()) + expectAssignable(dnsResolver.clear()) +} diff --git a/types/agent.d.ts b/types/agent.d.ts index 9d915367c0b..6ded02dcc00 100644 --- a/types/agent.d.ts +++ b/types/agent.d.ts @@ -24,7 +24,7 @@ declare namespace Agent { interceptors?: { Agent?: readonly Dispatcher.DispatchInterceptor[] } & Pool.Options["interceptors"] - dnsResolverOptions: DNSResolver.Options; + dnsResolverOptions?: DNSResolver.Options; } export interface DispatchOptions extends Dispatcher.DispatchOptions { diff --git a/types/dns-resolver.d.ts b/types/dns-resolver.d.ts index a39c5a4d5e7..f6ff2e90461 100644 --- a/types/dns-resolver.d.ts +++ b/types/dns-resolver.d.ts @@ -8,12 +8,12 @@ declare class DNSResolver { lookup( hostname: string, family: number, - callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void, + callback: (err: NodeJS.ErrnoException | null, address: string, family: number, ttl: number, expires: number) => void, ): void; lookup( hostname: string, options: LookupOneOptions, - callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void, + callback: (err: NodeJS.ErrnoException | null, address: string, family: number, ttl: number, expires: number) => void, ): void; lookup( hostname: string, @@ -27,15 +27,12 @@ declare class DNSResolver { ): void; lookup( hostname: string, - callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void, + callback: (err: NodeJS.ErrnoException | null, address: string, family: number, ttl: number, expires: number) => void, ): void; - server: { - get: () => string[]; - set: (servers: string[]) => void; - } - lookupAsync: (hostname: string, options: LookupOptions) => Promise - queryAndCache: (hostname: string) => Promise; - query: (hostname: string) => Promise; + servers: string[] + lookupAsync: (hostname: string, options: LookupOptions | number) => Promise + queryAndCache: (hostname: string) => Promise; + query: (hostname: string) => Promise; updateInterfaceInfo: () => void; clear: (hostname?: string) => void; } @@ -52,10 +49,11 @@ declare namespace DNSResolver { address: string; family: 4 | 6; ttl: number; + expires: number; } export interface Options { - lookupOptions: { + lookupOptions?: { family?: 4 | 6 | 0; hints?: number; all?: boolean; @@ -64,8 +62,8 @@ declare namespace DNSResolver { cache?: Map | CacheInterface; fallbackDuration?: number; errorTtl?: number; - resolver: typeof promises.Resolver; - lookup: typeof lookup; - roundRobinStrategy: 'first'; + resolver?: typeof promises.Resolver; + lookup?: typeof lookup; + scheduling?: 'first' | 'random'; } } diff --git a/types/index.d.ts b/types/index.d.ts index 8b35475219b..106e582c142 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -16,6 +16,7 @@ import mockErrors from'./mock-errors' import ProxyAgent from'./proxy-agent' import RetryHandler from'./retry-handler' import { request, pipeline, stream, connect, upgrade } from './api' +import DNSResolver from './dns-resolver' export * from './util' export * from './cookies' @@ -29,7 +30,7 @@ export * from './content-type' export * from './cache' export { Interceptable } from './mock-interceptor' -export { Dispatcher, BalancedPool, Pool, Client, buildConnector, errors, Agent, request, stream, pipeline, connect, upgrade, setGlobalDispatcher, getGlobalDispatcher, setGlobalOrigin, getGlobalOrigin, MockClient, MockPool, MockAgent, mockErrors, ProxyAgent, RedirectHandler, DecoratorHandler, RetryHandler } +export { Dispatcher, BalancedPool, Pool, Client, buildConnector, errors, Agent, request, stream, pipeline, connect, upgrade, setGlobalDispatcher, getGlobalDispatcher, setGlobalOrigin, getGlobalOrigin, MockClient, MockPool, MockAgent, mockErrors, ProxyAgent, RedirectHandler, DecoratorHandler, RetryHandler, DNSResolver } export default Undici declare namespace Undici { @@ -63,4 +64,5 @@ declare namespace Undici { var File: typeof import('./file').File; var FileReader: typeof import('./filereader').FileReader; var caches: typeof import('./cache').caches; + var DNSResolver: typeof import('./dns-resolver').default; } diff --git a/types/pool.d.ts b/types/pool.d.ts index 1753343bbb3..a8caa1543d2 100644 --- a/types/pool.d.ts +++ b/types/pool.d.ts @@ -37,6 +37,6 @@ declare namespace Pool { interceptors?: { Pool?: readonly Dispatcher.DispatchInterceptor[] } & Client.Options["interceptors"]; - dnsResolver?: typeof DNSResolver; + dnsResolver?: DNSResolver; } }