Skip to content

Commit 9535702

Browse files
committedJan 25, 2020
[fix] Make _final() a noop if no socket is assigned
Prevent `_final()` from throwing an error if `duplex.end()` is called while the websocket is closing and no socket is assigned to it.
1 parent 1863504 commit 9535702

File tree

2 files changed

+64
-1
lines changed

2 files changed

+64
-1
lines changed
 

‎lib/stream.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,20 @@ function createWebSocketStream(ws, options) {
109109
return;
110110
}
111111

112+
// If the value of the `_socket` property is `null` it means that `ws` is a
113+
// client websocket and the handshake failed. In fact, when this happens, a
114+
// socket is never assigned to the websocket. Wait for the `'error'` event
115+
// that will be emitted by the websocket.
116+
if (ws._socket === null) return;
117+
112118
if (ws._socket._writableState.finished) {
113119
if (duplex._readableState.endEmitted) duplex.destroy();
114120
callback();
115121
} else {
116122
ws._socket.once('finish', function finish() {
117123
// `duplex` is not destroyed here because the `'end'` event will be
118124
// emitted on `duplex` after this `'finish'` event. The EOF signaling
119-
// `null` chunk is, in fact, pushed when the WebSocket emits `'close'`.
125+
// `null` chunk is, in fact, pushed when the websocket emits `'close'`.
120126
callback();
121127
});
122128
ws.close();

‎test/create-websocket-stream.test.js

+57
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const assert = require('assert');
44
const EventEmitter = require('events');
5+
const { createServer } = require('http');
56
const { Duplex } = require('stream');
67
const { randomBytes } = require('crypto');
78

@@ -145,6 +146,62 @@ describe('createWebSocketStream', () => {
145146
});
146147
});
147148

149+
it('makes `_final()` a noop if no socket is assigned', (done) => {
150+
const server = createServer();
151+
152+
server.on('upgrade', (request, socket) => {
153+
socket.on('end', socket.end);
154+
155+
const headers = [
156+
'HTTP/1.1 101 Switching Protocols',
157+
'Upgrade: websocket',
158+
'Connection: Upgrade',
159+
'Sec-WebSocket-Accept: foo'
160+
];
161+
162+
socket.write(headers.concat('\r\n').join('\r\n'));
163+
});
164+
165+
server.listen(() => {
166+
const called = [];
167+
const ws = new WebSocket(`ws://localhost:${server.address().port}`);
168+
const duplex = WebSocket.createWebSocketStream(ws);
169+
const final = duplex._final;
170+
171+
duplex._final = (callback) => {
172+
called.push('final');
173+
assert.strictEqual(ws.readyState, WebSocket.CLOSING);
174+
assert.strictEqual(ws._socket, null);
175+
176+
final(callback);
177+
};
178+
179+
duplex.on('error', (err) => {
180+
called.push('error');
181+
assert.ok(err instanceof Error);
182+
assert.strictEqual(
183+
err.message,
184+
'Invalid Sec-WebSocket-Accept header'
185+
);
186+
});
187+
188+
duplex.on('finish', () => {
189+
called.push('finish');
190+
});
191+
192+
duplex.on('close', () => {
193+
assert.deepStrictEqual(called, ['final', 'error']);
194+
server.close(done);
195+
});
196+
197+
ws.on('upgrade', () => {
198+
process.nextTick(() => {
199+
duplex.end();
200+
});
201+
});
202+
});
203+
});
204+
148205
it('reemits errors', (done) => {
149206
const wss = new WebSocket.Server({ port: 0 }, () => {
150207
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);

0 commit comments

Comments
 (0)
Please sign in to comment.