From 8d6c22e493820255d1fddb27c9fd169032870f41 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 18:58:04 +0200 Subject: [PATCH 01/11] initial work - normalize if/else style --- lib/dns.js | 58 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 04cc58755142ce..2eb0908533d418 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -176,8 +176,9 @@ exports.lookup = function lookup(hostname, options, callback) { function onlookupservice(err, host, service) { - if (err) + if (err) { return this.callback(errnoException(err, 'getnameinfo', this.host)); + } this.callback(null, host, service); } @@ -185,16 +186,19 @@ function onlookupservice(err, host, service) { // lookupService(address, port, callback) exports.lookupService = function(host, port, callback) { - if (arguments.length !== 3) + if (arguments.length !== 3) { throw new Error('Invalid arguments'); + } - if (cares.isIP(host) === 0) + if (cares.isIP(host) === 0) { throw new TypeError('"host" argument needs to be a valid IP address'); + } - if (port == null || !isLegalPort(port)) + if (port == null || !isLegalPort(port)) { throw new TypeError(`"port" should be >= 0 and < 65536, got "${port}"`); + } - port = +port; + port = Number(port); callback = makeAsync(callback); var req = new GetNameInfoReqWrap(); @@ -204,7 +208,9 @@ exports.lookupService = function(host, port, callback) { req.oncomplete = onlookupservice; var err = cares.getnameinfo(req, host, port); - if (err) throw errnoException(err, 'getnameinfo', host); + if (err) { + throw errnoException(err, 'getnameinfo', host); + } callback.immediately = true; return req; @@ -212,10 +218,11 @@ exports.lookupService = function(host, port, callback) { function onresolve(err, result) { - if (err) + if (err) { this.callback(errnoException(err, this.bindingName, this.hostname)); - else + } else { this.callback(null, result); + } } @@ -236,7 +243,10 @@ function resolver(bindingName) { req.hostname = name; req.oncomplete = onresolve; var err = binding(req, name); - if (err) throw errnoException(err, bindingName); + if (err) { + throw errnoException(err, bindingName); + } + callback.immediately = true; return req; }; @@ -272,7 +282,7 @@ exports.resolve = function(hostname, type_, callback_) { if (typeof resolver === 'function') { return resolver(hostname, callback); } else { - throw new Error('Unknown type "' + type_ + '"'); + throw new Error(`Unknown type ${type_}`); } }; @@ -292,36 +302,40 @@ exports.setServers = function(servers) { servers.forEach(function(serv) { var ver = isIp(serv); - if (ver) - return newSet.push([ver, serv]); - + if (ver) { + newSet.push([ver, serv]); + return; + } var match = serv.match(/\[(.*)\](:\d+)?/); // we have an IPv6 in brackets if (match) { ver = isIp(match[1]); - if (ver) - return newSet.push([ver, match[1]]); + if (ver) { + newSet.push([ver, match[1]]); + return; + } } var s = serv.split(/:\d+$/)[0]; ver = isIp(s); - if (ver) - return newSet.push([ver, s]); + if (ver) { + newSet.push([ver, s]); + return; + } - throw new Error('IP address is not properly formatted: ' + serv); + throw new Error(`IP address is not properly formatted: ${serv}`); }); - var r = cares.setServers(newSet); + var errorNumber = cares.setServers(newSet); - if (r) { + if (errorNumber !== 0) { // reset the servers to the old servers, because ares probably unset them cares.setServers(orig.join(',')); var err = cares.strerror(r); - throw new Error('c-ares failed to set servers: "' + err + - '" [' + servers + ']'); + throw new Error(`c-ares failed to set servers: ${err} [${servers}]`); } }; From f5c4f55fb96ca56e650f2404580009946b13709f Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 18:59:06 +0200 Subject: [PATCH 02/11] more template strings and less abused returns --- lib/dns.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 2eb0908533d418..e7b68a15bd8cf7 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -25,7 +25,7 @@ function errnoException(err, syscall, hostname) { } var ex = null; if (typeof err === 'string') { // c-ares error code. - ex = new Error(syscall + ' ' + err + (hostname ? ' ' + hostname : '')); + ex = new Error(`${syscall} ${err} ${(hostname ? ' ' + hostname : '')}`); ex.code = err; ex.errno = err; ex.syscall = syscall; @@ -76,8 +76,10 @@ function makeAsync(callback) { function onlookup(err, addresses) { if (err) { - return this.callback(errnoException(err, 'getaddrinfo', this.hostname)); + this.callback(errnoException(err, 'getaddrinfo', this.hostname)); + return; } + if (this.family) { this.callback(null, addresses[0], this.family); } else { @@ -89,7 +91,8 @@ function onlookup(err, addresses) { function onlookupall(err, addresses) { var results = []; if (err) { - return this.callback(errnoException(err, 'getaddrinfo', this.hostname)); + this.callback(errnoException(err, 'getaddrinfo', this.hostname)); + return; } for (var i = 0; i < addresses.length; i++) { From 2784847fc6b2ca60e89fd4c941b6f63fc7fb4b67 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 19:03:32 +0200 Subject: [PATCH 03/11] block over for loop, spread arguments from apply --- lib/dns.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index e7b68a15bd8cf7..ea2b74c51c29fa 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -62,12 +62,13 @@ function makeAsync(callback) { return function asyncCallback() { if (asyncCallback.immediately) { // The API already returned, we can invoke the callback immediately. - callback.apply(null, arguments); + callback(...arguments); } else { var args = new Array(arguments.length + 1); args[0] = callback; - for (var i = 1, a = 0; a < arguments.length; ++i, ++a) + for (var i = 1, a = 0; a < arguments.length; ++i, ++a) { args[i] = arguments[a]; + } process.nextTick.apply(null, args); } }; @@ -89,12 +90,13 @@ function onlookup(err, addresses) { function onlookupall(err, addresses) { - var results = []; if (err) { this.callback(errnoException(err, 'getaddrinfo', this.hostname)); return; } + const results = Array(addresses.length); + for (var i = 0; i < addresses.length; i++) { results.push({ address: addresses[i], From 48fde13751ad777302751495d89791bbbfb2380f Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 20:25:02 +0200 Subject: [PATCH 04/11] address comments --- lib/dns.js | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index ea2b74c51c29fa..c319d38386d859 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -62,7 +62,7 @@ function makeAsync(callback) { return function asyncCallback() { if (asyncCallback.immediately) { // The API already returned, we can invoke the callback immediately. - callback(...arguments); + callback.apply(null, arguments); } else { var args = new Array(arguments.length + 1); args[0] = callback; @@ -96,12 +96,12 @@ function onlookupall(err, addresses) { } const results = Array(addresses.length); - + for (var i = 0; i < addresses.length; i++) { - results.push({ + results[i] = { address: addresses[i], family: this.family || (addresses[i].indexOf(':') >= 0 ? 6 : 4) - }); + }; } this.callback(null, results); @@ -182,7 +182,8 @@ exports.lookup = function lookup(hostname, options, callback) { function onlookupservice(err, host, service) { if (err) { - return this.callback(errnoException(err, 'getnameinfo', this.host)); + this.callback(errnoException(err, 'getnameinfo', this.host)); + return; } this.callback(null, host, service); @@ -192,7 +193,7 @@ function onlookupservice(err, host, service) { // lookupService(address, port, callback) exports.lookupService = function(host, port, callback) { if (arguments.length !== 3) { - throw new Error('Invalid arguments'); + throw new TypeError(`Expected 3 arguments, got ${arguments.length}`); } if (cares.isIP(host) === 0) { @@ -236,9 +237,10 @@ function resolver(bindingName) { return function query(name, callback) { if (typeof name !== 'string') { - throw new Error('"name" argument must be a string'); + const msg = `"name" argument must be a string, got ${typeof name}`; + throw new TypeError(msg); } else if (typeof callback !== 'function') { - throw new Error('"callback" argument must be a function'); + throw new TypeError('"callback" argument must be a function'); } callback = makeAsync(callback); @@ -272,7 +274,7 @@ exports.resolveSoa = resolveMap.SOA = resolver('querySoa'); exports.reverse = resolver('getHostByAddr'); -exports.resolve = function(hostname, type_, callback_) { +exports.resolve = (hostname, type_, callback_) => { var resolver, callback; if (typeof type_ === 'string') { resolver = resolveMap[type_]; @@ -292,7 +294,7 @@ exports.resolve = function(hostname, type_, callback_) { }; -exports.getServers = function() { +exports.getServers = () => { return cares.getServers(); }; From bfe0f40fe261747cc2da0e53040e72919f9138a0 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 20:27:36 +0200 Subject: [PATCH 05/11] address comments from mscdex --- lib/dns.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns.js b/lib/dns.js index c319d38386d859..3abd5292a4879e 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -95,7 +95,7 @@ function onlookupall(err, addresses) { return; } - const results = Array(addresses.length); + const results = new Array(addresses.length); for (var i = 0; i < addresses.length; i++) { results[i] = { From 2d54ae9ebe9241dbf1dbc649dc0de162784d9344 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 20:49:31 +0200 Subject: [PATCH 06/11] refactor forEach to map --- lib/dns.js | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 3abd5292a4879e..8a6f1636d08e84 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -1,6 +1,5 @@ 'use strict'; -const net = require('net'); const util = require('util'); const cares = process.binding('cares_wrap'); @@ -11,7 +10,7 @@ const GetAddrInfoReqWrap = cares.GetAddrInfoReqWrap; const GetNameInfoReqWrap = cares.GetNameInfoReqWrap; const QueryReqWrap = cares.QueryReqWrap; -const isIp = net.isIP; +const isIp = cares.isIP; const isLegalPort = internalNet.isLegalPort; @@ -38,6 +37,9 @@ function errnoException(err, syscall, hostname) { return ex; } +function validIpVersion(isIpResult) { + return isIpResult !== 0; +} // c-ares invokes a callback either synchronously or asynchronously, // but the dns API should always invoke a callback asynchronously. @@ -196,7 +198,7 @@ exports.lookupService = function(host, port, callback) { throw new TypeError(`Expected 3 arguments, got ${arguments.length}`); } - if (cares.isIP(host) === 0) { + if (isIP(host) === 0) { throw new TypeError('"host" argument needs to be a valid IP address'); } @@ -304,32 +306,27 @@ exports.setServers = function(servers) { // servers cares won't have any servers available for resolution var orig = cares.getServers(); - var newSet = []; - - servers.forEach(function(serv) { - var ver = isIp(serv); + var newSet = servers.map((serv) => { + var ipVersion = isIp(serv); - if (ver) { - newSet.push([ver, serv]); - return; + if (validIpVersion(ipVersion)) { + return [ver, serv]; } var match = serv.match(/\[(.*)\](:\d+)?/); // we have an IPv6 in brackets if (match) { - ver = isIp(match[1]); - if (ver) { - newSet.push([ver, match[1]]); - return; + ipVersion = isIp(match[1]); + if (validIpVersion(ipVersion)) { + return [ver, match[1]]; } } var s = serv.split(/:\d+$/)[0]; - ver = isIp(s); + ipVersion = isIp(s); - if (ver) { - newSet.push([ver, s]); - return; + if (validIpVersion(ipVersion)) { + return [ver, s]; } throw new Error(`IP address is not properly formatted: ${serv}`); From 85f9b8718af7ccd5eeee4dff7cc7022ee885eb65 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 20:50:33 +0200 Subject: [PATCH 07/11] var to const --- lib/dns.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 8a6f1636d08e84..05c42f40c99801 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -304,15 +304,15 @@ exports.getServers = () => { exports.setServers = function(servers) { // cache the original servers because in the event of an error setting the // servers cares won't have any servers available for resolution - var orig = cares.getServers(); + var originalServers = cares.getServers(); - var newSet = servers.map((serv) => { + const newSet = servers.map((serv) => { var ipVersion = isIp(serv); if (validIpVersion(ipVersion)) { return [ver, serv]; } - var match = serv.match(/\[(.*)\](:\d+)?/); + const match = serv.match(/\[(.*)\](:\d+)?/); // we have an IPv6 in brackets if (match) { @@ -336,7 +336,7 @@ exports.setServers = function(servers) { if (errorNumber !== 0) { // reset the servers to the old servers, because ares probably unset them - cares.setServers(orig.join(',')); + cares.setServers(originalServers.join(',')); var err = cares.strerror(r); throw new Error(`c-ares failed to set servers: ${err} [${servers}]`); From 457de39ca7279a29e725abdf33c7ebb2c9e26d05 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 20:54:24 +0200 Subject: [PATCH 08/11] removed references to net module --- lib/dns.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 05c42f40c99801..9fba2c2777d7e1 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -141,8 +141,10 @@ exports.lookup = function lookup(hostname, options, callback) { family = options >>> 0; } - if (family !== 0 && family !== 4 && family !== 6) - throw new TypeError('Invalid argument: family must be 4 or 6'); + if (family !== 0 && family !== 4 && family !== 6) { + const msg = `Invalid argument: family must be 4 or 6, got {family}`; + throw new TypeError(msg); + } callback = makeAsync(callback); @@ -155,7 +157,7 @@ exports.lookup = function lookup(hostname, options, callback) { return {}; } - var matchedFamily = net.isIP(hostname); + var matchedFamily = isIP(hostname); if (matchedFamily) { if (all) { callback(null, [{address: hostname, family: matchedFamily}]); From 4f124f71328457a00b3e7c64f6818f3c100772fb Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 20:56:58 +0200 Subject: [PATCH 09/11] changed {} to Object.create(null) to prevent __proto__ meddling --- lib/dns.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 9fba2c2777d7e1..e0c631fc613689 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -264,7 +264,7 @@ function resolver(bindingName) { } -var resolveMap = {}; +var resolveMap = Object.create(null); exports.resolve4 = resolveMap.A = resolver('queryA'); exports.resolve6 = resolveMap.AAAA = resolver('queryAaaa'); exports.resolveCname = resolveMap.CNAME = resolver('queryCname'); @@ -334,7 +334,7 @@ exports.setServers = function(servers) { throw new Error(`IP address is not properly formatted: ${serv}`); }); - var errorNumber = cares.setServers(newSet); + const errorNumber = cares.setServers(newSet); if (errorNumber !== 0) { // reset the servers to the old servers, because ares probably unset them From 6fe93ed8063458928d8851e60eaca9190eb35571 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 21:23:25 +0200 Subject: [PATCH 10/11] remove isValidIp and check for 0 instead --- lib/dns.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index e0c631fc613689..94aca9db940cc5 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -37,10 +37,6 @@ function errnoException(err, syscall, hostname) { return ex; } -function validIpVersion(isIpResult) { - return isIpResult !== 0; -} - // c-ares invokes a callback either synchronously or asynchronously, // but the dns API should always invoke a callback asynchronously. // @@ -311,7 +307,7 @@ exports.setServers = function(servers) { const newSet = servers.map((serv) => { var ipVersion = isIp(serv); - if (validIpVersion(ipVersion)) { + if (ipVersion !== 0) { return [ver, serv]; } const match = serv.match(/\[(.*)\](:\d+)?/); @@ -319,7 +315,7 @@ exports.setServers = function(servers) { // we have an IPv6 in brackets if (match) { ipVersion = isIp(match[1]); - if (validIpVersion(ipVersion)) { + if (ipVersion !== 0) { return [ver, match[1]]; } } @@ -327,7 +323,7 @@ exports.setServers = function(servers) { var s = serv.split(/:\d+$/)[0]; ipVersion = isIp(s); - if (validIpVersion(ipVersion)) { + if (ipVersion !== 0) { return [ver, s]; } From b3ced41b324f6baba0f4e32e7f62c6424bc8cd6c Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 17 Mar 2016 21:31:58 +0200 Subject: [PATCH 11/11] silly mistake missing $ --- lib/dns.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns.js b/lib/dns.js index 94aca9db940cc5..0796c880b00b65 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -138,7 +138,7 @@ exports.lookup = function lookup(hostname, options, callback) { } if (family !== 0 && family !== 4 && family !== 6) { - const msg = `Invalid argument: family must be 4 or 6, got {family}`; + const msg = `Invalid argument: family must be 4 or 6, got ${family}`; throw new TypeError(msg); }