From 611851dabad3ab4f2f419f490b9b1f6818de7754 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 2 Aug 2017 02:50:31 +0200 Subject: [PATCH] child_process: fix handle passing w large payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix situations in which the handle passed along with a message that has a large payload and can’t be read entirely by a single `recvmsg()` call isn’t associated with the message to which it belongs. PR-URL: https://github.com/nodejs/node/pull/14588 Fixes: https://github.com/nodejs/node/issues/13778 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Santiago Gimeno --- lib/internal/child_process.js | 12 ++++-- .../test-cluster-send-handle-large-payload.js | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-cluster-send-handle-large-payload.js diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 28866d2f2ae..d2856df4891 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -455,10 +455,14 @@ function setupChannel(target, channel) { var decoder = new StringDecoder('utf8'); var jsonBuffer = ''; + var pendingHandle = null; channel.buffering = false; channel.onread = function(nread, pool, recvHandle) { // TODO(bnoordhuis) Check that nread > 0. if (pool) { + if (recvHandle) + pendingHandle = recvHandle; + // Linebreak is used as a message end sign var chunks = decoder.write(pool).split('\n'); var numCompleteChunks = chunks.length - 1; @@ -478,10 +482,12 @@ function setupChannel(target, channel) { // read because SCM_RIGHTS messages don't get coalesced. Make sure // that we deliver the handle with the right message however. if (isInternal(message)) { - if (message.cmd === 'NODE_HANDLE') - handleMessage(message, recvHandle, true); - else + if (message.cmd === 'NODE_HANDLE') { + handleMessage(message, pendingHandle, true); + pendingHandle = null; + } else { handleMessage(message, undefined, true); + } } else { handleMessage(message, undefined, false); } diff --git a/test/parallel/test-cluster-send-handle-large-payload.js b/test/parallel/test-cluster-send-handle-large-payload.js new file mode 100644 index 00000000000..ba529593e28 --- /dev/null +++ b/test/parallel/test-cluster-send-handle-large-payload.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); +const net = require('net'); + +const payload = 'a'.repeat(800004); + +if (cluster.isMaster) { + const server = net.createServer(); + + server.on('connection', common.mustCall((socket) => socket.unref())); + + const worker = cluster.fork(); + worker.on('message', common.mustCall(({ payload: received }, handle) => { + assert.strictEqual(payload, received); + assert(handle instanceof net.Socket); + server.close(); + handle.destroy(); + })); + + server.listen(0, common.mustCall(() => { + const port = server.address().port; + const socket = new net.Socket(); + socket.connect(port, (err) => { + assert.ifError(err); + worker.send({ payload }, socket); + }); + })); +} else { + process.on('message', common.mustCall(({ payload: received }, handle) => { + assert.strictEqual(payload, received); + assert(handle instanceof net.Socket); + process.send({ payload }, handle); + + // Prepare for a clean exit. + process.channel.unref(); + handle.unref(); + })); +}