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 99749dc

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 ba91c41 commit 99749dc
Copy full SHA for 99749dc

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
@@ -391,6 +391,12 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
391391
res = null;
392392
});
393393

394+
if (wrap) {
395+
wrap.on('close', function() {
396+
res.onStreamClose();
397+
});
398+
}
399+
394400
return res;
395401
};
396402

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
@@ -523,7 +523,7 @@ int TLSWrap::GetFD() {
523523

524524

525525
bool TLSWrap::IsAlive() {
526-
return ssl_ != nullptr && stream_->IsAlive();
526+
return ssl_ != nullptr && stream_ != nullptr && stream_->IsAlive();
527527
}
528528

529529

@@ -783,6 +783,14 @@ void TLSWrap::EnableSessionCallbacks(
783783
}
784784

785785

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

917926
StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev);
918927
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
@@ -135,6 +135,7 @@ class TLSWrap : public AsyncWrap,
135135
static void EnableCertCb(
136136
const v8::FunctionCallbackInfo<v8::Value>& args);
137137
static void DestroySSL(const v8::FunctionCallbackInfo<v8::Value>& args);
138+
static void OnStreamClose(const v8::FunctionCallbackInfo<v8::Value>& args);
138139

139140
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
140141
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.