net: multiple listen() events fail silently

Problem:
It's possible to run listen()
  on a net.Server that's already listening to a port.
The result is silent failure,
  with the side effect of changing the connectionKey and or pipeName.

Solution:
  throw an error if listen method called more than once.
  close() method should be called between listen() method calls.

Refs: nodejs/node#8294
Fixes: nodejs/node#6190
Fixes: nodejs/node#11685
PR-URL: nodejs/node#13149
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>

Author: eduardbme

doc/api/http.md

@@ -809,8 +809,9 @@ This function is asynchronous. `callback` will be added as a listener for the
 
 Returns `server`.
 
-*Note*: The `server.listen()` method may be called multiple times. Each
-subsequent call will *re-open* the server using the provided options.
+*Note*: The `server.listen()` method can be called again if and only if there was an error
+during the first `server.listen()` call or `server.close()` has been called.
+Otherwise, an `ERR_SERVER_ALREADY_LISTEN` error will be thrown.
 
 ### server.listen(path[, callback])
 <!-- YAML
@@ -825,8 +826,9 @@ Start a UNIX socket server listening for connections on the given `path`.
 This function is asynchronous. `callback` will be added as a listener for the
 [`'listening'`][] event.  See also [`net.Server.listen(path)`][].
 
-*Note*: The `server.listen()` method may be called multiple times. Each
-subsequent call will *re-open* the server using the provided options.
+*Note*: The `server.listen()` method can be called again if and only if there was an error
+during the first `server.listen()` call or `server.close()` has been called.
+Otherwise, an `ERR_SERVER_ALREADY_LISTEN` error will be thrown.
 
 ### server.listen([port][, hostname][, backlog][, callback])
 <!-- YAML
@@ -861,8 +863,9 @@ parameter is 511 (not 512).
 This function is asynchronous. `callback` will be added as a listener for the
 [`'listening'`][] event.  See also [`net.Server.listen(port)`][].
 
-*Note*: The `server.listen()` method may be called multiple times. Each
-subsequent call will *re-open* the server using the provided options.
+*Note*: The `server.listen()` method can be called again if and only if there was an error
+during the first `server.listen()` call or `server.close()` has been called.
+Otherwise, an `ERR_SERVER_ALREADY_LISTEN` error will be thrown.
 
 ### server.listening
 <!-- YAML

doc/api/net.md

@@ -199,9 +199,9 @@ on Linux. The default value of this parameter is 511 (not 512).
 
 * All [`net.Socket`][] are set to `SO_REUSEADDR` (See [socket(7)][] for
   details).
-
-* The `server.listen()` method may be called multiple times. Each
-  subsequent call will *re-open* the server using the provided options.
+* The `server.listen()` method can be called again if and only if there was an error
+  during the first `server.listen()` call or `server.close()` has been called.
+  Otherwise, an `ERR_SERVER_ALREADY_LISTEN` error will be thrown.
 
 One of the most common errors raised when listening is `EADDRINUSE`.
 This happens when another server is already listening on the requested

lib/internal/errors.js

@@ -237,6 +237,8 @@ E('ERR_NO_ICU', '%s is not supported on Node.js compiled without ICU');
 E('ERR_NO_LONGER_SUPPORTED', '%s is no longer supported');
 E('ERR_OUTOFMEMORY', 'Out of memory');
 E('ERR_PARSE_HISTORY_DATA', 'Could not parse history data in %s');
+E('ERR_SERVER_ALREADY_LISTEN',
+  'Listen method has been called more than once without closing.');
 E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound');
 E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536');
 E('ERR_SOCKET_BAD_TYPE',

lib/net.js

@@ -1423,6 +1423,10 @@ Server.prototype.listen = function(...args) {
   var options = normalized[0];
   var cb = normalized[1];
 
+  if (this._handle) {
+    throw new errors.Error('ERR_SERVER_ALREADY_LISTEN');
+  }
+
   var hasCallback = (cb !== null);
   if (hasCallback) {
     this.once('listening', cb);

test/parallel/test-net-server-call-listen-multiple-times.js

@@ -0,0 +1,51 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+// First test. Check that after error event you can listen right away.
+{
+  const dummyServer = net.Server();
+  const server = net.Server();
+
+  // Run some server in order to simulate EADDRINUSE error.
+  dummyServer.listen(common.mustCall(() => {
+    // Try to listen used port.
+    server.listen(dummyServer.address().port);
+  }));
+
+  server.on('error', common.mustCall((e) => {
+    assert.doesNotThrow(
+      () => server.listen(common.mustCall(() => {
+        dummyServer.close();
+        server.close();
+      }))
+    );
+  }));
+}
+
+// Second test. Check that second listen call throws an error.
+{
+  const server = net.Server();
+
+  server.listen(common.mustCall(() => server.close()));
+
+  common.expectsError(() => server.listen(), {
+    code: 'ERR_SERVER_ALREADY_LISTEN',
+    type: Error
+  });
+}
+
+// Third test.
+// Check that after the close call you can run listen method just fine.
+{
+  const server = net.Server();
+
+  server.listen(common.mustCall(() => {
+    server.close();
+    assert.doesNotThrow(
+      () => server.listen(common.mustCall(() => server.close()))
+    );
+  }));
+}