Skip to content

Commit 6a72da3

Browse files
committedAug 6, 2021
[fix] Do not rely on undocumented behavior
Use the chunk returned by `socket.read()` to handle the buffered data instead of relying on a `'data'` event emitted after the `'close'` event. Refs: nodejs/node#39639
1 parent 04e74a1 commit 6a72da3

File tree

2 files changed

+93
-6
lines changed

2 files changed

+93
-6
lines changed
 

‎lib/websocket.js

+13-4
Original file line numberDiff line numberDiff line change
@@ -1031,24 +1031,33 @@ function socketOnClose() {
10311031
const websocket = this[kWebSocket];
10321032

10331033
this.removeListener('close', socketOnClose);
1034+
this.removeListener('data', socketOnData);
10341035
this.removeListener('end', socketOnEnd);
10351036

10361037
websocket._readyState = WebSocket.CLOSING;
10371038

1039+
let chunk;
1040+
10381041
//
10391042
// The close frame might not have been received or the `'end'` event emitted,
10401043
// for example, if the socket was destroyed due to an error. Ensure that the
10411044
// `receiver` stream is closed after writing any remaining buffered data to
10421045
// it. If the readable side of the socket is in flowing mode then there is no
10431046
// buffered data as everything has been already written and `readable.read()`
10441047
// will return `null`. If instead, the socket is paused, any possible buffered
1045-
// data will be read as a single chunk and emitted synchronously in a single
1046-
// `'data'` event.
1048+
// data will be read as a single chunk.
10471049
//
1048-
websocket._socket.read();
1050+
if (
1051+
!this._readableState.endEmitted &&
1052+
!websocket._closeFrameReceived &&
1053+
!websocket._receiver._writableState.errorEmitted &&
1054+
(chunk = websocket._socket.read()) !== null
1055+
) {
1056+
websocket._receiver.write(chunk);
1057+
}
1058+
10491059
websocket._receiver.end();
10501060

1051-
this.removeListener('data', socketOnData);
10521061
this[kWebSocket] = undefined;
10531062

10541063
clearTimeout(websocket._closeTimer);

‎test/websocket.test.js

+80-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const tls = require('tls');
1010
const fs = require('fs');
1111
const { URL } = require('url');
1212

13+
const Sender = require('../lib/sender');
1314
const WebSocket = require('..');
1415
const {
1516
CloseEvent,
@@ -2942,15 +2943,21 @@ describe('WebSocket', () => {
29422943
});
29432944
});
29442945

2945-
it('consumes all received data when connection is closed abnormally', (done) => {
2946+
it('consumes all received data when connection is closed (1/2)', (done) => {
29462947
const wss = new WebSocket.Server(
29472948
{
29482949
perMessageDeflate: { threshold: 0 },
29492950
port: 0
29502951
},
29512952
() => {
2952-
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
29532953
const messages = [];
2954+
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
2955+
2956+
ws.on('open', () => {
2957+
ws._socket.on('close', () => {
2958+
assert.strictEqual(ws._receiver._state, 5);
2959+
});
2960+
});
29542961

29552962
ws.on('message', (message, isBinary) => {
29562963
assert.ok(!isBinary);
@@ -2973,6 +2980,77 @@ describe('WebSocket', () => {
29732980
});
29742981
});
29752982

2983+
it('consumes all received data when connection is closed (2/2)', (done) => {
2984+
const payload1 = Buffer.alloc(15 * 1024);
2985+
const payload2 = Buffer.alloc(1);
2986+
2987+
const opts = {
2988+
fin: true,
2989+
opcode: 0x02,
2990+
mask: false,
2991+
readOnly: false
2992+
};
2993+
2994+
const list = [
2995+
...Sender.frame(payload1, { rsv1: false, ...opts }),
2996+
...Sender.frame(payload2, { rsv1: true, ...opts })
2997+
];
2998+
2999+
for (let i = 0; i < 399; i++) {
3000+
list.push(list[list.length - 2], list[list.length - 1]);
3001+
}
3002+
3003+
const data = Buffer.concat(list);
3004+
3005+
const wss = new WebSocket.Server(
3006+
{
3007+
perMessageDeflate: true,
3008+
port: 0
3009+
},
3010+
() => {
3011+
const messageLengths = [];
3012+
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
3013+
3014+
ws.on('open', () => {
3015+
ws._socket.prependListener('close', () => {
3016+
assert.strictEqual(ws._receiver._state, 5);
3017+
assert.strictEqual(ws._socket._readableState.length, 3);
3018+
});
3019+
3020+
const push = ws._socket.push;
3021+
3022+
ws._socket.push = (data) => {
3023+
ws._socket.push = push;
3024+
ws._socket.push(data);
3025+
ws.terminate();
3026+
};
3027+
3028+
// This hack is used because there is no guarantee that more than
3029+
// 16 KiB will be sent as a single TCP packet.
3030+
push.call(ws._socket, data);
3031+
3032+
wss.clients
3033+
.values()
3034+
.next()
3035+
.value.send(payload2, { compress: false });
3036+
});
3037+
3038+
ws.on('message', (message, isBinary) => {
3039+
assert.ok(isBinary);
3040+
messageLengths.push(message.length);
3041+
});
3042+
3043+
ws.on('close', (code) => {
3044+
assert.strictEqual(code, 1006);
3045+
assert.strictEqual(messageLengths.length, 402);
3046+
assert.strictEqual(messageLengths[0], 15360);
3047+
assert.strictEqual(messageLengths[messageLengths.length - 1], 1);
3048+
wss.close(done);
3049+
});
3050+
}
3051+
);
3052+
});
3053+
29763054
it('handles a close frame received while compressing data', (done) => {
29773055
const wss = new WebSocket.Server(
29783056
{

0 commit comments

Comments
 (0)
Please sign in to comment.