Skip to content

Commit

Permalink
tls: keep track of stream that is closed
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
  • Loading branch information
jBarz authored and MylesBorins committed Apr 19, 2017
1 parent ba91c41 commit 99749dc
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 1 deletion.
6 changes: 6 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
res = null;
});

if (wrap) {
wrap.on('close', function() {
res.onStreamClose();
});
}

return res;
};

Expand Down
11 changes: 10 additions & 1 deletion src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ int TLSWrap::GetFD() {


bool TLSWrap::IsAlive() {
return ssl_ != nullptr && stream_->IsAlive();
return ssl_ != nullptr && stream_ != nullptr && stream_->IsAlive();
}


Expand Down Expand Up @@ -783,6 +783,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());
Expand Down Expand Up @@ -913,6 +921,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);
Expand Down
1 change: 1 addition & 0 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,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);
Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-tls-socket-close.js
Original file line number Diff line number Diff line change
@@ -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');
}));

0 comments on commit 99749dc

Please sign in to comment.