Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cluster: correctly handle relative unix socket paths #16749

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/internal/cluster/child.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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(':');
Expand All @@ -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();
Expand Down
15 changes: 14 additions & 1 deletion lib/internal/cluster/master.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -275,6 +276,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
Expand All @@ -286,7 +299,7 @@ function queryServer(worker, message) {
}

handles[key] = handle = new constructor(key,
message.address,
address,
message.port,
message.addressType,
message.fd,
Expand Down
48 changes: 48 additions & 0 deletions test/parallel/test-cluster-net-listen-relative-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
'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');

// Ref:
// https://github.com/nodejs/node/issues/16387
// https://github.com/nodejs/node/pull/16749
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These refs can be removed during landing. They should be in the metadata, not in the comments.


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();
}));
}