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 9aec3e6

Browse filesBrowse files
oyydMylesBorins
authored andcommitted
tls: close StreamWrap and its stream correctly
When sockets of the "net" module destroyed, they will call `this._handle.close()` which will also emit EOF if not emitted before. This feature makes sockets on the other side emit "end" and "close" even though we haven't called `end()`. As `stream` of `StreamWrap` are likely to be instances of `net.Socket`, calling `destroy()` manually will avoid issues that don't properly close wrapped connections. Fixes: #14605 PR-URL: #23654 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 874371d commit 9aec3e6
Copy full SHA for 9aec3e6

File tree

Expand file treeCollapse file tree

3 files changed

+201
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+201
-0
lines changed
Open diff view settings
Collapse file

‎lib/internal/wrap_js_stream.js‎

Copy file name to clipboardExpand all lines: lib/internal/wrap_js_stream.js
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ class JSStreamWrap extends Socket {
6868
if (this._handle)
6969
this._handle.emitEOF();
7070
});
71+
// Some `Stream` don't pass `hasError` parameters when closed.
72+
stream.once('close', () => {
73+
// Errors emitted from `stream` have also been emitted to this instance
74+
// so that we don't pass errors to `destroy()` again.
75+
this.destroy();
76+
});
7177

7278
super({ handle, manualStart: true });
7379
this.stream = stream;
@@ -188,6 +194,14 @@ class JSStreamWrap extends Socket {
188194
doClose(cb) {
189195
const handle = this._handle;
190196

197+
// When sockets of the "net" module destroyed, they will call
198+
// `this._handle.close()` which will also emit EOF if not emitted before.
199+
// This feature makes sockets on the other side emit "end" and "close"
200+
// even though we haven't called `end()`. As `stream` are likely to be
201+
// instances of `net.Socket`, calling `stream.destroy()` manually will
202+
// avoid issues that don't properly close wrapped connections.
203+
this.stream.destroy();
204+
191205
setImmediate(() => {
192206
// Should be already set by net.js
193207
assert.strictEqual(this._handle, null);
Collapse file
+69Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto) common.skip('missing crypto');
5+
6+
const fixtures = require('../common/fixtures');
7+
const makeDuplexPair = require('../common/duplexpair');
8+
const net = require('net');
9+
const assert = require('assert');
10+
const tls = require('tls');
11+
12+
// This test ensures that an instance of StreamWrap should emit "end" and
13+
// "close" when the socket on the other side call `destroy()` instead of
14+
// `end()`.
15+
// Refs: https://github.com/nodejs/node/issues/14605
16+
const CONTENT = 'Hello World';
17+
const tlsServer = tls.createServer(
18+
{
19+
key: fixtures.readSync('test_key.pem'),
20+
cert: fixtures.readSync('test_cert.pem'),
21+
ca: [fixtures.readSync('test_ca.pem')],
22+
},
23+
(socket) => {
24+
socket.on('error', common.mustNotCall());
25+
socket.on('close', common.mustCall());
26+
socket.write(CONTENT);
27+
socket.destroy();
28+
},
29+
);
30+
31+
const server = net.createServer((conn) => {
32+
conn.on('error', common.mustNotCall());
33+
// Assume that we want to use data to determine what to do with connections.
34+
conn.once('data', common.mustCall((chunk) => {
35+
const { clientSide, serverSide } = makeDuplexPair();
36+
serverSide.on('close', common.mustCall(() => {
37+
conn.destroy();
38+
}));
39+
clientSide.pipe(conn);
40+
conn.pipe(clientSide);
41+
42+
conn.on('close', common.mustCall(() => {
43+
clientSide.destroy();
44+
}));
45+
clientSide.on('close', common.mustCall(() => {
46+
conn.destroy();
47+
}));
48+
49+
process.nextTick(() => {
50+
conn.unshift(chunk);
51+
});
52+
53+
tlsServer.emit('connection', serverSide);
54+
}));
55+
});
56+
57+
server.listen(0, () => {
58+
const port = server.address().port;
59+
const conn = tls.connect({ port, rejectUnauthorized: false }, () => {
60+
conn.on('data', common.mustCall((data) => {
61+
assert.strictEqual(data.toString('utf8'), CONTENT);
62+
}));
63+
conn.on('error', common.mustNotCall());
64+
conn.on(
65+
'close',
66+
common.mustCall(() => server.close()),
67+
);
68+
});
69+
});
Collapse file
+118Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const StreamWrap = require('_stream_wrap');
5+
const net = require('net');
6+
7+
// This test ensures that when we directly call `socket.destroy()` without
8+
// having called `socket.end()` on an instance of streamWrap, it will
9+
// still emit EOF which makes the socket on the other side emit "end" and
10+
// "close" events, and vice versa.
11+
{
12+
let port;
13+
const server = net.createServer((socket) => {
14+
socket.on('error', common.mustNotCall());
15+
socket.on('end', common.mustNotCall());
16+
socket.on('close', common.mustCall());
17+
socket.destroy();
18+
});
19+
20+
server.listen(() => {
21+
port = server.address().port;
22+
createSocket();
23+
});
24+
25+
function createSocket() {
26+
let streamWrap;
27+
const socket = new net.connect({
28+
port,
29+
}, () => {
30+
socket.on('error', common.mustNotCall());
31+
socket.on('end', common.mustCall());
32+
socket.on('close', common.mustCall());
33+
34+
streamWrap.on('error', common.mustNotCall());
35+
// The "end" events will be emitted which is as same as
36+
// the same situation for an instance of `net.Socket` without
37+
// `StreamWrap`.
38+
streamWrap.on('end', common.mustCall());
39+
// Destroying a socket in the server side should emit EOF and cause
40+
// the corresponding client-side socket closed.
41+
streamWrap.on('close', common.mustCall(() => {
42+
server.close();
43+
}));
44+
});
45+
streamWrap = new StreamWrap(socket);
46+
}
47+
}
48+
49+
// Destroy the streamWrap and test again.
50+
{
51+
let port;
52+
const server = net.createServer((socket) => {
53+
socket.on('error', common.mustNotCall());
54+
socket.on('end', common.mustCall());
55+
socket.on('close', common.mustCall(() => {
56+
server.close();
57+
}));
58+
// Do not `socket.end()` and directly `socket.destroy()`.
59+
});
60+
61+
server.listen(() => {
62+
port = server.address().port;
63+
createSocket();
64+
});
65+
66+
function createSocket() {
67+
let streamWrap;
68+
const socket = new net.connect({
69+
port,
70+
}, () => {
71+
socket.on('error', common.mustNotCall());
72+
socket.on('end', common.mustNotCall());
73+
socket.on('close', common.mustCall());
74+
75+
streamWrap.on('error', common.mustNotCall());
76+
streamWrap.on('end', common.mustNotCall());
77+
// Destroying a socket in the server side should emit EOF and cause
78+
// the corresponding client-side socket closed.
79+
streamWrap.on('close', common.mustCall());
80+
streamWrap.destroy();
81+
});
82+
streamWrap = new StreamWrap(socket);
83+
}
84+
}
85+
86+
// Destroy the client socket and test again.
87+
{
88+
let port;
89+
const server = net.createServer((socket) => {
90+
socket.on('error', common.mustNotCall());
91+
socket.on('end', common.mustCall());
92+
socket.on('close', common.mustCall(() => {
93+
server.close();
94+
}));
95+
});
96+
97+
server.listen(() => {
98+
port = server.address().port;
99+
createSocket();
100+
});
101+
102+
function createSocket() {
103+
let streamWrap;
104+
const socket = new net.connect({
105+
port,
106+
}, () => {
107+
socket.on('error', common.mustNotCall());
108+
socket.on('end', common.mustNotCall());
109+
socket.on('close', common.mustCall());
110+
111+
streamWrap.on('error', common.mustNotCall());
112+
streamWrap.on('end', common.mustNotCall());
113+
streamWrap.on('close', common.mustCall());
114+
socket.destroy();
115+
});
116+
streamWrap = new StreamWrap(socket);
117+
}
118+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.