net: do not treat server.maxConnections=0 as Infinity

ignoramous · web-flow · commit 720285940265 · 2023-06-26T06:17:51.000Z
Setting the `maxConnections` to 0 should result in no connection. Instead, it was treated as if the option was not there. PR-URL: #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>
diff --git a/doc/api/net.md b/doc/api/net.md
@@ -565,6 +565,11 @@ added: v5.7.0
 
 <!-- YAML
 added: v0.2.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/48276
+    description: Setting `maxConnections` to `0` drops all the incoming
+                 connections. Previously, it was interpreted as `Infinity`.
 -->
 
 * {integer}
diff --git a/lib/internal/cluster/child.js b/lib/internal/cluster/child.js
@@ -233,7 +233,7 @@ function onconnection(message, handle) {
 
   if (accepted && server[owner_symbol]) {
     const self = server[owner_symbol];
-    if (self.maxConnections && self._connections >= self.maxConnections) {
+    if (self.maxConnections != null && self._connections >= self.maxConnections) {
       accepted = false;
     }
   }
diff --git a/lib/net.js b/lib/net.js
@@ -2062,7 +2062,7 @@ function onconnection(err, clientHandle) {
     return;
   }
 
-  if (self.maxConnections && self._connections >= self.maxConnections) {
+  if (self.maxConnections != null && self._connections >= self.maxConnections) {
     if (clientHandle.getsockname || clientHandle.getpeername) {
       const data = { __proto__: null };
       if (clientHandle.getsockname) {
diff --git a/test/parallel/test-cluster-net-server-drop-connection.js b/test/parallel/test-cluster-net-server-drop-connection.js
@@ -10,25 +10,29 @@ const tmpdir = require('../common/tmpdir');
 if (common.isWindows)
   common.skip('no setSimultaneousAccepts on pipe handle');
 
-let connectionCount = 0;
-let listenCount = 0;
+const totalConns = 10;
+const totalWorkers = 3;
+let worker0;
 let worker1;
 let worker2;
+let connectionCount = 0;
+let listenCount = 0;
 
 function request(path) {
-  for (let i = 0; i < 10; i++) {
+  for (let i = 0; i < totalConns; i++) {
     net.connect(path);
   }
 }
 
 function handleMessage(message) {
   assert.match(message.action, /listen|connection/);
   if (message.action === 'listen') {
-    if (++listenCount === 2) {
+    if (++listenCount === totalWorkers) {
       request(common.PIPE);
     }
   } else if (message.action === 'connection') {
-    if (++connectionCount === 10) {
+    if (++connectionCount === totalConns) {
+      worker0.send({ action: 'disconnect' });
       worker1.send({ action: 'disconnect' });
       worker2.send({ action: 'disconnect' });
     }
@@ -38,8 +42,13 @@ function handleMessage(message) {
 if (cluster.isPrimary) {
   cluster.schedulingPolicy = cluster.SCHED_RR;
   tmpdir.refresh();
+  worker0 = cluster.fork({ maxConnections: 0, pipePath: common.PIPE });
   worker1 = cluster.fork({ maxConnections: 1, pipePath: common.PIPE });
   worker2 = cluster.fork({ maxConnections: 9, pipePath: common.PIPE });
+  // expected = { action: 'listen' } + maxConnections * { action: 'connection' }
+  worker0.on('message', common.mustCall((message) => {
+    handleMessage(message);
+  }, 1));
   worker1.on('message', common.mustCall((message) => {
     handleMessage(message);
   }, 2));
diff --git a/test/parallel/test-net-server-drop-connections.js b/test/parallel/test-net-server-drop-connections.js
@@ -4,12 +4,23 @@ const assert = require('assert');
 const net = require('net');
 
 let firstSocket;
+const dormantServer = net.createServer(common.mustNotCall());
 const server = net.createServer(common.mustCall((socket) => {
   firstSocket = socket;
 }));
 
+dormantServer.maxConnections = 0;
 server.maxConnections = 1;
 
+dormantServer.on('drop', common.mustCall((data) => {
+  assert.strictEqual(!!data.localAddress, true);
+  assert.strictEqual(!!data.localPort, true);
+  assert.strictEqual(!!data.remoteAddress, true);
+  assert.strictEqual(!!data.remotePort, true);
+  assert.strictEqual(!!data.remoteFamily, true);
+  dormantServer.close();
+}));
+
 server.on('drop', common.mustCall((data) => {
   assert.strictEqual(!!data.localAddress, true);
   assert.strictEqual(!!data.localPort, true);
@@ -20,6 +31,10 @@ server.on('drop', common.mustCall((data) => {
   server.close();
 }));
 
+dormantServer.listen(0, () => {
+  net.createConnection(dormantServer.address().port);
+});
+
 server.listen(0, () => {
   net.createConnection(server.address().port);
   net.createConnection(server.address().port);

Original file line number	Diff line number	Diff line change
`@@ -233,7 +233,7 @@ function onconnection(message, handle) {`
`233`	`233`
`234`	`234`	`if (accepted && server[owner_symbol]) {`
`235`	`235`	`const self = server[owner_symbol];`
`236`		`- if (self.maxConnections && self._connections >= self.maxConnections) {`
	`236`	`+ if (self.maxConnections != null && self._connections >= self.maxConnections) {`
`237`	`237`	`accepted = false;`
`238`	`238`	`}`
`239`	`239`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2062,7 +2062,7 @@ function onconnection(err, clientHandle) {`
`2062`	`2062`	`return;`
`2063`	`2063`	`}`
`2064`	`2064`
`2065`		`- if (self.maxConnections && self._connections >= self.maxConnections) {`
	`2065`	`+ if (self.maxConnections != null && self._connections >= self.maxConnections) {`
`2066`	`2066`	`if (clientHandle.getsockname \|\| clientHandle.getpeername) {`
`2067`	`2067`	`const data = { __proto__: null };`
`2068`	`2068`	`if (clientHandle.getsockname) {`