Skip to content

Commit bd65004

Browse files
ignoramousCeres6
authored andcommitted
net: do not treat server.maxConnections=0 as Infinity
Setting the `maxConnections` to 0 should result in no connection. Instead, it was treated as if the option was not there. PR-URL: nodejs#48276 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent a80e180 commit bd65004

File tree

5 files changed

+36
-7
lines changed

5 files changed

+36
-7
lines changed

doc/api/net.md

+5
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,11 @@ added: v5.7.0
565565

566566
<!-- YAML
567567
added: v0.2.0
568+
changes:
569+
- version: REPLACEME
570+
pr-url: https://github.com/nodejs/node/pull/48276
571+
description: Setting `maxConnections` to `0` drops all the incoming
572+
connections. Previously, it was interpreted as `Infinity`.
568573
-->
569574

570575
* {integer}

lib/internal/cluster/child.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ function onconnection(message, handle) {
233233

234234
if (accepted && server[owner_symbol]) {
235235
const self = server[owner_symbol];
236-
if (self.maxConnections && self._connections >= self.maxConnections) {
236+
if (self.maxConnections != null && self._connections >= self.maxConnections) {
237237
accepted = false;
238238
}
239239
}

lib/net.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2062,7 +2062,7 @@ function onconnection(err, clientHandle) {
20622062
return;
20632063
}
20642064

2065-
if (self.maxConnections && self._connections >= self.maxConnections) {
2065+
if (self.maxConnections != null && self._connections >= self.maxConnections) {
20662066
if (clientHandle.getsockname || clientHandle.getpeername) {
20672067
const data = { __proto__: null };
20682068
if (clientHandle.getsockname) {

test/parallel/test-cluster-net-server-drop-connection.js

+14-5
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,29 @@ const tmpdir = require('../common/tmpdir');
1010
if (common.isWindows)
1111
common.skip('no setSimultaneousAccepts on pipe handle');
1212

13-
let connectionCount = 0;
14-
let listenCount = 0;
13+
const totalConns = 10;
14+
const totalWorkers = 3;
15+
let worker0;
1516
let worker1;
1617
let worker2;
18+
let connectionCount = 0;
19+
let listenCount = 0;
1720

1821
function request(path) {
19-
for (let i = 0; i < 10; i++) {
22+
for (let i = 0; i < totalConns; i++) {
2023
net.connect(path);
2124
}
2225
}
2326

2427
function handleMessage(message) {
2528
assert.match(message.action, /listen|connection/);
2629
if (message.action === 'listen') {
27-
if (++listenCount === 2) {
30+
if (++listenCount === totalWorkers) {
2831
request(common.PIPE);
2932
}
3033
} else if (message.action === 'connection') {
31-
if (++connectionCount === 10) {
34+
if (++connectionCount === totalConns) {
35+
worker0.send({ action: 'disconnect' });
3236
worker1.send({ action: 'disconnect' });
3337
worker2.send({ action: 'disconnect' });
3438
}
@@ -38,8 +42,13 @@ function handleMessage(message) {
3842
if (cluster.isPrimary) {
3943
cluster.schedulingPolicy = cluster.SCHED_RR;
4044
tmpdir.refresh();
45+
worker0 = cluster.fork({ maxConnections: 0, pipePath: common.PIPE });
4146
worker1 = cluster.fork({ maxConnections: 1, pipePath: common.PIPE });
4247
worker2 = cluster.fork({ maxConnections: 9, pipePath: common.PIPE });
48+
// expected = { action: 'listen' } + maxConnections * { action: 'connection' }
49+
worker0.on('message', common.mustCall((message) => {
50+
handleMessage(message);
51+
}, 1));
4352
worker1.on('message', common.mustCall((message) => {
4453
handleMessage(message);
4554
}, 2));

test/parallel/test-net-server-drop-connections.js

+15
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,23 @@ const assert = require('assert');
44
const net = require('net');
55

66
let firstSocket;
7+
const dormantServer = net.createServer(common.mustNotCall());
78
const server = net.createServer(common.mustCall((socket) => {
89
firstSocket = socket;
910
}));
1011

12+
dormantServer.maxConnections = 0;
1113
server.maxConnections = 1;
1214

15+
dormantServer.on('drop', common.mustCall((data) => {
16+
assert.strictEqual(!!data.localAddress, true);
17+
assert.strictEqual(!!data.localPort, true);
18+
assert.strictEqual(!!data.remoteAddress, true);
19+
assert.strictEqual(!!data.remotePort, true);
20+
assert.strictEqual(!!data.remoteFamily, true);
21+
dormantServer.close();
22+
}));
23+
1324
server.on('drop', common.mustCall((data) => {
1425
assert.strictEqual(!!data.localAddress, true);
1526
assert.strictEqual(!!data.localPort, true);
@@ -20,6 +31,10 @@ server.on('drop', common.mustCall((data) => {
2031
server.close();
2132
}));
2233

34+
dormantServer.listen(0, () => {
35+
net.createConnection(dormantServer.address().port);
36+
});
37+
2338
server.listen(0, () => {
2439
net.createConnection(server.address().port);
2540
net.createConnection(server.address().port);

0 commit comments

Comments
 (0)