From 4cdb0e89d8daf7e1371c3b8d3f057940aa327d4a Mon Sep 17 00:00:00 2001 From: jBarz Date: Thu, 9 Mar 2017 08:33:59 -0500 Subject: [PATCH] tls: keep track of stream that is closed TLSWrap object keeps a pointer reference to the underlying TCPWrap object. This TCPWrap object could be closed and deleted by the event-loop which leaves us with a dangling pointer. So the TLSWrap object needs to track the "close" event on the TCPWrap object. PR-URL: https://github.com/nodejs/node/pull/11776 Reviewed-By: Fedor Indutny Reviewed-By: James M Snell Reviewed-By: Brian White --- lib/_tls_wrap.js | 6 ++++ src/tls_wrap.cc | 11 ++++++- src/tls_wrap.h | 1 + test/parallel/test-tls-socket-close.js | 43 ++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-socket-close.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 64c82b6957efb4..f87a390dcc577b 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -395,6 +395,12 @@ TLSSocket.prototype._wrapHandle = function(wrap) { res = null; }); + if (wrap) { + wrap.on('close', function() { + res.onStreamClose(); + }); + } + return res; }; diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index e23e686b9d3088..ad6daa43011bda 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -543,7 +543,7 @@ int TLSWrap::GetFD() { bool TLSWrap::IsAlive() { - return ssl_ != nullptr && stream_->IsAlive(); + return ssl_ != nullptr && stream_ != nullptr && stream_->IsAlive(); } @@ -802,6 +802,14 @@ void TLSWrap::EnableSessionCallbacks( } +void TLSWrap::OnStreamClose(const FunctionCallbackInfo& args) { + TLSWrap* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); + + wrap->stream_ = nullptr; +} + + void TLSWrap::DestroySSL(const FunctionCallbackInfo& args) { TLSWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); @@ -932,6 +940,7 @@ void TLSWrap::Initialize(Local target, env->SetProtoMethod(t, "enableSessionCallbacks", EnableSessionCallbacks); env->SetProtoMethod(t, "destroySSL", DestroySSL); env->SetProtoMethod(t, "enableCertCb", EnableCertCb); + env->SetProtoMethod(t, "onStreamClose", OnStreamClose); StreamBase::AddMethods(env, t, StreamBase::kFlagHasWritev); SSLWrap::AddMethods(env, t); diff --git a/src/tls_wrap.h b/src/tls_wrap.h index d0313bd308f9dc..d6c4b62493d10b 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -158,6 +158,7 @@ class TLSWrap : public AsyncWrap, static void EnableCertCb( const v8::FunctionCallbackInfo& args); static void DestroySSL(const v8::FunctionCallbackInfo& args); + static void OnStreamClose(const v8::FunctionCallbackInfo& args); #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB static void GetServername(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-tls-socket-close.js b/test/parallel/test-tls-socket-close.js new file mode 100644 index 00000000000000..440c0c4ff7cde7 --- /dev/null +++ b/test/parallel/test-tls-socket-close.js @@ -0,0 +1,43 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const tls = require('tls'); +const fs = require('fs'); +const net = require('net'); + +const key = fs.readFileSync(common.fixturesDir + '/keys/agent2-key.pem'); +const cert = fs.readFileSync(common.fixturesDir + '/keys/agent2-cert.pem'); + +const T = 100; + +// tls server +const tlsServer = tls.createServer({ cert, key }, (socket) => { + setTimeout(() => { + socket.on('error', (error) => { + assert.strictEqual(error.code, 'EINVAL'); + tlsServer.close(); + netServer.close(); + }); + socket.write('bar'); + }, T * 2); +}); + +// plain tcp server +const netServer = net.createServer((socket) => { + // if client wants to use tls + tlsServer.emit('connection', socket); + + socket.setTimeout(T, () => { + // this breaks if TLSSocket is already managing the socket: + socket.destroy(); + }); +}).listen(0, common.mustCall(function() { + + // connect client + tls.connect({ + host: 'localhost', + port: this.address().port, + rejectUnauthorized: false + }).write('foo'); +}));