From 156ea66cb99e0228d5eb7dd28a4b625209e6fe4b Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 1 Apr 2020 15:41:33 +0200 Subject: [PATCH 1/3] net: close connection if no 'connection' listener A `net.Server` without a `'connection'` event listener would accept incoming connections but then proceeded to drop them into the void: the connections stay around, using up resources, but can no longer be reached by the program - i.e., they become resource leaks. Avoid that by closing the connection when there is no listener. It would be even better to not accept connections at all but libuv does not currently support that because It's Complicated. --- doc/api/net.md | 5 +++++ lib/net.js | 5 +++++ test/parallel/test-net-server-no-handler.js | 12 ++++++++++++ test/parallel/test-net-socket-timeout.js | 2 +- 4 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-net-server-no-handler.js diff --git a/doc/api/net.md b/doc/api/net.md index a7b6eff6008136..038f5d4591f99c 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -83,6 +83,11 @@ event is not emitted until all connections are ended. ### Event: `'connection'` * {net.Socket} The connection object diff --git a/lib/net.js b/lib/net.js index ee0ffebe0ec45e..c6c4433cf642e3 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1536,6 +1536,11 @@ function onconnection(err, clientHandle) { return; } + if (self.listenerCount('connection') === 0) { + clientHandle.close(); + return; + } + if (self.maxConnections && self._connections >= self.maxConnections) { clientHandle.close(); return; diff --git a/test/parallel/test-net-server-no-handler.js b/test/parallel/test-net-server-no-handler.js new file mode 100644 index 00000000000000..4a144a01e052f5 --- /dev/null +++ b/test/parallel/test-net-server-no-handler.js @@ -0,0 +1,12 @@ +// Ensure that a net.Server without 'connection' event listeners closes +// incoming connections rather than accepting, then forgetting about them. + +'use strict'; +require('../common'); +const net = require('net'); + +net.createServer(/* no handler */).listen(function() { + const server = this; + const { address: host, port } = server.address(); + net.connect(port, host).once('end', () => server.close()); +}); diff --git a/test/parallel/test-net-socket-timeout.js b/test/parallel/test-net-socket-timeout.js index 8b197b44d61281..6e1cd06af412f1 100644 --- a/test/parallel/test-net-socket-timeout.js +++ b/test/parallel/test-net-socket-timeout.js @@ -67,7 +67,7 @@ for (let i = 0; i < invalidCallbacks.length; i++) { ); } -const server = net.Server(); +const server = net.Server((conn) => { /* do nothing */ }); server.listen(0, common.mustCall(() => { const socket = net.createConnection(server.address().port); socket.setTimeout(1, common.mustCall(() => { From 7316d740f3401dad5727e90773cdbcc11b6966d6 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 1 Apr 2020 15:41:34 +0200 Subject: [PATCH 2/3] fixup! update doc pr-url --- doc/api/net.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/net.md b/doc/api/net.md index 038f5d4591f99c..0bce91d9ff67b7 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -85,7 +85,7 @@ event is not emitted until all connections are ended. added: v0.1.90 changes: - version: REPLACEME - pr-url: XXX + pr-url: https://github.com/nodejs/node/pull/32516 description: New connections are now closed instead of silently leaking them when there are no `'connection'` listeners. --> From 41475dc0968d257452d6b2b48c916a0ce2a463f7 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 1 Apr 2020 15:42:30 +0200 Subject: [PATCH 3/3] fixup! use common.mustCall() --- test/parallel/test-net-server-no-handler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-net-server-no-handler.js b/test/parallel/test-net-server-no-handler.js index 4a144a01e052f5..d27b19b7e3fd67 100644 --- a/test/parallel/test-net-server-no-handler.js +++ b/test/parallel/test-net-server-no-handler.js @@ -2,10 +2,10 @@ // incoming connections rather than accepting, then forgetting about them. 'use strict'; -require('../common'); +const common = require('../common'); const net = require('net'); -net.createServer(/* no handler */).listen(function() { +net.createServer(common.mustCall()).listen(function() { const server = this; const { address: host, port } = server.address(); net.connect(port, host).once('end', () => server.close());