From 12fb94c78c7c0e5944db425f40c4bf3ed61b4f76 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 13 Sep 2022 21:36:08 +0800 Subject: [PATCH] dns: refactor default resolver This patch refactors the DNS default resolver code to make it easier to be included in a snapshot: - The code specific for the callback-based DNS resolver are not in a separate module to make the dependency clearer (it's not actually needed if the user only ever loads `dns/promises`) - The common part of the callback-based resolver and the promise- based resolver is now ResolverBase. The other two user-facing resolvers are now subclasses of ResolverBase. The default Resolver is constructed with just ResolverBase. This would be fine as the default resolver is never actually exposed to the user-land and it has been working using duck-typing anyway. - Move the construction of Resolver subclasses into a common method `createResolverClass()` to reduce code duplication. The two subclasses now also share the same base constructor. This would make it possible for them to also share code for snapshot support later. - `--dns-result-order` is now queried and refreshed during pre-execution. To avoid loading the cares_wrap binding unnecessarily the loading of the binding is also made lazy. PR-URL: https://github.com/nodejs/node/pull/44541 Reviewed-By: Matteo Collina Reviewed-By: Zeyu "Alex" Yang Reviewed-By: Minwoo Jung --- lib/dns.js | 97 +------------------- lib/internal/dns/callback_resolver.js | 116 ++++++++++++++++++++++++ lib/internal/dns/promises.js | 44 ++------- lib/internal/dns/utils.js | 69 +++++++++++--- lib/internal/main/worker_thread.js | 3 + lib/internal/process/pre_execution.js | 2 + test/parallel/test-bootstrap-modules.js | 1 + 7 files changed, 191 insertions(+), 141 deletions(-) create mode 100644 lib/internal/dns/callback_resolver.js diff --git a/lib/dns.js b/lib/dns.js index 5fe34b4b049358..3a334b81edcd0f 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -22,11 +22,8 @@ 'use strict'; const { - ArrayPrototypeMap, - ObjectCreate, ObjectDefineProperties, ObjectDefineProperty, - ReflectApply, Symbol, } = primordials; @@ -37,15 +34,16 @@ const { customPromisifyArgs } = require('internal/util'); const errors = require('internal/errors'); const { bindDefaultResolver, - getDefaultResolver, setDefaultResolver, - Resolver, validateHints, emitInvalidHostnameWarning, getDefaultVerbatim, setDefaultResultOrder, errorCodes: dnsErrorCodes, } = require('internal/dns/utils'); +const { + Resolver +} = require('internal/dns/callback_resolver'); const { NODATA, FORMERR, @@ -89,12 +87,10 @@ const { const { GetAddrInfoReqWrap, GetNameInfoReqWrap, - QueryReqWrap, } = cares; const kPerfHooksDnsLookupContext = Symbol('kPerfHooksDnsLookupContext'); const kPerfHooksDnsLookupServiceContext = Symbol('kPerfHooksDnsLookupServiceContext'); -const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext'); const { hasObserver, @@ -293,91 +289,6 @@ function lookupService(address, port, callback) { ObjectDefineProperty(lookupService, customPromisifyArgs, { __proto__: null, value: ['hostname', 'service'], enumerable: false }); - -function onresolve(err, result, ttls) { - if (ttls && this.ttl) - result = ArrayPrototypeMap( - result, (address, index) => ({ address, ttl: ttls[index] })); - - if (err) - this.callback(dnsException(err, this.bindingName, this.hostname)); - else { - this.callback(null, result); - if (this[kPerfHooksDnsLookupResolveContext] && hasObserver('dns')) { - stopPerf(this, kPerfHooksDnsLookupResolveContext, { detail: { result } }); - } - } -} - -function resolver(bindingName) { - function query(name, /* options, */ callback) { - let options; - if (arguments.length > 2) { - options = callback; - callback = arguments[2]; - } - - validateString(name, 'name'); - validateFunction(callback, 'callback'); - - const req = new QueryReqWrap(); - req.bindingName = bindingName; - req.callback = callback; - req.hostname = name; - req.oncomplete = onresolve; - req.ttl = !!(options && options.ttl); - const err = this._handle[bindingName](req, toASCII(name)); - if (err) throw dnsException(err, bindingName, name); - if (hasObserver('dns')) { - startPerf(req, kPerfHooksDnsLookupResolveContext, { - type: 'dns', - name: bindingName, - detail: { - host: name, - ttl: req.ttl, - }, - }); - } - return req; - } - ObjectDefineProperty(query, 'name', { __proto__: null, value: bindingName }); - return query; -} - -const resolveMap = ObjectCreate(null); -Resolver.prototype.resolveAny = resolveMap.ANY = resolver('queryAny'); -Resolver.prototype.resolve4 = resolveMap.A = resolver('queryA'); -Resolver.prototype.resolve6 = resolveMap.AAAA = resolver('queryAaaa'); -Resolver.prototype.resolveCaa = resolveMap.CAA = resolver('queryCaa'); -Resolver.prototype.resolveCname = resolveMap.CNAME = resolver('queryCname'); -Resolver.prototype.resolveMx = resolveMap.MX = resolver('queryMx'); -Resolver.prototype.resolveNs = resolveMap.NS = resolver('queryNs'); -Resolver.prototype.resolveTxt = resolveMap.TXT = resolver('queryTxt'); -Resolver.prototype.resolveSrv = resolveMap.SRV = resolver('querySrv'); -Resolver.prototype.resolvePtr = resolveMap.PTR = resolver('queryPtr'); -Resolver.prototype.resolveNaptr = resolveMap.NAPTR = resolver('queryNaptr'); -Resolver.prototype.resolveSoa = resolveMap.SOA = resolver('querySoa'); -Resolver.prototype.reverse = resolver('getHostByAddr'); - -Resolver.prototype.resolve = resolve; - -function resolve(hostname, rrtype, callback) { - let resolver; - if (typeof rrtype === 'string') { - resolver = resolveMap[rrtype]; - } else if (typeof rrtype === 'function') { - resolver = resolveMap.A; - callback = rrtype; - } else { - throw new ERR_INVALID_ARG_TYPE('rrtype', 'string', rrtype); - } - - if (typeof resolver === 'function') { - return ReflectApply(resolver, this, [hostname, callback]); - } - throw new ERR_INVALID_ARG_VALUE('rrtype', rrtype); -} - function defaultResolverSetServers(servers) { const resolver = new Resolver(); @@ -429,7 +340,7 @@ module.exports = { CANCELLED, }; -bindDefaultResolver(module.exports, getDefaultResolver()); +bindDefaultResolver(module.exports, Resolver.prototype); ObjectDefineProperties(module.exports, { promises: { diff --git a/lib/internal/dns/callback_resolver.js b/lib/internal/dns/callback_resolver.js new file mode 100644 index 00000000000000..221cc9a76c70b3 --- /dev/null +++ b/lib/internal/dns/callback_resolver.js @@ -0,0 +1,116 @@ +'use strict'; + +const { + ObjectDefineProperty, + ReflectApply, + ArrayPrototypeMap, + Symbol +} = primordials; + +const { toASCII } = require('internal/idna'); + +const { + codes: { + ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, + }, + dnsException +} = require('internal/errors'); + +const { + createResolverClass, +} = require('internal/dns/utils'); + +const { + validateFunction, + validateString, +} = require('internal/validators'); + +const { + QueryReqWrap, +} = internalBinding('cares_wrap'); + +const { + hasObserver, + startPerf, + stopPerf, +} = require('internal/perf/observe'); + +const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext'); + +function onresolve(err, result, ttls) { + if (ttls && this.ttl) + result = ArrayPrototypeMap( + result, (address, index) => ({ address, ttl: ttls[index] })); + + if (err) + this.callback(dnsException(err, this.bindingName, this.hostname)); + else { + this.callback(null, result); + if (this[kPerfHooksDnsLookupResolveContext] && hasObserver('dns')) { + stopPerf(this, kPerfHooksDnsLookupResolveContext, { detail: { result } }); + } + } +} + +function resolver(bindingName) { + function query(name, /* options, */ callback) { + let options; + if (arguments.length > 2) { + options = callback; + callback = arguments[2]; + } + + validateString(name, 'name'); + validateFunction(callback, 'callback'); + + const req = new QueryReqWrap(); + req.bindingName = bindingName; + req.callback = callback; + req.hostname = name; + req.oncomplete = onresolve; + req.ttl = !!(options && options.ttl); + const err = this._handle[bindingName](req, toASCII(name)); + if (err) throw dnsException(err, bindingName, name); + if (hasObserver('dns')) { + startPerf(req, kPerfHooksDnsLookupResolveContext, { + type: 'dns', + name: bindingName, + detail: { + host: name, + ttl: req.ttl, + }, + }); + } + return req; + } + ObjectDefineProperty(query, 'name', { __proto__: null, value: bindingName }); + return query; +} + +// This is the callback-based resolver. There is another similar +// resolver in dns/promises.js with resolve methods that are based +// on promises instead. +const { Resolver, resolveMap } = createResolverClass(resolver); +Resolver.prototype.resolve = resolve; + +function resolve(hostname, rrtype, callback) { + let resolver; + if (typeof rrtype === 'string') { + resolver = resolveMap[rrtype]; + } else if (typeof rrtype === 'function') { + resolver = resolveMap.A; + callback = rrtype; + } else { + throw new ERR_INVALID_ARG_TYPE('rrtype', 'string', rrtype); + } + + if (typeof resolver === 'function') { + return ReflectApply(resolver, this, [hostname, callback]); + } + throw new ERR_INVALID_ARG_VALUE('rrtype', rrtype); +} + +module.exports = { + Resolver +}; diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js index ac40bc5541b997..a94f4c893530e5 100644 --- a/lib/internal/dns/promises.js +++ b/lib/internal/dns/promises.js @@ -1,7 +1,6 @@ 'use strict'; const { ArrayPrototypeMap, - ObjectCreate, ObjectDefineProperty, Promise, ReflectApply, @@ -10,16 +9,15 @@ const { const { bindDefaultResolver, - Resolver: CallbackResolver, + createResolverClass, validateHints, - validateTimeout, - validateTries, emitInvalidHostnameWarning, getDefaultVerbatim, errorCodes: dnsErrorCodes, setDefaultResultOrder, setDefaultResolver, } = require('internal/dns/utils'); + const { NODATA, FORMERR, @@ -52,7 +50,6 @@ const { isIP } = require('internal/net'); const { getaddrinfo, getnameinfo, - ChannelWrap, GetAddrInfoReqWrap, GetNameInfoReqWrap, QueryReqWrap @@ -305,36 +302,7 @@ function resolver(bindingName) { return query; } - -const resolveMap = ObjectCreate(null); - -// Resolver instances correspond 1:1 to c-ares channels. -class Resolver { - constructor(options = undefined) { - const timeout = validateTimeout(options); - const tries = validateTries(options); - this._handle = new ChannelWrap(timeout, tries); - } -} - -Resolver.prototype.getServers = CallbackResolver.prototype.getServers; -Resolver.prototype.setServers = CallbackResolver.prototype.setServers; -Resolver.prototype.cancel = CallbackResolver.prototype.cancel; -Resolver.prototype.setLocalAddress = CallbackResolver.prototype.setLocalAddress; -Resolver.prototype.resolveAny = resolveMap.ANY = resolver('queryAny'); -Resolver.prototype.resolve4 = resolveMap.A = resolver('queryA'); -Resolver.prototype.resolve6 = resolveMap.AAAA = resolver('queryAaaa'); -Resolver.prototype.resolveCaa = resolveMap.CAA = resolver('queryCaa'); -Resolver.prototype.resolveCname = resolveMap.CNAME = resolver('queryCname'); -Resolver.prototype.resolveMx = resolveMap.MX = resolver('queryMx'); -Resolver.prototype.resolveNs = resolveMap.NS = resolver('queryNs'); -Resolver.prototype.resolveTxt = resolveMap.TXT = resolver('queryTxt'); -Resolver.prototype.resolveSrv = resolveMap.SRV = resolver('querySrv'); -Resolver.prototype.resolvePtr = resolveMap.PTR = resolver('queryPtr'); -Resolver.prototype.resolveNaptr = resolveMap.NAPTR = resolver('queryNaptr'); -Resolver.prototype.resolveSoa = resolveMap.SOA = resolver('querySoa'); -Resolver.prototype.reverse = resolver('getHostByAddr'); -Resolver.prototype.resolve = function resolve(hostname, rrtype) { +function resolve(hostname, rrtype) { let resolver; if (rrtype !== undefined) { @@ -349,7 +317,11 @@ Resolver.prototype.resolve = function resolve(hostname, rrtype) { } return ReflectApply(resolver, this, [hostname]); -}; +} + +// Promise-based resolver. +const { Resolver, resolveMap } = createResolverClass(resolver); +Resolver.prototype.resolve = resolve; function defaultResolverSetServers(servers) { const resolver = new Resolver(); diff --git a/lib/internal/dns/utils.js b/lib/internal/dns/utils.js index adc50def9d6058..f895a46c1a037e 100644 --- a/lib/internal/dns/utils.js +++ b/lib/internal/dns/utils.js @@ -9,6 +9,7 @@ const { NumberParseInt, RegExpPrototypeExec, RegExpPrototypeSymbolReplace, + ObjectCreate, } = primordials; const errors = require('internal/errors'); @@ -20,13 +21,11 @@ const { validateOneOf, validateString, } = require('internal/validators'); -const { - ChannelWrap, - strerror, - AI_ADDRCONFIG, - AI_ALL, - AI_V4MAPPED, -} = internalBinding('cares_wrap'); +let binding; +function lazyBinding() { + binding ??= internalBinding('cares_wrap'); + return binding; +} const IANA_DNS_PORT = 53; const IPv6RE = /^\[([^[\]]*)\]/; const addrSplitRE = /(^.+?)(?::(\d+))?$/; @@ -49,10 +48,12 @@ function validateTries(options) { } // Resolver instances correspond 1:1 to c-ares channels. -class Resolver { + +class ResolverBase { constructor(options = undefined) { const timeout = validateTimeout(options); const tries = validateTries(options); + const { ChannelWrap } = lazyBinding(); this._handle = new ChannelWrap(timeout, tries); } @@ -122,6 +123,7 @@ class Resolver { if (errorNumber !== 0) { // Reset the servers to the old servers, because ares probably unset them. this._handle.setServers(ArrayPrototypeJoin(orig, ',')); + const { strerror } = lazyBinding(); const err = strerror(errorNumber); throw new ERR_DNS_SET_SERVERS_FAILED(err, servers); } @@ -138,7 +140,19 @@ class Resolver { } } -let defaultResolver = new Resolver(); +let defaultResolver; +let dnsOrder; + +function initializeDns() { + const orderFromCLI = getOptionValue('--dns-result-order'); + if (!orderFromCLI) { + dnsOrder ??= 'verbatim'; + } else { + // Allow the deserialized application to override order from CLI. + dnsOrder = orderFromCLI; + } +} + const resolverKeys = [ 'getServers', 'resolve', @@ -158,6 +172,11 @@ const resolverKeys = [ ]; function getDefaultResolver() { + // We do this here instead of pre-execution so that the default resolver is + // only ever created when the user loads any dns module. + if (defaultResolver === undefined) { + defaultResolver = new ResolverBase(); + } return defaultResolver; } @@ -166,12 +185,14 @@ function setDefaultResolver(resolver) { } function bindDefaultResolver(target, source) { + const defaultResolver = getDefaultResolver(); ArrayPrototypeForEach(resolverKeys, (key) => { target[key] = FunctionPrototypeBind(source[key], defaultResolver); }); } function validateHints(hints) { + const { AI_ADDRCONFIG, AI_ALL, AI_V4MAPPED } = lazyBinding(); if ((hints & ~(AI_ADDRCONFIG | AI_ALL | AI_V4MAPPED)) !== 0) { throw new ERR_INVALID_ARG_VALUE('hints', hints); } @@ -190,8 +211,6 @@ function emitInvalidHostnameWarning(hostname) { } } -let dnsOrder = getOptionValue('--dns-result-order') || 'verbatim'; - function getDefaultVerbatim() { return dnsOrder !== 'ipv4first'; } @@ -201,6 +220,31 @@ function setDefaultResultOrder(value) { dnsOrder = value; } +function createResolverClass(resolver) { + const resolveMap = ObjectCreate(null); + + class Resolver extends ResolverBase {} + + Resolver.prototype.resolveAny = resolveMap.ANY = resolver('queryAny'); + Resolver.prototype.resolve4 = resolveMap.A = resolver('queryA'); + Resolver.prototype.resolve6 = resolveMap.AAAA = resolver('queryAaaa'); + Resolver.prototype.resolveCaa = resolveMap.CAA = resolver('queryCaa'); + Resolver.prototype.resolveCname = resolveMap.CNAME = resolver('queryCname'); + Resolver.prototype.resolveMx = resolveMap.MX = resolver('queryMx'); + Resolver.prototype.resolveNs = resolveMap.NS = resolver('queryNs'); + Resolver.prototype.resolveTxt = resolveMap.TXT = resolver('queryTxt'); + Resolver.prototype.resolveSrv = resolveMap.SRV = resolver('querySrv'); + Resolver.prototype.resolvePtr = resolveMap.PTR = resolver('queryPtr'); + Resolver.prototype.resolveNaptr = resolveMap.NAPTR = resolver('queryNaptr'); + Resolver.prototype.resolveSoa = resolveMap.SOA = resolver('querySoa'); + Resolver.prototype.reverse = resolver('getHostByAddr'); + + return { + resolveMap, + Resolver + }; +} + // ERROR CODES const errorCodes = { NODATA: 'ENODATA', @@ -236,9 +280,10 @@ module.exports = { validateHints, validateTimeout, validateTries, - Resolver, emitInvalidHostnameWarning, getDefaultVerbatim, setDefaultResultOrder, errorCodes, + createResolverClass, + initializeDns }; diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 41cf85ccda8ff2..f7ead4084ed4ed 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -134,6 +134,9 @@ port.on('message', (message) => { } initializeDeprecations(); initializeWASI(); + + require('internal/dns/utils').initializeDns(); + initializeCJSLoader(); initializeESMLoader(); diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index d2f2dad8dd445a..23d4dbcf6cd8e1 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -89,6 +89,8 @@ function prepareMainThreadExecution(expandArgv1 = false, initializeDeprecations(); initializeWASI(); + require('internal/dns/utils').initializeDns(); + require('internal/v8/startup_snapshot').runDeserializeCallbacks(); if (!initialzeModules) { diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index e2a39dd4e9e144..e2fc58cdf26f57 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -57,6 +57,7 @@ const expectedModules = new Set([ 'NativeModule internal/console/constructor', 'NativeModule internal/console/global', 'NativeModule internal/constants', + 'NativeModule internal/dns/utils', 'NativeModule internal/encoding', 'NativeModule internal/errors', 'NativeModule internal/event_target',