Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls: call destroy() on StreamWrap and their streams #23654

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/internal/wrap_js_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ class JSStreamWrap extends Socket {
if (this._handle)
this._handle.emitEOF();
});
// Some `Stream` don't pass `hasError` parameters when closed.
stream.once('close', () => {
// Errors emitted from `stream` have also been emitted to this instance
// so that we don't pass errors to `destroy()` again.
this.destroy();
});

super({ handle, manualStart: true });
this.stream = stream;
Expand Down Expand Up @@ -188,6 +194,14 @@ class JSStreamWrap extends Socket {
doClose(cb) {
const handle = this._handle;

// 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` are likely to be
// instances of `net.Socket`, calling `stream.destroy()` manually will
// avoid issues that don't properly close wrapped connections.
this.stream.destroy();

setImmediate(() => {
// Should be already set by net.js
assert.strictEqual(this._handle, null);
Expand Down
69 changes: 69 additions & 0 deletions test/parallel/test-tls-destroy-stream.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto) common.skip('missing crypto');

const fixtures = require('../common/fixtures');
const makeDuplexPair = require('../common/duplexpair');
const net = require('net');
const assert = require('assert');
const tls = require('tls');

// This test ensures that an instance of StreamWrap should emit "end" and
// "close" when the socket on the other side call `destroy()` instead of
// `end()`.
// Refs: https://github.com/nodejs/node/issues/14605
const CONTENT = 'Hello World';
const tlsServer = tls.createServer(
{
key: fixtures.readSync('test_key.pem'),
cert: fixtures.readSync('test_cert.pem'),
ca: [fixtures.readSync('test_ca.pem')],
},
(socket) => {
socket.on('error', common.mustNotCall());
socket.on('close', common.mustCall());
socket.write(CONTENT);
socket.destroy();
},
);

const server = net.createServer((conn) => {
conn.on('error', common.mustNotCall());
// Assume that we want to use data to determine what to do with connections.
conn.once('data', common.mustCall((chunk) => {
const { clientSide, serverSide } = makeDuplexPair();
serverSide.on('close', common.mustCall(() => {
conn.destroy();
}));
clientSide.pipe(conn);
conn.pipe(clientSide);

conn.on('close', common.mustCall(() => {
clientSide.destroy();
}));
clientSide.on('close', common.mustCall(() => {
conn.destroy();
}));

process.nextTick(() => {
conn.unshift(chunk);
});

tlsServer.emit('connection', serverSide);
}));
});

server.listen(0, () => {
const port = server.address().port;
const conn = tls.connect({ port, rejectUnauthorized: false }, () => {
conn.on('data', common.mustCall((data) => {
assert.strictEqual(data.toString('utf8'), CONTENT);
}));
conn.on('error', common.mustNotCall());
conn.on(
'close',
common.mustCall(() => server.close()),
);
});
});
118 changes: 118 additions & 0 deletions test/parallel/test-wrap-js-stream-destroy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
'use strict';

const common = require('../common');
const StreamWrap = require('_stream_wrap');
const net = require('net');

// This test ensures that when we directly call `socket.destroy()` without
// having called `socket.end()` on an instance of streamWrap, it will
// still emit EOF which makes the socket on the other side emit "end" and
// "close" events, and vice versa.
{
let port;
const server = net.createServer((socket) => {
socket.on('error', common.mustNotCall());
socket.on('end', common.mustNotCall());
socket.on('close', common.mustCall());
socket.destroy();
});

server.listen(() => {
port = server.address().port;
createSocket();
});

function createSocket() {
let streamWrap;
const socket = new net.connect({
port,
}, () => {
socket.on('error', common.mustNotCall());
socket.on('end', common.mustCall());
socket.on('close', common.mustCall());

streamWrap.on('error', common.mustNotCall());
// The "end" events will be emitted which is as same as
// the same situation for an instance of `net.Socket` without
// `StreamWrap`.
streamWrap.on('end', common.mustCall());
// Destroying a socket in the server side should emit EOF and cause
// the corresponding client-side socket closed.
streamWrap.on('close', common.mustCall(() => {
server.close();
}));
});
streamWrap = new StreamWrap(socket);
}
}

// Destroy the streamWrap and test again.
{
let port;
const server = net.createServer((socket) => {
socket.on('error', common.mustNotCall());
socket.on('end', common.mustCall());
socket.on('close', common.mustCall(() => {
server.close();
}));
// Do not `socket.end()` and directly `socket.destroy()`.
});

server.listen(() => {
port = server.address().port;
createSocket();
});

function createSocket() {
let streamWrap;
const socket = new net.connect({
port,
}, () => {
socket.on('error', common.mustNotCall());
socket.on('end', common.mustNotCall());
socket.on('close', common.mustCall());

streamWrap.on('error', common.mustNotCall());
streamWrap.on('end', common.mustNotCall());
// Destroying a socket in the server side should emit EOF and cause
// the corresponding client-side socket closed.
streamWrap.on('close', common.mustCall());
streamWrap.destroy();
});
streamWrap = new StreamWrap(socket);
}
}

// Destroy the client socket and test again.
{
let port;
const server = net.createServer((socket) => {
socket.on('error', common.mustNotCall());
socket.on('end', common.mustCall());
socket.on('close', common.mustCall(() => {
server.close();
}));
});

server.listen(() => {
port = server.address().port;
createSocket();
});

function createSocket() {
let streamWrap;
const socket = new net.connect({
port,
}, () => {
socket.on('error', common.mustNotCall());
socket.on('end', common.mustNotCall());
socket.on('close', common.mustCall());

streamWrap.on('error', common.mustNotCall());
streamWrap.on('end', common.mustNotCall());
streamWrap.on('close', common.mustCall());
socket.destroy();
});
streamWrap = new StreamWrap(socket);
}
}