From 4e2e7d11e16d185a088aec77d1adc7bb4b606806 Mon Sep 17 00:00:00 2001 From: laino Date: Mon, 8 Jan 2018 21:05:33 +0100 Subject: [PATCH] cluster: resolve relative unix socket paths Relative unix sockets paths were previously interpreted relative to the master's CWD, which was inconsistent with non-cluster behavior. A test case has been added as well. PR-URL: https://github.com/nodejs/node/pull/16749 Fixes: https://github.com/nodejs/node/issues/16387 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- lib/internal/cluster/child.js | 12 ++++- lib/internal/cluster/master.js | 15 ++++++- .../test-cluster-net-listen-relative-path.js | 44 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-cluster-net-listen-relative-path.js diff --git a/lib/internal/cluster/child.js b/lib/internal/cluster/child.js index 98c5e7b5597f74..40c1a12327558f 100644 --- a/lib/internal/cluster/child.js +++ b/lib/internal/cluster/child.js @@ -1,6 +1,7 @@ 'use strict'; const assert = require('assert'); const util = require('util'); +const path = require('path'); const EventEmitter = require('events'); const Worker = require('internal/cluster/worker'); const { internal, sendHelper } = require('internal/cluster/utils'); @@ -48,7 +49,14 @@ cluster._setupWorker = function() { // obj is a net#Server or a dgram#Socket object. cluster._getServer = function(obj, options, cb) { - const indexesKey = [options.address, + let address = options.address; + + // Resolve unix socket paths to absolute paths + if (options.port < 0 && typeof address === 'string' && + process.platform !== 'win32') + address = path.resolve(address); + + const indexesKey = [address, options.port, options.addressType, options.fd ].join(':'); @@ -64,6 +72,8 @@ cluster._getServer = function(obj, options, cb) { data: null }, options); + message.address = address; + // Set custom data on handle (i.e. tls tickets key) if (obj._getServerData) message.data = obj._getServerData(); diff --git a/lib/internal/cluster/master.js b/lib/internal/cluster/master.js index 408b31c2b77805..280955549adfa5 100644 --- a/lib/internal/cluster/master.js +++ b/lib/internal/cluster/master.js @@ -2,6 +2,7 @@ const assert = require('assert'); const { fork } = require('child_process'); const util = require('util'); +const path = require('path'); const EventEmitter = require('events'); const RoundRobinHandle = require('internal/cluster/round_robin_handle'); const SharedHandle = require('internal/cluster/shared_handle'); @@ -276,6 +277,18 @@ function queryServer(worker, message) { var handle = handles[key]; if (handle === undefined) { + let address = message.address; + + // Find shortest path for unix sockets because of the ~100 byte limit + if (message.port < 0 && typeof address === 'string' && + process.platform !== 'win32') { + + address = path.relative(process.cwd(), address); + + if (message.address.length < address.length) + address = message.address; + } + var constructor = RoundRobinHandle; // UDP is exempt from round-robin connection balancing for what should // be obvious reasons: it's connectionless. There is nothing to send to @@ -287,7 +300,7 @@ function queryServer(worker, message) { } handles[key] = handle = new constructor(key, - message.address, + address, message.port, message.addressType, message.fd, diff --git a/test/parallel/test-cluster-net-listen-relative-path.js b/test/parallel/test-cluster-net-listen-relative-path.js new file mode 100644 index 00000000000000..0fcc6a0dca4160 --- /dev/null +++ b/test/parallel/test-cluster-net-listen-relative-path.js @@ -0,0 +1,44 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); +const net = require('net'); +const path = require('path'); +const fs = require('fs'); + +if (common.isWindows) + common.skip('On Windows named pipes live in their own ' + + 'filesystem and don\'t have a ~100 byte limit'); + +// Choose a socket name such that the absolute path would exceed 100 bytes. +const socketDir = './unix-socket-dir'; +const socketName = 'A'.repeat(100 - socketDir.length - 1); + +// Make sure we're not in a weird environment +assert.strictEqual(path.resolve(socketDir, socketName).length > 100, true, + 'absolute socket path should be longer than 100 bytes'); + +if (cluster.isMaster) { + // ensure that the worker exits peacefully + process.chdir(common.tmpDir); + fs.mkdirSync(socketDir); + cluster.fork().on('exit', common.mustCall(function(statusCode) { + assert.strictEqual(statusCode, 0); + + assert.strictEqual( + fs.existsSync(path.join(socketDir, socketName)), false, + 'Socket should be removed when the worker exits'); + })); +} else { + process.chdir(socketDir); + + const server = net.createServer(common.mustNotCall()); + + server.listen(socketName, common.mustCall(function() { + assert.strictEqual( + fs.existsSync(socketName), true, + 'Socket created in CWD'); + + process.disconnect(); + })); +}