From 6f2e8063ec4fb6ed41a4c7a58518ef56e546d09f Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Fri, 9 Jun 2017 17:29:50 +0200 Subject: [PATCH 1/2] http: handle cases where socket.server is null Fixes a regression that caused an error to be thrown when trying to emit the 'timeout' event on the server referenced by `socket.server`. Fixes: https://github.com/nodejs/node/issues/13435 Refs: https://github.com/nodejs/node/pull/11926 --- lib/_http_server.js | 5 ++++ test/parallel/test-regress-GH-13435.js | 37 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 test/parallel/test-regress-GH-13435.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 357400e3501228..6d5dbf4584a7ef 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -294,6 +294,11 @@ function connectionListener(socket) { httpSocketSetup(socket); + // Ensure that the server property of the socket is correctly set. + // See https://github.com/nodejs/node/issues/13435 + if (socket.server === null) + socket.server = this; + // If the user has added a listener to the server, // request, or response, then it's their responsibility. // otherwise, destroy on timeout by default diff --git a/test/parallel/test-regress-GH-13435.js b/test/parallel/test-regress-GH-13435.js new file mode 100644 index 00000000000000..28f00552092f8b --- /dev/null +++ b/test/parallel/test-regress-GH-13435.js @@ -0,0 +1,37 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); +const http = require('http'); +const net = require('net'); + +if (cluster.isMaster) { + const worker = cluster.fork(); + const server = net.createServer({ + pauseOnConnect: true + }, common.mustCall((socket) => { + worker.send('socket', socket); + })); + + worker.on('exit', common.mustCall((code, signal) => { + assert.strictEqual(code, 0); + server.close(); + })); + + server.listen(0, common.mustCall(() => { + net.createConnection(server.address().port); + })); +} else { + const server = http.createServer(); + + server.setTimeout(100, common.mustCall((socket) => { + socket.destroy(); + cluster.worker.kill(); + })); + + process.on('message', common.mustCall((message, socket) => { + server.emit('connection', socket); + socket.resume(); + })); +} From dd8e09a2d22db9d6c8b12b07e954b49e234de6f2 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Fri, 9 Jun 2017 18:44:59 +0200 Subject: [PATCH 2/2] fixup! http: handle cases where socket.server is null --- ...cluster-send-socket-to-worker-http-server.js} | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) rename test/parallel/{test-regress-GH-13435.js => test-cluster-send-socket-to-worker-http-server.js} (57%) diff --git a/test/parallel/test-regress-GH-13435.js b/test/parallel/test-cluster-send-socket-to-worker-http-server.js similarity index 57% rename from test/parallel/test-regress-GH-13435.js rename to test/parallel/test-cluster-send-socket-to-worker-http-server.js index 28f00552092f8b..805603d2c9b069 100644 --- a/test/parallel/test-regress-GH-13435.js +++ b/test/parallel/test-cluster-send-socket-to-worker-http-server.js @@ -1,5 +1,9 @@ 'use strict'; +// Regression test for https://github.com/nodejs/node/issues/13435 +// Tests that `socket.server` is correctly set when a socket is sent to a worker +// and the `'connection'` event is emitted manually on an HTTP server. + const common = require('../common'); const assert = require('assert'); const cluster = require('cluster'); @@ -8,13 +12,11 @@ const net = require('net'); if (cluster.isMaster) { const worker = cluster.fork(); - const server = net.createServer({ - pauseOnConnect: true - }, common.mustCall((socket) => { + const server = net.createServer(common.mustCall((socket) => { worker.send('socket', socket); })); - worker.on('exit', common.mustCall((code, signal) => { + worker.on('exit', common.mustCall((code) => { assert.strictEqual(code, 0); server.close(); })); @@ -25,13 +27,13 @@ if (cluster.isMaster) { } else { const server = http.createServer(); - server.setTimeout(100, common.mustCall((socket) => { + server.on('connection', common.mustCall((socket) => { + assert.strictEqual(socket.server, server); socket.destroy(); - cluster.worker.kill(); + cluster.worker.disconnect(); })); process.on('message', common.mustCall((message, socket) => { server.emit('connection', socket); - socket.resume(); })); }