Skip to content

Commit 1c6e372

Browse files
jBarzandrew749
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: nodejs/node#11776 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Brian White <[email protected]>
1 parent c21c63c commit 1c6e372

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
@@ -378,6 +378,12 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
378378
res = null;
379379
});
380380

381+
if (wrap) {
382+
wrap.on('close', function() {
383+
res.onStreamClose();
384+
});
385+
}
386+
381387
return res;
382388
};
383389

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

@@ -782,6 +782,14 @@ void TLSWrap::EnableSessionCallbacks(
782782
}
783783

784784

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

916925
StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev);
917926
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)