From 72028594026528f7eb640ebb2807d1a1c7bae172 Mon Sep 17 00:00:00 2001
From: ignoramous <ignoramous@users.noreply.github.com>
Date: Mon, 26 Jun 2023 11:47:51 +0530
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/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>
---
 doc/api/net.md                                |  5 +++++
 lib/internal/cluster/child.js                 |  2 +-
 lib/net.js                                    |  2 +-
 ...test-cluster-net-server-drop-connection.js | 19 ++++++++++++++-----
 .../test-net-server-drop-connections.js       | 15 +++++++++++++++
 5 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/doc/api/net.md b/doc/api/net.md
index 90306354c61b04..286e68d26aeda8 100644
--- 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
index 1bddd3ca0ac103..be5353c9288e15 100644
--- 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
index 68b577c7d52a14..e3591b72540ea8 100644
--- 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
index 5df62a9a630885..75a009a9eafa8e 100644
--- a/test/parallel/test-cluster-net-server-drop-connection.js
+++ b/test/parallel/test-cluster-net-server-drop-connection.js
@@ -10,13 +10,16 @@ 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);
   }
 }
@@ -24,11 +27,12 @@ function request(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
index 4c495bdfb9e981..3e10fc636f30f4 100644
--- 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);