Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: check EADDRINUSE after binding localPort #15097

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
It makes me happy to see a new test with a description.


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();
}));
}));
}));