From c14baa7029ece70ee88654e844c9e96670b1f9bb Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sat, 30 Sep 2023 17:09:37 +0200 Subject: [PATCH] tls: handle cases where the raw socket is destroyed 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: https://github.com/nodejs/node/commit/048e0bec5147 Refs: https://github.com/nodejs/node/issues/49902#issuecomment-1741203813 Fixes: https://github.com/nodejs/node/issues/49902 --- lib/_tls_wrap.js | 16 +++++++++--- test/parallel/test-tls-socket-close.js | 35 ++++++++++++++++++-------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index c2dd958f95106e..170c1ea4b7e922 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -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); }; diff --git a/test/parallel/test-tls-socket-close.js b/test/parallel/test-tls-socket-close.js index 70af760d53bb4d..e6401d19dfbdfa 100644 --- a/test/parallel/test-tls-socket-close.js +++ b/test/parallel/test-tls-socket-close.js @@ -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'); @@ -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({ @@ -41,18 +51,21 @@ function connectClient(server) { assert(serverTlsSocket); netSocket.destroy(); + assert.strictEqual(netSocket.destroyed, true); setImmediate(() => { - assert.strictEqual(netSocket.destroyed, true); - + 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(); }