Skip to content

Commit

Permalink
Node v20 test fix (#3184)
Browse files Browse the repository at this point in the history
* chore: handle dns.reverse invalid throws
* chore: update a few deps in package.json
* dns_list_base: avoid test failure when public DNS used
  • Loading branch information
msimerson authored May 3, 2023
1 parent 4ecb82c commit 7569902
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 43 deletions.
9 changes: 8 additions & 1 deletion Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@

#### Fixed

- fix: rename redis command setex to setEx #3174
- rename redis command setex to setEx #3174

#### Changed

- connection: handle dns.reverse invalid throws on node v20
- chore: bump a few dependency versions
- dns_list_base: avoid test failure when public DNS used


### [3.0.1] - 2023-01-19

Expand Down
13 changes: 10 additions & 3 deletions connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -734,9 +734,16 @@ class Connection {
});
break;
default:
dns.reverse(this.remote.ip, (err, domains) => {
this.rdns_response(err, domains);
});
// BUG: dns.reverse throws on invalid input (and sometimes valid
// input nodejs/node#47847). Also throws when empty results
try {
dns.reverse(this.remote.ip, (err, domains) => {
this.rdns_response(err, domains);
})
}
catch (err) {
this.rdns_response(err, []);
}
}
}
rdns_response (err, domains) {
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
"daemon": "~1.1.0",
"ipaddr.js": "~2.0.1",
"node-gyp": "^9.3.1",
"nopt": "~7.0.0",
"nopt": "~7.1.0",
"npid": "~0.4.0",
"semver": "~7.3.8",
"semver": "~7.5.0",
"sprintf-js": "~1.1.2",
"haraka-config": "^1.1.0",
"haraka-constants": "^1.0.6",
Expand Down Expand Up @@ -73,12 +73,12 @@
"tmp": "~0.2.1"
},
"devDependencies": {
"nodeunit-x": "^0.15.0",
"nodeunit-x": "^0.16.0",
"haraka-test-fixtures": "^1.2.1",
"mock-require": "^3.0.3",
"eslint": "^8.27.0",
"eslint": "^8.39.0",
"eslint-plugin-haraka": "^1.0.15",
"nodemailer": "^6.7.7"
"nodemailer": "^6.9.1"
},
"bugs": {
"mail": "[email protected]",
Expand Down
6 changes: 3 additions & 3 deletions plugins/dns_list_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ exports.multi = function (lookup, zones, cb) {
}
cb(err, zone, a, true);
done();
});
})
}
function zonesDone (err) {
cb(err, null, null, false);
Expand All @@ -148,8 +148,8 @@ exports.first = function (lookup, zones, cb, cb_each) {

// has pending queries OR this one is a positive result
ran_cb = true;
return cb(err, zone, a);
});
cb(err, zone, a);
})
}

