From b24e269a482812193fb3cd8b6ced4477f8e5e1c5 Mon Sep 17 00:00:00 2001
From: Eduard Bondarenko <eduardbcom@gmail.com>
Date: Mon, 22 May 2017 13:35:22 +0300
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/8294
Fixes: https://github.com/nodejs/node/issues/6190
Fixes: https://github.com/nodejs/node/issues/11685
PR-URL: https://github.com/nodejs/node/pull/13149
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
---
 doc/api/http.md                               | 15 +++---
 doc/api/net.md                                |  6 +--
 lib/internal/errors.js                        |  2 +
 lib/net.js                                    |  4 ++
 ...t-net-server-call-listen-multiple-times.js | 51 +++++++++++++++++++
 5 files changed, 69 insertions(+), 9 deletions(-)
 create mode 100644 test/parallel/test-net-server-call-listen-multiple-times.js

diff --git a/doc/api/http.md b/doc/api/http.md
index 2a2a87ce60..c0fe0e72bc 100644
--- a/doc/api/http.md
+++ b/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
diff --git a/doc/api/net.md b/doc/api/net.md
index 36280c0649..2c5a05e2dc 100644
--- a/doc/api/net.md
+++ b/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
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 906bd51652..623e57d875 100644
--- a/lib/internal/errors.js
+++ b/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',
diff --git a/lib/net.js b/lib/net.js
index 797c098335..d693b25d46 100644
--- a/lib/net.js
+++ b/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);
diff --git a/test/parallel/test-net-server-call-listen-multiple-times.js b/test/parallel/test-net-server-call-listen-multiple-times.js
new file mode 100644
index 0000000000..05099dadc8
--- /dev/null
+++ b/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()))
+    );
+  }));
+}