From a0408fcf70615abd3e60370f7970232537881142 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Wed, 11 Nov 2020 22:22:41 +0100 Subject: [PATCH 1/3] dns: add a cancel() method to the promise Resolver --- doc/api/dns.md | 8 +++ lib/internal/dns/promises.js | 1 + .../test-dns-channel-cancel-promise.js | 71 +++++++++++++++++++ test/parallel/test-dns-channel-cancel.js | 46 +++++++++--- 4 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-dns-channel-cancel-promise.js diff --git a/doc/api/dns.md b/doc/api/dns.md index a284c9d0c57bd5..90774abc0919ef 100644 --- a/doc/api/dns.md +++ b/doc/api/dns.md @@ -730,6 +730,14 @@ The following methods from the `dnsPromises` API are available: * [`resolver.reverse()`][`dnsPromises.reverse()`] * [`resolver.setServers()`][`dnsPromises.setServers()`] +### `resolver.cancel()` +<!-- YAML +added: REPLACEME +--> + +Cancel all outstanding DNS queries made by this resolver. The corresponding +promises will be rejected with an error with code `ECANCELLED`. + ### `dnsPromises.getServers()` <!-- YAML added: v10.6.0 diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js index e8505a95d12fed..4240f9397487d8 100644 --- a/lib/internal/dns/promises.js +++ b/lib/internal/dns/promises.js @@ -217,6 +217,7 @@ class Resolver { 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'); diff --git a/test/parallel/test-dns-channel-cancel-promise.js b/test/parallel/test-dns-channel-cancel-promise.js new file mode 100644 index 00000000000000..974a11ca4f15be --- /dev/null +++ b/test/parallel/test-dns-channel-cancel-promise.js @@ -0,0 +1,71 @@ +'use strict'; +const common = require('../common'); +const dnstools = require('../common/dns'); +const { promises: dnsPromises } = require('dns'); +const assert = require('assert'); +const dgram = require('dgram'); + +const server = dgram.createSocket('udp4'); +const resolver = new dnsPromises.Resolver(); + +const receivedDomains = []; +const expectedDomains = []; + +server.bind(0, common.mustCall(async () => { + resolver.setServers([`127.0.0.1:${server.address().port}`]); + + // Single promise + { + const hostname = 'example0.org'; + expectedDomains.push(hostname); + + await assert.rejects( + resolver.resolve4(hostname), + { + code: 'ECANCELLED', + syscall: 'queryA', + hostname: 'example0.org' + } + ); + } + + // Multiple promises + { + const assertions = []; + const assertionCount = 10; + + for (let i = 1; i <= assertionCount; i++) { + const hostname = `example${i}.org`; + + expectedDomains.push(hostname); + + assertions.push( + assert.rejects( + resolver.resolve4(hostname), + { + code: 'ECANCELLED', + syscall: 'queryA', + hostname: hostname + } + ) + ); + } + + await Promise.all(assertions); + } + + assert.deepStrictEqual(expectedDomains.sort(), receivedDomains.sort()); + + server.close(); +})); + +server.on('message', (msg, { address, port }) => { + const parsed = dnstools.parseDNSPacket(msg); + + for (const question of parsed.questions) { + receivedDomains.push(question.domain); + } + + // Do not send a reply. + resolver.cancel(); +}); diff --git a/test/parallel/test-dns-channel-cancel.js b/test/parallel/test-dns-channel-cancel.js index 0c59ee53161a5e..801fd8d5ceaa26 100644 --- a/test/parallel/test-dns-channel-cancel.js +++ b/test/parallel/test-dns-channel-cancel.js @@ -8,21 +8,49 @@ const dgram = require('dgram'); const server = dgram.createSocket('udp4'); const resolver = new Resolver(); -server.bind(0, common.mustCall(() => { +const expectedDomains = []; +const receivedDomains = []; +const desiredQueries = 11; +let finishedQueries = 0; + +server.bind(0, common.mustCall(async () => { resolver.setServers([`127.0.0.1:${server.address().port}`]); - resolver.resolve4('example.org', common.mustCall((err, res) => { + + const callback = common.mustCall((err, res) => { assert.strictEqual(err.code, 'ECANCELLED'); assert.strictEqual(err.syscall, 'queryA'); - assert.strictEqual(err.hostname, 'example.org'); - server.close(); - })); + assert.strictEqual(err.hostname, `example${finishedQueries}.org`); + + finishedQueries++; + if (finishedQueries === desiredQueries) { + assert.deepStrictEqual(expectedDomains.sort(), receivedDomains.sort()); + server.close(); + } + }, desiredQueries); + + const next = (...args) => { + callback(...args); + + // Multiple queries + for (let i = 1; i < desiredQueries; i++) { + const domain = `example${i}.org`; + expectedDomains.push(domain); + resolver.resolve4(domain, callback); + } + }; + + // Single query + expectedDomains.push('example0.org'); + resolver.resolve4('example0.org', next); })); -server.on('message', common.mustCall((msg, { address, port }) => { +server.on('message', (msg, { address, port }) => { const parsed = dnstools.parseDNSPacket(msg); - const domain = parsed.questions[0].domain; - assert.strictEqual(domain, 'example.org'); + + for (const question of parsed.questions) { + receivedDomains.push(question.domain); + } // Do not send a reply. resolver.cancel(); -})); +}); From 39ee90e7424a5b8acf5184671a2cd66022c189c1 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 13 Nov 2020 15:55:58 +0100 Subject: [PATCH 2/3] fixup! do not expect new messages - callback --- test/parallel/test-dns-channel-cancel.js | 32 +++++++++++------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/test/parallel/test-dns-channel-cancel.js b/test/parallel/test-dns-channel-cancel.js index 801fd8d5ceaa26..919fb44a6a3f1d 100644 --- a/test/parallel/test-dns-channel-cancel.js +++ b/test/parallel/test-dns-channel-cancel.js @@ -8,11 +8,19 @@ const dgram = require('dgram'); const server = dgram.createSocket('udp4'); const resolver = new Resolver(); -const expectedDomains = []; -const receivedDomains = []; const desiredQueries = 11; let finishedQueries = 0; +const addMessageListener = () => { + server.removeAllListeners('message'); + + server.once('message', () => { + server.once('message', common.mustNotCall); + + resolver.cancel(); + }); +}; + server.bind(0, common.mustCall(async () => { resolver.setServers([`127.0.0.1:${server.address().port}`]); @@ -23,7 +31,6 @@ server.bind(0, common.mustCall(async () => { finishedQueries++; if (finishedQueries === desiredQueries) { - assert.deepStrictEqual(expectedDomains.sort(), receivedDomains.sort()); server.close(); } }, desiredQueries); @@ -31,26 +38,15 @@ server.bind(0, common.mustCall(async () => { const next = (...args) => { callback(...args); + addMessageListener(); + // Multiple queries for (let i = 1; i < desiredQueries; i++) { - const domain = `example${i}.org`; - expectedDomains.push(domain); - resolver.resolve4(domain, callback); + resolver.resolve4(`example${i}.org`, callback); } }; // Single query - expectedDomains.push('example0.org'); + addMessageListener(); resolver.resolve4('example0.org', next); })); - -server.on('message', (msg, { address, port }) => { - const parsed = dnstools.parseDNSPacket(msg); - - for (const question of parsed.questions) { - receivedDomains.push(question.domain); - } - - // Do not send a reply. - resolver.cancel(); -}); From 4df27f426b1f8aa457e49a68fdd32b30e5e1cd6b Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 13 Nov 2020 15:59:48 +0100 Subject: [PATCH 3/3] fixup! do not expect new messages - promise --- .../test-dns-channel-cancel-promise.js | 34 ++++++++----------- test/parallel/test-dns-channel-cancel.js | 1 - 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/test/parallel/test-dns-channel-cancel-promise.js b/test/parallel/test-dns-channel-cancel-promise.js index 974a11ca4f15be..382ac3dd508d68 100644 --- a/test/parallel/test-dns-channel-cancel-promise.js +++ b/test/parallel/test-dns-channel-cancel-promise.js @@ -1,6 +1,5 @@ 'use strict'; const common = require('../common'); -const dnstools = require('../common/dns'); const { promises: dnsPromises } = require('dns'); const assert = require('assert'); const dgram = require('dgram'); @@ -8,27 +7,37 @@ const dgram = require('dgram'); const server = dgram.createSocket('udp4'); const resolver = new dnsPromises.Resolver(); -const receivedDomains = []; -const expectedDomains = []; +const addMessageListener = () => { + server.removeAllListeners('message'); + + server.once('message', () => { + server.once('message', common.mustNotCall); + + resolver.cancel(); + }); +}; server.bind(0, common.mustCall(async () => { resolver.setServers([`127.0.0.1:${server.address().port}`]); + addMessageListener(); + // Single promise { const hostname = 'example0.org'; - expectedDomains.push(hostname); await assert.rejects( resolver.resolve4(hostname), { code: 'ECANCELLED', syscall: 'queryA', - hostname: 'example0.org' + hostname } ); } + addMessageListener(); + // Multiple promises { const assertions = []; @@ -37,8 +46,6 @@ server.bind(0, common.mustCall(async () => { for (let i = 1; i <= assertionCount; i++) { const hostname = `example${i}.org`; - expectedDomains.push(hostname); - assertions.push( assert.rejects( resolver.resolve4(hostname), @@ -54,18 +61,5 @@ server.bind(0, common.mustCall(async () => { await Promise.all(assertions); } - assert.deepStrictEqual(expectedDomains.sort(), receivedDomains.sort()); - server.close(); })); - -server.on('message', (msg, { address, port }) => { - const parsed = dnstools.parseDNSPacket(msg); - - for (const question of parsed.questions) { - receivedDomains.push(question.domain); - } - - // Do not send a reply. - resolver.cancel(); -}); diff --git a/test/parallel/test-dns-channel-cancel.js b/test/parallel/test-dns-channel-cancel.js index 919fb44a6a3f1d..f92fb2e30a4d12 100644 --- a/test/parallel/test-dns-channel-cancel.js +++ b/test/parallel/test-dns-channel-cancel.js @@ -1,6 +1,5 @@ 'use strict'; const common = require('../common'); -const dnstools = require('../common/dns'); const { Resolver } = require('dns'); const assert = require('assert'); const dgram = require('dgram');