net: check EADDRINUSE after binding localPort

PR-URL: #15097 Fixes: #15084 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
nodejs · Sep 3, 2017 · c419adf · c419adf
1 parent d7200d6
commit c419adf
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 14 deletions.
diff --git a/lib/net.js b/lib/net.js
@@ -877,6 +877,27 @@ function afterWrite(status, handle, req, err) {
 }
 
 
+function checkBindError(err, port, handle) {
+  // EADDRINUSE may not be reported until we call listen() or connect().
+  // To complicate matters, a failed bind() followed by listen() or connect()
+  // will implicitly bind to a random port. Ergo, check that the socket is
+  // bound to the expected port before calling listen() or connect().
+  //
+  // FIXME(bnoordhuis) Doesn't work for pipe handles, they don't have a
+  // getsockname() method. Non-issue for now, the cluster module doesn't
+  // really support pipes anyway.
+  if (err === 0 && port > 0 && handle.getsockname) {
+    var out = {};
+    err = handle.getsockname(out);
+    if (err === 0 && port !== out.port) {
+      debug(`checkBindError, bound to ${out.port} instead of ${port}`);
+      err = UV_EADDRINUSE;
+    }
+  }
+  return err;
+}
+
+
 function internalConnect(
   self, address, port, addressType, localAddress, localPort) {
   // TODO return promise from Socket.prototype.connect which
@@ -900,6 +921,7 @@ function internalConnect(
     debug('binding to localAddress: %s and localPort: %d (addressType: %d)',
           localAddress, localPort, addressType);
 
+    err = checkBindError(err, localPort, self._handle);
     if (err) {
       const ex = exceptionWithHostPort(err, 'bind', localAddress, localPort);
       self.destroy(ex);
@@ -1380,20 +1402,7 @@ function listenInCluster(server, address, port, addressType,
   cluster._getServer(server, serverQuery, listenOnMasterHandle);
 
   function listenOnMasterHandle(err, handle) {
-    // EADDRINUSE may not be reported until we call listen(). To complicate
-    // matters, a failed bind() followed by listen() will implicitly bind to
-    // a random port. Ergo, check that the socket is bound to the expected
-    // port before calling listen().
-    //
-    // FIXME(bnoordhuis) Doesn't work for pipe handles, they don't have a
-    // getsockname() method. Non-issue for now, the cluster module doesn't
-    // really support pipes anyway.
-    if (err === 0 && port > 0 && handle.getsockname) {
-      var out = {};
-      err = handle.getsockname(out);
-      if (err === 0 && port !== out.port)
-        err = UV_EADDRINUSE;
-    }
+    err = checkBindError(err, port, handle);
 
     if (err) {
       var ex = exceptionWithHostPort(err, 'bind', address, port);

diff --git a/test/parallel/test-net-client-bind-twice.js b/test/parallel/test-net-client-bind-twice.js
@@ -0,0 +1,26 @@
+'use strict';
+
+// This tests that net.connect() from a used local port throws EADDRINUSE.
+
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+const server1 = net.createServer(common.mustNotCall());
+server1.listen(0, common.localhostIPv4, common.mustCall(() => {
+  const server2 = net.createServer(common.mustNotCall());
+  server2.listen(0, common.localhostIPv4, common.mustCall(() => {
+    const client = net.connect({
+      host: common.localhostIPv4,
+      port: server1.address().port,
+      localAddress: common.localhostIPv4,
+      localPort: server2.address().port
+    }, common.mustNotCall());
+
+    client.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'EADDRINUSE');
+      server1.close();
+      server2.close();
+    }));
+  }));
+}));