test: remove common.getServiceName()
Replace lightly-used services file parsing in favor of confirming one of a small number of allowable values in service name lookup tests. In https://github.com/nodejs/node-v0.x-archive/issues/8047, it was decided that this sort of service file parsing was superior to hardcoding acceptable values, but I'm not convinced: * No guarantee that the host uses /etc/services before, e.g., nscd. * Increases complexity of tests without guaranteeing robustness. I think that simply checking against a small set of expected values may be a better solution. Ideally, there would also be a unit test that used a test double for the appropriate `cares` function and confirms that it is called with the correct parameters, but now we're getting way ahead of ourselves. PR-URL: https://github.com/nodejs/node/pull/6709 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit is contained in:
parent
517d1da47c
commit
1a0c80a1da
@ -160,7 +160,7 @@ Object.defineProperty(exports, 'opensslCli', {get: function() {
|
|||||||
opensslCli = false;
|
opensslCli = false;
|
||||||
}
|
}
|
||||||
return opensslCli;
|
return opensslCli;
|
||||||
}, enumerable: true });
|
}, enumerable: true});
|
||||||
|
|
||||||
Object.defineProperty(exports, 'hasCrypto', {
|
Object.defineProperty(exports, 'hasCrypto', {
|
||||||
get: function() {
|
get: function() {
|
||||||
@ -395,55 +395,6 @@ exports.mustCall = function(fn, expected) {
|
|||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
var etcServicesFileName = path.join('/etc', 'services');
|
|
||||||
if (exports.isWindows) {
|
|
||||||
etcServicesFileName = path.join(process.env.SystemRoot, 'System32', 'drivers',
|
|
||||||
'etc', 'services');
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
|
||||||
* Returns a string that represents the service name associated
|
|
||||||
* to the service bound to port "port" and using protocol "protocol".
|
|
||||||
*
|
|
||||||
* If the service is not defined in the services file, it returns
|
|
||||||
* the port number as a string.
|
|
||||||
*
|
|
||||||
* Returns undefined if /etc/services (or its equivalent on non-UNIX
|
|
||||||
* platforms) can't be read.
|
|
||||||
*/
|
|
||||||
exports.getServiceName = function getServiceName(port, protocol) {
|
|
||||||
if (port == null) {
|
|
||||||
throw new Error('Missing port number');
|
|
||||||
}
|
|
||||||
|
|
||||||
if (typeof protocol !== 'string') {
|
|
||||||
throw new Error('Protocol must be a string');
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
|
||||||
* By default, if a service can't be found in /etc/services,
|
|
||||||
* its name is considered to be its port number.
|
|
||||||
*/
|
|
||||||
var serviceName = port.toString();
|
|
||||||
|
|
||||||
try {
|
|
||||||
var servicesContent = fs.readFileSync(etcServicesFileName,
|
|
||||||
{ encoding: 'utf8'});
|
|
||||||
var regexp = `^(\\w+)\\s+\\s${port}/${protocol}\\s`;
|
|
||||||
var re = new RegExp(regexp, 'm');
|
|
||||||
|
|
||||||
var matches = re.exec(servicesContent);
|
|
||||||
if (matches && matches.length > 1) {
|
|
||||||
serviceName = matches[1];
|
|
||||||
}
|
|
||||||
} catch (e) {
|
|
||||||
console.error('Cannot read file: ', etcServicesFileName);
|
|
||||||
return undefined;
|
|
||||||
}
|
|
||||||
|
|
||||||
return serviceName;
|
|
||||||
};
|
|
||||||
|
|
||||||
exports.hasMultiLocalhost = function hasMultiLocalhost() {
|
exports.hasMultiLocalhost = function hasMultiLocalhost() {
|
||||||
var TCP = process.binding('tcp_wrap').TCP;
|
var TCP = process.binding('tcp_wrap').TCP;
|
||||||
var t = new TCP();
|
var t = new TCP();
|
||||||
|
@ -1,5 +1,5 @@
|
|||||||
'use strict';
|
'use strict';
|
||||||
const common = require('../common');
|
require('../common');
|
||||||
const assert = require('assert');
|
const assert = require('assert');
|
||||||
const dns = require('dns');
|
const dns = require('dns');
|
||||||
const net = require('net');
|
const net = require('net');
|
||||||
@ -173,24 +173,7 @@ TEST(function test_lookupservice_ip_ipv4(done) {
|
|||||||
if (err) throw err;
|
if (err) throw err;
|
||||||
assert.equal(typeof host, 'string');
|
assert.equal(typeof host, 'string');
|
||||||
assert(host);
|
assert(host);
|
||||||
|
assert(['http', 'www', '80'].includes(service));
|
||||||
/*
|
|
||||||
* Retrieve the actual HTTP service name as setup on the host currently
|
|
||||||
* running the test by reading it from /etc/services. This is not ideal,
|
|
||||||
* as the service name lookup could use another mechanism (e.g nscd), but
|
|
||||||
* it's already better than hardcoding it.
|
|
||||||
*/
|
|
||||||
var httpServiceName = common.getServiceName(80, 'tcp');
|
|
||||||
if (!httpServiceName) {
|
|
||||||
/*
|
|
||||||
* Couldn't find service name, reverting to the most sensible default
|
|
||||||
* for port 80.
|
|
||||||
*/
|
|
||||||
httpServiceName = 'http';
|
|
||||||
}
|
|
||||||
|
|
||||||
assert.strictEqual(service, httpServiceName);
|
|
||||||
|
|
||||||
done();
|
done();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -182,24 +182,7 @@ TEST(function test_lookupservice_ip_ipv6(done) {
|
|||||||
if (err) throw err;
|
if (err) throw err;
|
||||||
assert.equal(typeof host, 'string');
|
assert.equal(typeof host, 'string');
|
||||||
assert(host);
|
assert(host);
|
||||||
|
assert(['http', 'www', '80'].includes(service));
|
||||||
/*
|
|
||||||
* Retrieve the actual HTTP service name as setup on the host currently
|
|
||||||
* running the test by reading it from /etc/services. This is not ideal,
|
|
||||||
* as the service name lookup could use another mechanism (e.g nscd), but
|
|
||||||
* it's already better than hardcoding it.
|
|
||||||
*/
|
|
||||||
var httpServiceName = common.getServiceName(80, 'tcp');
|
|
||||||
if (!httpServiceName) {
|
|
||||||
/*
|
|
||||||
* Couldn't find service name, reverting to the most sensible default
|
|
||||||
* for port 80.
|
|
||||||
*/
|
|
||||||
httpServiceName = 'http';
|
|
||||||
}
|
|
||||||
|
|
||||||
assert.strictEqual(service, httpServiceName);
|
|
||||||
|
|
||||||
done();
|
done();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user