From 3bbb759d476aed402326bfb93d91a1ef6c48288a Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Wed, 4 Oct 2023 16:19:42 +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 PR-URL: https://github.com/nodejs/node/pull/49980 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Joyee Cheung --- lib/_tls_wrap.js | 16 +++++++++--- test/parallel/test-tls-socket-close.js | 36 ++++++++++++++++++-------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index a5be90a4a1583f..b38558cbe93280 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..0c90593c6af5ca 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,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(); }