From 0c9161057ca9a0fbcd2a8ab967bf15cce9bbedd4 Mon Sep 17 00:00:00 2001 From: Benjamin Ki Date: Sat, 4 May 2019 13:55:31 -0400 Subject: [PATCH 1/2] test: move dgram invalid host test to internet tests This moves a dgram test from `parallel` to `internet` because it relies on a DNS request. In certain cases, ISPs hijack invalid IETF-reserved invalid names which causes a false negative failure. Fixes: https://github.com/nodejs/node/issues/27341 --- test/internet/test-dgram-connect.js | 21 +++++++++++++++++++++ test/parallel/test-dgram-connect.js | 14 ++------------ 2 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 test/internet/test-dgram-connect.js diff --git a/test/internet/test-dgram-connect.js b/test/internet/test-dgram-connect.js new file mode 100644 index 00000000000000..785181cee6bed3 --- /dev/null +++ b/test/internet/test-dgram-connect.js @@ -0,0 +1,21 @@ +'use strict'; + +const common = require('../common'); +const { addresses } = require('../common/internet'); +const assert = require('assert'); +const dgram = require('dgram'); + +const PORT = 12345; + +const client = dgram.createSocket('udp4'); +client.connect(PORT, addresses.INVALID_HOST, common.mustCall((err) => { + assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); + + client.once('error', common.mustCall((err) => { + assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); + client.once('connect', common.mustCall(() => client.close())); + client.connect(PORT); + })); + + client.connect(PORT, addresses.INVALID_HOST); +})); diff --git a/test/parallel/test-dgram-connect.js b/test/parallel/test-dgram-connect.js index 8fba27bcd2ac83..a9815f439a731a 100644 --- a/test/parallel/test-dgram-connect.js +++ b/test/parallel/test-dgram-connect.js @@ -1,7 +1,6 @@ 'use strict'; const common = require('../common'); -const { addresses } = require('../common/internet'); const assert = require('assert'); const dgram = require('dgram'); @@ -36,17 +35,8 @@ client.connect(PORT, common.mustCall(() => { code: 'ERR_SOCKET_DGRAM_NOT_CONNECTED' }); - client.connect(PORT, addresses.INVALID_HOST, common.mustCall((err) => { - assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); - - client.once('error', common.mustCall((err) => { - assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); - client.once('connect', common.mustCall(() => client.close())); - client.connect(PORT); - })); - - client.connect(PORT, addresses.INVALID_HOST); - })); + client.once('connect', common.mustCall(() => client.close())); + client.connect(PORT); })); assert.throws(() => { From 5369cd0fad3bc0d0ab4f392193600ef821c8486d Mon Sep 17 00:00:00 2001 From: Benjamin Ki Date: Sat, 4 May 2019 21:28:32 -0400 Subject: [PATCH 2/2] test: use common.PORT instead of an extraneous variable This test is not parallelized and so we can use the test commons PORT variable. Refs: https://github.com/nodejs/node/pull/27565#discussion_r281000162 --- test/internet/test-dgram-connect.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/internet/test-dgram-connect.js b/test/internet/test-dgram-connect.js index 785181cee6bed3..47a12c7890924c 100644 --- a/test/internet/test-dgram-connect.js +++ b/test/internet/test-dgram-connect.js @@ -5,17 +5,15 @@ const { addresses } = require('../common/internet'); const assert = require('assert'); const dgram = require('dgram'); -const PORT = 12345; - const client = dgram.createSocket('udp4'); -client.connect(PORT, addresses.INVALID_HOST, common.mustCall((err) => { +client.connect(common.PORT, addresses.INVALID_HOST, common.mustCall((err) => { assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); client.once('error', common.mustCall((err) => { assert.ok(err.code === 'ENOTFOUND' || err.code === 'EAI_AGAIN'); client.once('connect', common.mustCall(() => client.close())); - client.connect(PORT); + client.connect(common.PORT); })); - client.connect(PORT, addresses.INVALID_HOST); + client.connect(common.PORT, addresses.INVALID_HOST); }));