exports.check_zones = function (interval) {
Expand Down
72 changes: 41 additions & 31 deletions tests/plugins/dns_list_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ exports.multi = {
setUp : _set_up,
'Spamcop' (test) {
test.expect(4);
function cb (err, zone, a, pending) {
this.plugin.multi('127.0.0.2', 'bl.spamcop.net', (err, zone, a, pending) => {
test.equal(null, err);
if (pending) {
test.ok((Array.isArray(a) && a.length > 0));
Expand All @@ -78,12 +78,11 @@ exports.multi = {
else {
test.done();
}
}
this.plugin.multi('127.0.0.2', 'bl.spamcop.net', cb);
})
},
'CBL' (test) {
test.expect(4);
function cb (err, zone, a, pending) {
this.plugin.multi('127.0.0.2', 'xbl.spamhaus.org', (err, zone, a, pending) => {
test.equal(null, err);
if (pending) {
test.ok((Array.isArray(a) && a.length > 0));
Expand All @@ -92,12 +91,12 @@ exports.multi = {
else {
test.done();
}
}
this.plugin.multi('127.0.0.2', 'xbl.spamhaus.org', cb);
})
},
'Spamcop + CBL' (test) {
test.expect(12);
function cb (err, zone, a, pending) {
const dnsbls = ['bl.spamcop.net','xbl.spamhaus.org'];
this.plugin.multi('127.0.0.2', dnsbls, (err, zone, a, pending) => {
test.equal(null, err);
if (pending) {
test.ok(zone);
Expand All @@ -110,15 +109,20 @@ exports.multi = {
test.equal(false, pending);
test.done();
}
}
const dnsbls = ['bl.spamcop.net','xbl.spamhaus.org'];
this.plugin.multi('127.0.0.2', dnsbls, cb);
})
},
'Spamcop + CBL + negative result' (test) {
test.expect(12);
function cb (err, zone, a, pending) {
const dnsbls = [ 'bl.spamcop.net','xbl.spamhaus.org' ];
this.plugin.multi('127.0.0.1', dnsbls, (err, zone, a, pending) => {
test.equal(null, err);
test.equal(null, a);
if (a && a[0] && a[0] === '127.255.255.254') {
test.deepEqual(['127.255.255.254'], a)
console.warn(`ERROR: DNSBLs don't work with PUBLIC DNS!`)
}
else {
test.equal(null, a)
}
if (pending) {
test.equal(true, pending);
test.ok(zone);
Expand All @@ -128,14 +132,19 @@ exports.multi = {
test.equal(null, zone);
test.done();
}
}
const dnsbls = ['bl.spamcop.net','xbl.spamhaus.org'];
this.plugin.multi('127.0.0.1', dnsbls, cb);
})
},
'IPv6 addresses supported' (test) {
test.expect(12);
function cb (err, zone, a, pending) {
test.equal(null, a);
const dnsbls = ['bl.spamcop.net','xbl.spamhaus.org'];
this.plugin.multi('::1', dnsbls, (err, zone, a, pending) => {
if (a && a[0] && a[0] === '127.255.255.254') {
test.deepEqual(['127.255.255.254'], a)
console.warn(`ERROR: DNSBLs don't work with PUBLIC DNS!`)
}
else {
test.equal(null, a);
}
if (pending) {
test.deepEqual(null, err);
test.equal(true, pending);
Expand All @@ -147,35 +156,36 @@ exports.multi = {
test.equal(null, zone);
test.done();
}
}
const dnsbls = ['bl.spamcop.net','xbl.spamhaus.org'];
this.plugin.multi('::1', dnsbls, cb);
})
}
}

exports.first = {
setUp : _set_up,
'positive result' (test) {
test.expect(3);
function cb (err, zone, a) {
const dnsbls = [ 'xbl.spamhaus.org', 'bl.spamcop.net' ];
this.plugin.first('127.0.0.2', dnsbls, (err, zone, a) => {
test.equal(null, err);
test.ok(zone);
test.ok((Array.isArray(a) && a.length > 0));
test.done();
}
const dnsbls = [ 'xbl.spamhaus.org', 'bl.spamcop.net' ];
this.plugin.first('127.0.0.2', dnsbls , cb);
})
},
'negative result' (test) {
test.expect(3);
function cb (err, zone, a) {
test.expect(2);
const dnsbls = [ 'xbl.spamhaus.org', 'bl.spamcop.net' ];
this.plugin.first('127.0.0.1', dnsbls, (err, zone, a) => {
test.equal(null, err);
test.equal(null, zone);
test.equal(null, a);
if (a && a[0] && a[0] === '127.255.255.254') {
test.deepEqual(['127.255.255.254'], a)
console.warn(`ERROR: DNSBLs don't work with PUBLIC DNS!`)
}
else {
test.equal(null, a);
}
test.done();
}
const dnsbls = [ 'xbl.spamhaus.org', 'bl.spamcop.net' ];
this.plugin.first('127.0.0.1', dnsbls, cb);
})
},
'each_cb' (test) {
test.expect(7);
Expand Down

0 comments on commit 7569902

Please sign in to comment.