Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 1baee18

Browse filesBrowse files
jBarzMylesBorins
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 <fedor.indutny@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
1 parent 2c2a664 commit 1baee18
Copy full SHA for 1baee18

File tree

Expand file treeCollapse file tree

4 files changed

+60
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+60
-1
lines changed
Open diff view settings
Collapse file

‎lib/_tls_wrap.js‎

Copy file name to clipboardExpand all lines: lib/_tls_wrap.js
+6Lines changed: 6 additions & 0 deletions
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

Collapse file

‎src/tls_wrap.cc‎

Copy file name to clipboardExpand all lines: src/tls_wrap.cc
+10-1Lines changed: 10 additions & 1 deletion
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);
Collapse file

‎src/tls_wrap.h‎

Copy file name to clipboardExpand all lines: src/tls_wrap.h
+1Lines changed: 1 addition & 0 deletions
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);
Collapse file
+43Lines changed: 43 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.