diff --git a/CLAUDE.md b/CLAUDE.md index 30ff0ad..5f6d5e3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,7 @@ formatAddress -> resolvePolicy -> resolveMX -> validateMxPolicy -> resolveIP -> - `resolve-mx.js` - Async DNS MX record resolution with fallback to A/AAAA records - `resolve-ip.js` - Async resolution of MX hostnames to IPv4/IPv6 addresses (parallel) - `get-connection.js` - Async iteration through MX hosts attempting TCP connections -- `tools.js` - Shared utilities: `getDnsResolver` (promisifies custom DNS resolvers or uses native `dns.promises`), `isNotFoundError`, IP validation (`isLocal`, `isInvalid`) +- `tools.js` - Shared utilities: `getDnsResolver` (selects the DNS resolver, see below), `isNotFoundError`, IP validation (`isLocal`, `isInvalid`, `checkAddress`) - `dns-errors.js` / `net-errors.js` - Error code to message mappings **Key data structure:** The `delivery` object flows through the pipeline, accumulating: @@ -75,7 +75,7 @@ formatAddress -> resolvePolicy -> resolveMX -> validateMxPolicy -> resolveIP -> **MTA-STS integration:** Uses `mailauth` library for policy fetching and MX validation. Policies are cached via user-provided cache handlers. -**Custom DNS resolvers:** The library accepts callback-style custom resolvers via `dnsOptions.resolve`. These are automatically promisified internally. When no custom resolver is provided, native `dns.promises` is used. +**Custom DNS resolvers:** `tools.getDnsResolver(dnsOptions)` builds the one resolver the whole pipeline uses, choosing between `dnsOptions.resolveRecords`, the older callback-style `dnsOptions.resolve`, and native `dns.promises`. The user-facing contract for both options, including why the callback form still omits the record type for A lookups, is documented in the README under Custom DNS resolver. TLSA lookups do not go through here; they have their own `dane.resolveTlsa`. **Async conventions:** All asynchronous code uses async/await. Raw promise primitives are limited to justified boundaries: `new Promise` wrappers where callback-style user APIs (custom DNS resolvers, `connectHook`) or event-based APIs (`net.connect` with timeout race) meet async code, `Promise.all` for parallel fan-out, and the `.then()` bridge in `mx-connect.js` that feeds the public callback API. diff --git a/README.md b/README.md index 3f36337..65cd11b 100644 --- a/README.md +++ b/README.md @@ -85,16 +85,66 @@ The family-specific options let you bind a different source address per family. ### dnsOptions -| Option | Type | Default | Description | -| ----------------------- | -------- | ------------- | ----------------------------------------------------------------------------------------------- | -| `ignoreIPv6` | boolean | `false` | Never use IPv6 for sending. See [below](#ignoreipv6) | -| `preferIPv6` | boolean | `false` | Try IPv6 addresses before IPv4 when a host has both | -| `blockLocalAddresses` | boolean | `false` | Refuse local and private scope addresses. See [Address validation](#address-validation) | -| `blockReservedNetworks` | boolean | `false` | Refuse IANA special-purpose addresses. See [Address validation](#address-validation) | -| `nat64Prefixes` | string[] | `[]` | NAT64 prefixes your own network runs. See [NAT64 on your own prefix](#nat64-on-your-own-prefix) | -| `resolve` | function | `dns.resolve` | Custom callback-style DNS resolver | - -A custom `resolve` function is called as `resolve(domain, type, callback)`, except for A record lookups which always use `resolve(domain, callback)`. Other record types (`MX`, `AAAA`, `TXT`) always pass the type. +| Option | Type | Default | Description | +| ----------------------- | -------- | ------- | ----------------------------------------------------------------------------------------------- | +| `ignoreIPv6` | boolean | `false` | Never use IPv6 for sending. See [below](#ignoreipv6) | +| `preferIPv6` | boolean | `false` | Try IPv6 addresses before IPv4 when a host has both | +| `blockLocalAddresses` | boolean | `false` | Refuse local and private scope addresses. See [Address validation](#address-validation) | +| `blockReservedNetworks` | boolean | `false` | Refuse IANA special-purpose addresses. See [Address validation](#address-validation) | +| `nat64Prefixes` | string[] | `[]` | NAT64 prefixes your own network runs. See [NAT64 on your own prefix](#nat64-on-your-own-prefix) | +| `resolveRecords` | function | | DNS resolver returning the records. See [Custom DNS resolver](#custom-dns-resolver) | +| `resolve` | function | | Callback-style DNS resolver. See [Custom DNS resolver](#custom-dns-resolver) | + +With neither set, native `dns.promises` is used. + +#### Custom DNS resolver + +`resolveRecords` receives a domain and a record type and returns the records: + +```javascript +const connection = await mxConnect({ + target: 'user@example.com', + dnsOptions: { + async resolveRecords(domain, type) { + // type is 'MX', 'A', 'AAAA' or 'TXT' + return myResolver.lookup(domain, type); + } + } +}); +``` + +Returning the records directly is fine too, so a resolver backed by a cache does not have to pretend to be asynchronous: + +```javascript +const dnsOptions = { + resolveRecords: (domain, type) => cache.get(`${domain}:${type}`) ?? [] +}; +``` + +Throwing, or returning a rejected promise, is how you report a lookup failure. Set `err.code` to `ENOTFOUND` or `ENODATA` to say "no records of this type", which lets resolution fall through to the next step; any other code is treated as a real DNS failure. + +> [!TIP] +> `resolveRecords` always receives an explicit record type, A lookups included. + +The older `resolve` option takes a callback and is still supported: + +```javascript +const dnsOptions = { + resolve(domain, type, callback) { + // A lookups arrive as resolve(domain, callback), with no type + myResolver.lookup(domain, type, callback); + } +}; +``` + +It is called as `resolve(domain, type, callback)`, except for A records where it is called as `resolve(domain, callback)` with no type at all. That quirk is why `resolveRecords` exists; prefer it for new code. + +> [!NOTE] +> If both are set, `resolveRecords` is used and `resolve` is ignored, so you can migrate one deployment at a time. + +Setting either option to something that is not a function throws, rather than quietly falling back to the system resolver. A mistyped option would otherwise send mail through a resolver you did not choose, losing whatever yours was there for, and nothing would say so. Leaving an option unset, or setting it to `null`, is not a mistake and falls through as normal. + +Neither option covers TLSA lookups for DANE, which have their own [`dane.resolveTlsa`](#custom-tlsa-resolver). Wiring up a single resolver here does not route those through it. #### ignoreIPv6 diff --git a/lib/mx-connect.js b/lib/mx-connect.js index ef4a0ff..861cbb7 100644 --- a/lib/mx-connect.js +++ b/lib/mx-connect.js @@ -56,7 +56,7 @@ const EMPTY_CACHE_HANDLER = { */ function createPolicyResolver(delivery) { const dnsOptions = delivery.dnsOptions || {}; - const dnsResolve = tools.getDnsResolver(dnsOptions.resolve); + const dnsResolve = tools.getDnsResolver(dnsOptions); return async (domain, type) => { // mailauth falls back to AAAA when the A lookup comes back empty, which would @@ -72,7 +72,15 @@ function createPolicyResolver(delivery) { return list; } - return (list || []).filter(ip => { + // Anything but a real array is not an answer this can vet. A custom resolver + // returning an object with its own filter method would otherwise choose what + // survives, and mailauth connects to whatever comes back, so the check has to run + // over an array we recognise rather than one the value supplies. + if (!Array.isArray(list)) { + return []; + } + + return list.filter(ip => { const invalid = tools.checkAddress(delivery, domain, ip); if (!invalid) { return true; @@ -558,7 +566,9 @@ async function runPipeline(delivery) { * @param {boolean} [options.dnsOptions.blockReservedNetworks=false] - Block IANA special-purpose IPs * @param {Array} [options.dnsOptions.nat64Prefixes] - NAT64 prefixes this network runs, * in CIDR form, so the IPv4 address such an address carries is validated too - * @param {Function} [options.dnsOptions.resolve] - Custom DNS resolver + * @param {Function} [options.dnsOptions.resolveRecords] - DNS resolver returning records, or a + * promise for them: (domain, type) => records + * @param {Function} [options.dnsOptions.resolve] - Callback-based DNS resolver (legacy) * @param {Array} [options.mx] - Pre-resolved MX entries (skips DNS MX lookup) * @param {Array} [options.ignoreMXHosts] - IP addresses to skip * @param {Function} [options.connectHook] - Pre-connection hook: (delivery, options, callback) diff --git a/lib/resolve-ip.js b/lib/resolve-ip.js index a9088c7..67bd76f 100644 --- a/lib/resolve-ip.js +++ b/lib/resolve-ip.js @@ -100,7 +100,8 @@ async function diagnoseSkippedIPv6(skipped, dnsResolve, filterAddress) { * @param {Array} delivery.mx - Array of MX entries with exchange hostnames * @param {Object} [delivery.dnsOptions] - DNS configuration options * @param {boolean} [delivery.dnsOptions.ignoreIPv6=false] - Skip AAAA lookups - * @param {Function} [delivery.dnsOptions.resolve] - Custom DNS resolver + * @param {Function} [delivery.dnsOptions.resolveRecords] - Promise-based DNS resolver + * @param {Function} [delivery.dnsOptions.resolve] - Callback-based DNS resolver (legacy) * @returns {Promise} Delivery object with populated A/AAAA arrays * @throws {Error} If no valid IP addresses can be resolved (error.category = 'dns') */ @@ -142,7 +143,7 @@ async function resolveIP(delivery) { }; const dnsOptions = delivery.dnsOptions || {}; - const dnsResolve = tools.getDnsResolver(dnsOptions.resolve); + const dnsResolve = tools.getDnsResolver(dnsOptions); // Hosts whose AAAA lookup ignoreIPv6 skipped, kept only to explain a failed delivery const skippedIPv6 = []; diff --git a/lib/resolve-mx.js b/lib/resolve-mx.js index 603e12c..65c2140 100644 --- a/lib/resolve-mx.js +++ b/lib/resolve-mx.js @@ -105,13 +105,13 @@ function isRecoverableError(err) { * * @param {Function} dnsResolve - Promise-based DNS resolver function * @param {string} domain - Domain to resolve - * @param {string} [type] - Record type (MX, AAAA). Omit for A records. + * @param {string} type - Record type (MX, A, AAAA) * @returns {Promise<{list: Array, error: Error|null}>} Resolution result with list and error * @private */ async function tryResolve(dnsResolve, domain, type) { try { - const list = type !== undefined ? await dnsResolve(domain, type) : await dnsResolve(domain); + const list = await dnsResolve(domain, type); return { list: list || [], error: null }; } catch (err) { return { list: [], error: err }; @@ -135,7 +135,8 @@ async function tryResolve(dnsResolve, domain, type) { * @param {boolean} delivery.isIp - True if target is already an IP address * @param {Object} [delivery.dnsOptions] - DNS configuration options * @param {boolean} [delivery.dnsOptions.ignoreIPv6=false] - Skip AAAA record lookup - * @param {Function} [delivery.dnsOptions.resolve] - Custom DNS resolver + * @param {Function} [delivery.dnsOptions.resolveRecords] - Promise-based DNS resolver + * @param {Function} [delivery.dnsOptions.resolve] - Callback-based DNS resolver (legacy) * @returns {Promise} Delivery object with populated mx array * @throws {Error} If no valid MX servers can be resolved (error.category = 'dns') */ @@ -189,7 +190,7 @@ async function resolveMX(delivery) { } const domain = delivery.decodedDomain; - const dnsResolve = tools.getDnsResolver(dnsOptions.resolve); + const dnsResolve = tools.getDnsResolver(dnsOptions); // Step 1: Try MX records (canonical mail server entries) const mxResult = await tryResolve(dnsResolve, domain, 'MX'); @@ -225,7 +226,7 @@ async function resolveMX(delivery) { } // Step 2: Fallback to A records (RFC 5321 Section 5.1 implicit MX) - const aResult = await tryResolve(dnsResolve, domain); + const aResult = await tryResolve(dnsResolve, domain, 'A'); if (aResult.list.length) { // RFC 5321 Section 5.1: the domain itself is a single implicit mail exchanger, so all // of its addresses belong to one entry. Giving each address an entry of its own would diff --git a/lib/tools.js b/lib/tools.js index 27d534f..c8ff933 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -16,52 +16,65 @@ const { promises: dnsPromises } = require('dns'); const LOCAL_ADDRESSES = collectLocalAddresses(os.networkInterfaces()); /** - * Creates a promise-based DNS resolver function. + * Builds the promise-based DNS resolver the pipeline uses. * - * When no custom resolver is provided, uses native dns.promises for optimal performance. - * When a custom callback-style resolver is provided (via dnsOptions.resolve), wraps it - * with promisification to maintain a consistent async interface. + * Three sources, in order of preference: * - * The returned resolver accepts an optional record type. An omitted type and an - * explicit 'A' are equivalent: both resolve A records (IPv4). For custom - * resolvers, A lookups always use the legacy two-argument form - * (domain, callback), so resolvers that only implement that form keep working. + * 1. `dnsOptions.resolveRecords`, which returns the records. It may return them directly or + * as a promise; awaiting covers both, so a resolver backed by a cache can answer + * synchronously without pretending to be asynchronous. + * 2. `dnsOptions.resolve`, the older callback form, promisified here. + * 3. Native `dns.promises`, which avoids the callback hop entirely. * - * @param {Function} [customResolver] - Optional callback-style DNS resolver with signature - * (domain, callback) or (domain, type, callback). If not provided, uses dns.promises. + * An omitted record type means A, which is defaulted once here so each branch below can + * just read it. Only the callback form still omits the type for A lookups, because + * resolvers written against it may accept nothing else, and that is precisely the wart the + * newer option exists to leave behind. + * + * @param {Object} [dnsOptions] - DNS configuration + * @param {Function} [dnsOptions.resolveRecords] - (domain, type) => records or Promise + * @param {Function} [dnsOptions.resolve] - Callback resolver, (domain, type, callback) or + * (domain, callback) for A records * @returns {Function} Promise-based resolver: (domain, type?) => Promise + * @throws {TypeError} If a resolver option is set to something that cannot be called * * @example - * // Using default resolver - * const resolve = getDnsResolver(); + * const resolve = getDnsResolver({ resolveRecords: (domain, type) => myCache.lookup(domain, type) }); * const mxRecords = await resolve('example.com', 'MX'); - * - * @example - * // Using custom resolver - * const resolve = getDnsResolver(myCustomDnsResolve); - * const ipAddresses = await resolve('example.com'); */ -function getDnsResolver(customResolver) { - // Use native dns.promises when no custom resolver - faster and avoids callback overhead - if (!customResolver) { - return (domain, type) => { - if (type === undefined || type === 'A') { - return dnsPromises.resolve4(domain); - } - return dnsPromises.resolve(domain, type); - }; +function getDnsResolver(dnsOptions) { + const options = dnsOptions || {}; + + // Falling back to the system resolver because an option was mistyped would quietly send + // mail through a resolver the operator did not choose, losing whatever the configured + // one was there for. Say so instead. + for (const name of ['resolveRecords', 'resolve']) { + if (options[name] !== undefined && options[name] !== null && typeof options[name] !== 'function') { + throw new TypeError(`dnsOptions.${name} must be a function, got ${typeof options[name]}`); + } } - // Promisify custom callback-style resolver - return (domain, type) => - new Promise((resolve, reject) => { - const callback = (err, data) => (err ? reject(err) : resolve(data)); - if (type === undefined || type === 'A') { - customResolver(domain, callback); - } else { - customResolver(domain, type, callback); - } - }); + if (typeof options.resolveRecords === 'function') { + // async so a resolver that returns records directly, or throws synchronously, still + // behaves like every other branch here + return async (domain, type = 'A') => options.resolveRecords(domain, type); + } + + if (typeof options.resolve === 'function') { + return (domain, type = 'A') => + new Promise((resolve, reject) => { + const callback = (err, data) => (err ? reject(err) : resolve(data)); + // The legacy quirk, contained to this one line + if (type === 'A') { + options.resolve(domain, callback); + } else { + options.resolve(domain, type, callback); + } + }); + } + + // Native dns.promises - faster, and avoids the callback hop + return (domain, type = 'A') => (type === 'A' ? dnsPromises.resolve4(domain) : dnsPromises.resolve(domain, type)); } /** diff --git a/test/mx-connect-test.js b/test/mx-connect-test.js index 4f3b262..87e85aa 100644 --- a/test/mx-connect-test.js +++ b/test/mx-connect-test.js @@ -4,7 +4,15 @@ const { test } = require('node:test'); const assert = require('node:assert'); const mxConnect = require('../lib/mx-connect'); -const { createMockDnsResolver, createTrackingDnsResolver, createMockSocket, startGreetingServer, closeServer } = require('./test-utils'); +const { + createMockDnsResolver, + createTrackingDnsResolver, + createMockRecordResolver, + createMockConnectHook, + createMockSocket, + startGreetingServer, + closeServer +} = require('./test-utils'); test('basicWithMock', (t, done) => { const mockResolver = createMockDnsResolver({ @@ -853,3 +861,115 @@ test('endToEndOverRealSocket', async () => { await closeServer(server); }); + +test('resolveRecordsDrivesTheWholePipeline', async () => { + // The promise-based resolver has to serve every lookup the delivery makes, not just the + // MX one, and each has to arrive with its record type named + const { resolver, calls } = createMockRecordResolver({ + 'async.example.com:MX': { data: [{ exchange: 'mail.example.com', priority: 10 }] }, + 'mail.example.com:A': { data: ['192.0.2.1'] } + }); + + const connection = await mxConnect({ + target: 'async.example.com', + dnsOptions: { resolveRecords: resolver }, + connectHook: createMockConnectHook() + }); + + assert.strictEqual(connection.host, '192.0.2.1'); + assert.ok(calls.includes('async.example.com:MX'), 'The MX lookup should go through resolveRecords'); + assert.ok(calls.includes('mail.example.com:A'), 'Address lookups should go through it too'); + assert.ok(!calls.some(call => call.endsWith(':undefined')), 'Every lookup should name its record type'); +}); + +test('resolveRecordsCoversTheMtaStsPolicyLookup', async () => { + // The policy host is resolved before any MX record is considered, and it must use the + // caller's resolver like everything else. Falling back to system DNS there would both + // ignore the configuration and reach a different answer than the rest of the delivery. + const { resolver, calls } = createMockRecordResolver({ + '_mta-sts.sts-async.example.com:TXT': { data: [['v=STSv1; id=async1']] }, + 'mail.example.com:A': { data: ['192.0.2.1'] } + }); + + const connection = await mxConnect({ + target: 'sts-async.example.com', + mx: ['mail.example.com'], + mtaSts: { enabled: true }, + dnsOptions: { resolveRecords: resolver }, + connectHook: createMockConnectHook() + }); + + assert.ok(connection.socket); + assert.ok(calls.includes('_mta-sts.sts-async.example.com:TXT'), 'The policy TXT lookup should use resolveRecords'); + assert.ok( + calls.some(call => call.startsWith('mta-sts.sts-async.example.com:')), + 'The policy host lookup should use resolveRecords' + ); +}); + +test('legacyResolveStillHonouredAlongsideTheNewOption', async () => { + // Adding resolveRecords changed how the resolver is selected, so the callback option has + // to keep being picked up when it is the only one set. Its arity contract is pinned by + // mtaStsPolicyResolverUsesTwoArgumentAForm; this only guards the selection. + const { resolver, calls } = createTrackingDnsResolver({ + 'legacy.example.com:MX': { data: [{ exchange: 'mail.example.com', priority: 10 }] }, + 'mail.example.com:A': { data: ['192.0.2.3'] } + }); + + const connection = await mxConnect({ + target: 'legacy.example.com', + dnsOptions: { resolve: resolver }, + connectHook: createMockConnectHook() + }); + + assert.strictEqual(connection.host, '192.0.2.3'); + assert.ok(calls.length > 0, 'The callback resolver must still be used when it is the only one set'); +}); + +test('mtaStsPolicyFilterOnlyTrustsRealArrays', async () => { + // mailauth connects to whatever the resolver returned, so the validation in between has + // to run over an array we recognise. An answer that supplies its own filter method would + // otherwise choose what survives and hand back an address nothing checked. + const https = require('https'); + const originalRequest = https.request; + const requestedHosts = []; + + https.request = options => { + requestedHosts.push(options.host); + throw new Error('no request should be made in this test'); + }; + + try { + const connection = await mxConnect({ + target: 'duck.example.com', + mx: ['mail.example.com'], + mtaSts: { enabled: true }, + dnsOptions: { + blockLocalAddresses: true, + resolveRecords(domain, type) { + if (type === 'TXT') { + return [['v=STSv1; id=duck1']]; + } + if (domain.startsWith('mta-sts.')) { + // Array-like, with a filter that would wave anything through + return { length: 1, 0: '127.0.0.1', filter: () => ['127.0.0.1'] }; + } + if (type === 'A') { + return ['192.0.2.1']; + } + const err = new Error('ENODATA'); + err.code = 'ENODATA'; + throw err; + } + }, + connectHook: createMockConnectHook() + }); + + assert.ok(connection.socket, 'Delivery should still proceed over the legitimate MX'); + assert.strictEqual(connection.host, '192.0.2.1'); + } finally { + https.request = originalRequest; + } + + assert.deepStrictEqual(requestedHosts, [], 'No policy fetch may be made to an address the filter never saw'); +}); diff --git a/test/test-utils.js b/test/test-utils.js index ca8213b..ae298d1 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -94,6 +94,35 @@ function createTrackingDnsResolver(responses) { return { resolver, calls }; } +/** + * Creates a promise-based mock resolver for the dnsOptions.resolveRecords option, using the + * same response map as createMockDnsResolver. + * + * Returns { resolver, calls }, where calls holds `domain:type` strings in order. The + * resolver returns the records rather than taking a callback, and throws for a miss, which + * is how that option reports a lookup failure. + * + * @param {Object} responses - Map of `domain:type` (or bare domain) keys to { data } or { error } + */ +function createMockRecordResolver(responses) { + const calls = []; + + const resolver = (domain, type) => { + calls.push(`${domain}:${type}`); + + const response = responses[`${domain}:${type}`] || responses[domain]; + if (!response) { + throw createDnsError('ENOTFOUND'); + } + if (response.error) { + throw response.error; + } + return response.data; + }; + + return { resolver, calls }; +} + /** * Creates a DNS error with the specified code. */ @@ -178,6 +207,7 @@ module.exports = { getFreePort, createMockDnsResolver, createTrackingDnsResolver, + createMockRecordResolver, createDnsError, createMockSocket, createMockConnectHook, diff --git a/test/tools-test.js b/test/tools-test.js index 982a821..007a719 100644 --- a/test/tools-test.js +++ b/test/tools-test.js @@ -16,7 +16,7 @@ test('getDnsResolverWithCustomResolver', async () => { setImmediate(() => callback(null, ['192.0.2.1'])); }; - const resolver = tools.getDnsResolver(customResolver); + const resolver = tools.getDnsResolver({ resolve: customResolver }); try { // Test with type argument @@ -45,7 +45,7 @@ test('getDnsResolverWithCustomResolverError', async () => { setImmediate(() => callback(err)); }; - const resolver = tools.getDnsResolver(customResolver); + const resolver = tools.getDnsResolver({ resolve: customResolver }); try { await resolver('fail.example.com', 'MX'); @@ -57,7 +57,7 @@ test('getDnsResolverWithCustomResolverError', async () => { test('getDnsResolverWithoutCustomResolver', async () => { // When no custom resolver provided, should use native dns.promises - const resolver = tools.getDnsResolver(null); + const resolver = tools.getDnsResolver(); // Just verify it returns a function assert.strictEqual(typeof resolver, 'function'); @@ -285,7 +285,7 @@ test('getDnsResolverExplicitATypeUsesLegacyForm', async () => { setImmediate(() => callback(null, ['192.0.2.1'])); }; - const resolver = tools.getDnsResolver(customResolver); + const resolver = tools.getDnsResolver({ resolve: customResolver }); try { const aRecords = await resolver('example.com', 'A'); @@ -428,3 +428,114 @@ test('isInvalidNat64PrefixesNotAnArray', async () => { assert.ok(tools.isInvalid(options, '127.0.0.1'), 'ordinary validation must be undisturbed'); } }); + +test('getDnsResolverResolveAsyncReturningPromise', async () => { + // The newer option returns the records rather than taking a callback + const calls = []; + const resolver = tools.getDnsResolver({ + async resolveRecords(domain, type) { + calls.push({ domain, type }); + return ['192.0.2.1']; + } + }); + + assert.deepStrictEqual(await resolver('example.com', 'MX'), ['192.0.2.1']); + assert.deepStrictEqual(calls, [{ domain: 'example.com', type: 'MX' }]); +}); + +test('getDnsResolverResolveAsyncMayBeSynchronous', async () => { + // Returning the records directly is allowed, so a resolver backed by a cache does not + // have to pretend to be asynchronous + const resolver = tools.getDnsResolver({ + resolveRecords: () => ['192.0.2.2'] + }); + + assert.deepStrictEqual(await resolver('example.com', 'A'), ['192.0.2.2']); +}); + +test('getDnsResolverResolveAsyncAlwaysReceivesAType', async () => { + // The callback form omits the type for A lookups, which is the wart this option exists + // to leave behind, so an A lookup arrives here named like every other + const types = []; + const resolver = tools.getDnsResolver({ + resolveRecords: (domain, type) => { + types.push(type); + return []; + } + }); + + await resolver('example.com'); + await resolver('example.com', 'A'); + await resolver('example.com', 'AAAA'); + + assert.deepStrictEqual(types, ['A', 'A', 'AAAA'], 'An omitted type must reach the resolver as A'); +}); + +test('getDnsResolverResolveAsyncErrorsReject', async () => { + // Both a rejected promise and a synchronous throw have to surface as a rejection, or + // the resolution step cannot tell a failed lookup from an empty answer + const rejecting = tools.getDnsResolver({ + async resolveRecords() { + const err = new Error('SERVFAIL'); + err.code = 'SERVFAIL'; + throw err; + } + }); + await assert.rejects(() => rejecting('example.com', 'MX'), { code: 'SERVFAIL' }); + + const throwing = tools.getDnsResolver({ + resolveRecords() { + const err = new Error('SERVFAIL'); + err.code = 'SERVFAIL'; + throw err; + } + }); + await assert.rejects(() => throwing('example.com', 'MX'), { code: 'SERVFAIL' }, 'A synchronous throw must reject too'); +}); + +test('getDnsResolverPrefersResolveAsyncOverResolve', async () => { + // Both may be set while a caller migrates. The newer one wins, and the callback is + // left alone rather than being called as well. + let callbackUsed = false; + const resolver = tools.getDnsResolver({ + resolveRecords: () => ['from-async'], + resolve: (domain, typeOrCallback, maybeCallback) => { + callbackUsed = true; + const callback = typeof typeOrCallback === 'function' ? typeOrCallback : maybeCallback; + return setImmediate(() => callback(null, ['from-callback'])); + } + }); + + assert.deepStrictEqual(await resolver('example.com', 'MX'), ['from-async']); + assert.strictEqual(callbackUsed, false, 'The callback resolver must not be consulted as well'); +}); + +test('getDnsResolverRejectsUncallableResolvers', async () => { + // Quietly falling back to the system resolver because an option was mistyped would send + // mail through a resolver the operator did not choose, losing whatever theirs was there + // for, and nothing would say so. Both options are checked, so the mistake surfaces + // whichever one carries it. + for (const bad of ['nope', 42, {}, [], true]) { + assert.throws( + () => tools.getDnsResolver({ resolveRecords: bad }), + { name: 'TypeError', message: /resolveRecords must be a function/ }, + `resolveRecords of type ${typeof bad} must be refused` + ); + assert.throws( + () => tools.getDnsResolver({ resolve: bad }), + { name: 'TypeError', message: /resolve must be a function/ }, + `resolve of type ${typeof bad} must be refused` + ); + } + + // An option left explicitly empty is not a mistake, it just means "not set" + for (const empty of [undefined, null]) { + const resolver = tools.getDnsResolver({ + resolveRecords: empty, + resolve: (domain, callback) => setImmediate(() => callback(null, ['fell-through'])) + }); + assert.deepStrictEqual(await resolver('example.com'), ['fell-through'], 'An unset resolveRecords must fall through to resolve'); + } + + assert.strictEqual(typeof tools.getDnsResolver({ resolveRecords: null, resolve: null }), 'function', 'Both unset falls through to the native resolver'); +});