Skip to content

Commit

Permalink
net: check EADDRINUSE after binding localPort
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
joyeecheung authored and BridgeAR committed Sep 3, 2017
1 parent d7200d6 commit c419adf
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 14 deletions.
37 changes: 23 additions & 14 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-net-client-bind-twice.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}));
}));

0 comments on commit c419adf

Please sign in to comment.