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: #11776 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
nodejs · Mar 20, 2017 · 5408301 · 5408301
1 parent 9a59913
commit 5408301
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 1 deletion.
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -374,6 +374,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
@@ -522,7 +522,7 @@ int TLSWrap::GetFD() {
 
 
 bool TLSWrap::IsAlive() {
-  return ssl_ != nullptr && stream_->IsAlive();
+  return ssl_ != nullptr && stream_ != nullptr && stream_->IsAlive();
 }
 
 
@@ -781,6 +781,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());
@@ -911,6 +919,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
@@ -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
@@ -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');
+}));