diff options
author | jBarz <jbarboza@ca.ibm.com> | 2017-03-09 08:33:59 -0500 |
---|---|---|
committer | Myles Borins <mylesborins@google.com> | 2017-04-18 20:02:07 -0400 |
commit | 1baee1829b5ead5bde582d3e146e0c2ee16734fa (patch) | |
tree | ea1ae9f9ad7595c3d3b14c3afc9f171fac9cde65 | |
parent | 2c2a6649c18a6ef2b0ea01f280b7467f35b4c081 (diff) | |
download | node-new-1baee1829b5ead5bde582d3e146e0c2ee16734fa.tar.gz |
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 <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
-rw-r--r-- | lib/_tls_wrap.js | 6 | ||||
-rw-r--r-- | src/tls_wrap.cc | 11 | ||||
-rw-r--r-- | src/tls_wrap.h | 1 | ||||
-rw-r--r-- | test/parallel/test-tls-socket-close.js | 43 |
4 files changed, 60 insertions, 1 deletions
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index a4e3d6bd08..b4b975b9d4 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -378,6 +378,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 d1b1aeccdd..6a838c610b 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -522,7 +522,7 @@ int TLSWrap::GetFD() { bool TLSWrap::IsAlive() { - return ssl_ != nullptr && stream_->IsAlive(); + return ssl_ != nullptr && stream_ != nullptr && stream_->IsAlive(); } @@ -782,6 +782,14 @@ void TLSWrap::EnableSessionCallbacks( } +void TLSWrap::OnStreamClose(const FunctionCallbackInfo<Value>& args) { + TLSWrap* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); + + wrap->stream_ = nullptr; +} + + void TLSWrap::DestroySSL(const FunctionCallbackInfo<Value>& args) { TLSWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); @@ -912,6 +920,7 @@ void TLSWrap::Initialize(Local<Object> target, env->SetProtoMethod(t, "enableSessionCallbacks", EnableSessionCallbacks); env->SetProtoMethod(t, "destroySSL", DestroySSL); env->SetProtoMethod(t, "enableCertCb", EnableCertCb); + env->SetProtoMethod(t, "onStreamClose", OnStreamClose); StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev); SSLWrap<TLSWrap>::AddMethods(env, t); diff --git a/src/tls_wrap.h b/src/tls_wrap.h index f390c9fe92..0efdbf0e39 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -137,6 +137,7 @@ class TLSWrap : public AsyncWrap, static void EnableCertCb( const v8::FunctionCallbackInfo<v8::Value>& args); static void DestroySSL(const v8::FunctionCallbackInfo<v8::Value>& args); + static void OnStreamClose(const v8::FunctionCallbackInfo<v8::Value>& args); #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB static void GetServername(const v8::FunctionCallbackInfo<v8::Value>& args); diff --git a/test/parallel/test-tls-socket-close.js b/test/parallel/test-tls-socket-close.js new file mode 100644 index 0000000000..440c0c4ff7 --- /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'); +})); |