Skip to content

Commit

Permalink
[fix] Make _final() a noop if no socket is assigned
Browse files Browse the repository at this point in the history
Prevent `_final()` from throwing an error if `duplex.end()` is called
while the websocket is closing and no socket is assigned to it.
  • Loading branch information
lpinca committed Jan 25, 2020
1 parent 1863504 commit 9535702
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,20 @@ function createWebSocketStream(ws, options) {
return;
}

// If the value of the `_socket` property is `null` it means that `ws` is a
// client websocket and the handshake failed. In fact, when this happens, a
// socket is never assigned to the websocket. Wait for the `'error'` event
// that will be emitted by the websocket.
if (ws._socket === null) return;

if (ws._socket._writableState.finished) {
if (duplex._readableState.endEmitted) duplex.destroy();
callback();
} else {
ws._socket.once('finish', function finish() {
// `duplex` is not destroyed here because the `'end'` event will be
// emitted on `duplex` after this `'finish'` event. The EOF signaling
// `null` chunk is, in fact, pushed when the WebSocket emits `'close'`.
// `null` chunk is, in fact, pushed when the websocket emits `'close'`.
callback();
});
ws.close();
Expand Down
57 changes: 57 additions & 0 deletions test/create-websocket-stream.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const assert = require('assert');
const EventEmitter = require('events');
const { createServer } = require('http');
const { Duplex } = require('stream');
const { randomBytes } = require('crypto');

Expand Down Expand Up @@ -145,6 +146,62 @@ describe('createWebSocketStream', () => {
});
});

it('makes `_final()` a noop if no socket is assigned', (done) => {
const server = createServer();

server.on('upgrade', (request, socket) => {
socket.on('end', socket.end);

const headers = [
'HTTP/1.1 101 Switching Protocols',
'Upgrade: websocket',
'Connection: Upgrade',
'Sec-WebSocket-Accept: foo'
];

socket.write(headers.concat('\r\n').join('\r\n'));
});

server.listen(() => {
const called = [];
const ws = new WebSocket(`ws://localhost:${server.address().port}`);
const duplex = WebSocket.createWebSocketStream(ws);
const final = duplex._final;

duplex._final = (callback) => {
called.push('final');
assert.strictEqual(ws.readyState, WebSocket.CLOSING);
assert.strictEqual(ws._socket, null);

final(callback);
};

duplex.on('error', (err) => {
called.push('error');
assert.ok(err instanceof Error);
assert.strictEqual(
err.message,
'Invalid Sec-WebSocket-Accept header'
);
});

duplex.on('finish', () => {
called.push('finish');
});

duplex.on('close', () => {
assert.deepStrictEqual(called, ['final', 'error']);
server.close(done);
});

ws.on('upgrade', () => {
process.nextTick(() => {
duplex.end();
});
});
});
});

it('reemits errors', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
Expand Down

0 comments on commit 9535702

Please sign in to comment.