Skip to content

Commit 5408301

Browse files
jBarzitaloacasas
authored andcommitted
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Brian White <[email protected]>
1 parent 9a59913 commit 5408301

File tree

4 files changed

+60
-1
lines changed

4 files changed

+60
-1
lines changed

lib/_tls_wrap.js

+6
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,12 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
374374
res = null;
375375
});
376376

377+
if (wrap) {
378+
wrap.on('close', function() {
379+
res.onStreamClose();
380+
});
381+
}
382+
377383
return res;
378384
};
379385

src/tls_wrap.cc

+10-1
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ int TLSWrap::GetFD() {
522522

523523

524524
bool TLSWrap::IsAlive() {
525-
return ssl_ != nullptr && stream_->IsAlive();
525+
return ssl_ != nullptr && stream_ != nullptr && stream_->IsAlive();
526526
}
527527

528528

@@ -781,6 +781,14 @@ void TLSWrap::EnableSessionCallbacks(
781781
}
782782

783783

784+
void TLSWrap::OnStreamClose(const FunctionCallbackInfo<Value>& args) {
785+
TLSWrap* wrap;
786+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
787+
788+
wrap->stream_ = nullptr;
789+
}
790+
791+
784792
void TLSWrap::DestroySSL(const FunctionCallbackInfo<Value>& args) {
785793
TLSWrap* wrap;
786794
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
@@ -911,6 +919,7 @@ void TLSWrap::Initialize(Local<Object> target,
911919
env->SetProtoMethod(t, "enableSessionCallbacks", EnableSessionCallbacks);
912920
env->SetProtoMethod(t, "destroySSL", DestroySSL);
913921
env->SetProtoMethod(t, "enableCertCb", EnableCertCb);
922+
env->SetProtoMethod(t, "onStreamClose", OnStreamClose);
914923

915924
StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev);
916925
SSLWrap<TLSWrap>::AddMethods(env, t);

src/tls_wrap.h

+1
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ class TLSWrap : public AsyncWrap,
137137
static void EnableCertCb(
138138
const v8::FunctionCallbackInfo<v8::Value>& args);
139139
static void DestroySSL(const v8::FunctionCallbackInfo<v8::Value>& args);
140+
static void OnStreamClose(const v8::FunctionCallbackInfo<v8::Value>& args);
140141

141142
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
142143
static void GetServername(const v8::FunctionCallbackInfo<v8::Value>& args);
+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
5+
const tls = require('tls');
6+
const fs = require('fs');
7+
const net = require('net');
8+
9+
const key = fs.readFileSync(common.fixturesDir + '/keys/agent2-key.pem');
10+
const cert = fs.readFileSync(common.fixturesDir + '/keys/agent2-cert.pem');
11+
12+
const T = 100;
13+
14+
// tls server
15+
const tlsServer = tls.createServer({ cert, key }, (socket) => {
16+
setTimeout(() => {
17+
socket.on('error', (error) => {
18+
assert.strictEqual(error.code, 'EINVAL');
19+
tlsServer.close();
20+
netServer.close();
21+
});
22+
socket.write('bar');
23+
}, T * 2);
24+
});
25+
26+
// plain tcp server
27+
const netServer = net.createServer((socket) => {
28+
// if client wants to use tls
29+
tlsServer.emit('connection', socket);
30+
31+
socket.setTimeout(T, () => {
32+
// this breaks if TLSSocket is already managing the socket:
33+
socket.destroy();
34+
});
35+
}).listen(0, common.mustCall(function() {
36+
37+
// connect client
38+
tls.connect({
39+
host: 'localhost',
40+
port: this.address().port,
41+
rejectUnauthorized: false
42+
}).write('foo');
43+
}));

0 commit comments

Comments
 (0)