Skip to content

Commit

Permalink
dns: coerce port to number in lookupService
Browse files Browse the repository at this point in the history
Previously, port could be any number in dns.lookupService. This change
throws a TypeError if port is outside the range of 0-65535. It also
coerces the port to a number.

PR-URL: #4883
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
evanlucas authored and jasnell committed Feb 1, 2016
1 parent 089d84f commit f3be421
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
4 changes: 4 additions & 0 deletions doc/api/dns.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ on some operating systems (e.g FreeBSD 10.1).
Resolves the given `address` and `port` into a hostname and service using
the operating system's underlying `getnameinfo` implementation.

If `address` is not a valid IP address, a `TypeError` will be thrown.
The `port` will be coerced to a number. If it is not a legal port, a `TypeError`
will be thrown.

The callback has arguments `(err, hostname, service)`. The `hostname` and
`service` arguments are strings (e.g. `'localhost'` and `'http'` respectively).

Expand Down
7 changes: 5 additions & 2 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ const util = require('util');

const cares = process.binding('cares_wrap');
const uv = process.binding('uv');
const internalNet = require('internal/net');

const GetAddrInfoReqWrap = cares.GetAddrInfoReqWrap;
const GetNameInfoReqWrap = cares.GetNameInfoReqWrap;
const QueryReqWrap = cares.QueryReqWrap;

const isIp = net.isIP;
const isLegalPort = internalNet.isLegalPort;


function errnoException(err, syscall, hostname) {
Expand Down Expand Up @@ -189,9 +191,10 @@ exports.lookupService = function(host, port, callback) {
if (cares.isIP(host) === 0)
throw new TypeError('"host" argument needs to be a valid IP address');

if (typeof port !== 'number')
throw new TypeError(`"port" argument must be a number, got "${port}"`);
if (port == null || !isLegalPort(port))
throw new TypeError(`"port" should be >= 0 and < 65536, got "${port}"`);

port = +port;
callback = makeAsync(callback);

var req = new GetNameInfoReqWrap();
Expand Down
20 changes: 18 additions & 2 deletions test/parallel/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,26 @@ assert.throws(function() {
dns.lookupService('fasdfdsaf', 0, noop);
}, /"host" argument needs to be a valid IP address/);

assert.throws(function() {
assert.doesNotThrow(function() {
dns.lookupService('0.0.0.0', '0', noop);
}, /"port" argument must be a number, got "0"/);
});

assert.doesNotThrow(function() {
dns.lookupService('0.0.0.0', 0, noop);
});

assert.throws(function() {
dns.lookupService('0.0.0.0', null, noop);
}, /"port" should be >= 0 and < 65536, got "null"/);

assert.throws(function() {
dns.lookupService('0.0.0.0', undefined, noop);
}, /"port" should be >= 0 and < 65536, got "undefined"/);

assert.throws(function() {
dns.lookupService('0.0.0.0', 65538, noop);
}, /"port" should be >= 0 and < 65536, got "65538"/);

assert.throws(function() {
dns.lookupService('0.0.0.0', 'test', noop);
}, /"port" should be >= 0 and < 65536, got "test"/);

0 comments on commit f3be421

Please sign in to comment.