Skip to content

Commit

Permalink
tls: handle cases where the raw socket is destroyed
Browse files Browse the repository at this point in the history
Ensure that the `'close'` event is emitted on a `TLSSocket` when it is
created from an existing raw `net.Socket` and the raw socket is not
closed cleanly (destroyed).

Refs: nodejs@048e0bec5147
Refs: nodejs#49902 (comment)
Fixes: nodejs#49902
PR-URL: nodejs#49980
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
lpinca authored and debadree25 committed Apr 15, 2024
1 parent b665c95 commit 3bbb759
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 14 deletions.
16 changes: 13 additions & 3 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -657,10 +657,20 @@ tls_wrap.TLSWrap.prototype.close = function close(cb) {
cb();
};

if (this._parentWrap && this._parentWrap._handle === this._parent) {
this._parentWrap.once('close', done);
return this._parentWrap.destroy();
if (this._parentWrap) {
if (this._parentWrap._handle === null) {
// The socket handle was already closed.
done();
return;
}

if (this._parentWrap._handle === this._parent) {
this._parentWrap.once('close', done);
this._parentWrap.destroy();
return;
}
}

return this._parent.close(done);
};

Expand Down
36 changes: 25 additions & 11 deletions test/parallel/test-tls-socket-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ if (!common.hasCrypto)
const assert = require('assert');
const tls = require('tls');
const net = require('net');
const Countdown = require('../common/countdown');
const fixtures = require('../common/fixtures');

const key = fixtures.readKey('agent2-key.pem');
Expand All @@ -14,19 +15,28 @@ const cert = fixtures.readKey('agent2-cert.pem');
let serverTlsSocket;
const tlsServer = tls.createServer({ cert, key }, (socket) => {
serverTlsSocket = socket;
socket.on('close', dec);
});

// A plain net server, that manually passes connections to the TLS
// server to be upgraded
// server to be upgraded.
let netSocket;
let netSocketCloseEmitted = false;
const netServer = net.createServer((socket) => {
tlsServer.emit('connection', socket);

netSocket = socket;
}).listen(0, common.mustCall(function() {
tlsServer.emit('connection', socket);
socket.on('close', () => {
netSocketCloseEmitted = true;
assert.strictEqual(serverTlsSocket.destroyed, true);
});
}).listen(0, common.mustCall(() => {
connectClient(netServer);
}));

const countdown = new Countdown(2, () => {
netServer.close();
});

// A client that connects, sends one message, and closes the raw connection:
function connectClient(server) {
const clientTlsSocket = tls.connect({
Expand All @@ -41,18 +51,22 @@ function connectClient(server) {
assert(serverTlsSocket);

netSocket.destroy();
assert.strictEqual(netSocket.destroyed, true);

setImmediate(() => {
assert.strictEqual(netSocket.destroyed, true);

// Close callbacks are executed after `setImmediate()` callbacks.
assert.strictEqual(netSocketCloseEmitted, false);
assert.strictEqual(serverTlsSocket.destroyed, false);
setImmediate(() => {
assert.strictEqual(clientTlsSocket.destroyed, true);
assert.strictEqual(serverTlsSocket.destroyed, true);

tlsServer.close();
netServer.close();
assert.strictEqual(netSocketCloseEmitted, true);
});
});
}));
}));

clientTlsSocket.on('close', dec);
}

function dec() {
countdown.dec();
}

0 comments on commit 3bbb759

Please sign in to comment.