Skip to content

Commit

Permalink
tls: reapply servername on happy eyeballs connect
Browse files Browse the repository at this point in the history
When establishing a TLS connection to a server with `autoSelectFamily`
set to `true`, the `net.Socket` will call `[kWrapConnectedHandle]()` to
reinitialize the socket (in case if it got broken during previous
connect attempts). Unfortunately, prior to this patch this resulted in a
brand new `TLSWrap` instance being created for the socket. While most of
the configuration of `TLSWrap` is restored, the `servername` was sadly
dropped and not reinitalized.

With this patch `servername` will be reinitialized if there are
`tls.connect` options present on the `TLSSocket` instance, making it
possible to connect with "Happy Eyeballs" to TLS servers that require
the servername extension.

PR-URL: nodejs#48255
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
  • Loading branch information
indutny authored and Ceres6 committed Aug 14, 2023
1 parent a27c30f commit 95715b1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
8 changes: 8 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,14 @@ TLSSocket.prototype._init = function(socket, wrap) {
}
}

// We can only come here via [kWrapConnectedHandle]() call that happens
// if the connection is established with `autoSelectFamily` set to `true`.
const connectOptions = this[kConnectOptions];
if (!options.isServer && connectOptions) {
if (connectOptions.servername) {
this.setServername(connectOptions.servername);
}
}

if (options.handshakeTimeout > 0)
this.setTimeout(options.handshakeTimeout, this._handleTimeout);
Expand Down
11 changes: 8 additions & 3 deletions test/parallel/test-https-autoselectfamily.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) {
// Test that IPV4 is reached if IPV6 is not reachable
{
createDnsServer('::1', '127.0.0.1', common.mustCall(function({ dnsServer, lookup }) {
const ipv4Server = createServer(options, common.mustCall((_, res) => {
const ipv4Server = createServer(options, common.mustCall((req, res) => {
assert.strictEqual(req.socket.servername, 'example.org');
res.writeHead(200, { Connection: 'close' });
res.end('response-ipv4');
}));
Expand All @@ -86,6 +87,7 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) {
lookup,
rejectUnauthorized: false,
autoSelectFamily: true,
servername: 'example.org',
},
(res) => {
assert.strictEqual(res.statusCode, 200);
Expand All @@ -111,12 +113,14 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) {
// Test that IPV4 is NOT reached if IPV6 is reachable
if (common.hasIPv6) {
createDnsServer('::1', '127.0.0.1', common.mustCall(function({ dnsServer, lookup }) {
const ipv4Server = createServer(options, common.mustNotCall((_, res) => {
const ipv4Server = createServer(options, common.mustNotCall((req, res) => {
assert.strictEqual(req.socket.servername, 'example.org');
res.writeHead(200, { Connection: 'close' });
res.end('response-ipv4');
}));

const ipv6Server = createServer(options, common.mustCall((_, res) => {
const ipv6Server = createServer(options, common.mustCall((req, res) => {
assert.strictEqual(req.socket.servername, 'example.org');
res.writeHead(200, { Connection: 'close' });
res.end('response-ipv6');
}));
Expand All @@ -131,6 +135,7 @@ if (common.hasIPv6) {
lookup,
rejectUnauthorized: false,
autoSelectFamily: true,
servername: 'example.org',
},
(res) => {
assert.strictEqual(res.statusCode, 200);
Expand Down

0 comments on commit 95715b1

Please sign in to comment.