Skip to content

Commit e173423

Browse files
committedJul 14, 2021
[major] Do not decode Buffers to strings
Avoid decoding text messages and close reasons to strings. Pass them as `Buffer`s to the listeners of their respective events. Also, make listeners of the `'message'` event take a boolean argument to speficy whether or not the message is binary. Refs: #1878 Refs: #1804
1 parent ebea038 commit e173423

15 files changed

+326
-209
lines changed
 

‎README.md

+6-6
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ ws.on('open', function open() {
149149
ws.send('something');
150150
});
151151

152-
ws.on('message', function incoming(data) {
153-
console.log(data);
152+
ws.on('message', function incoming(message) {
153+
console.log('received: %s', message);
154154
});
155155
```
156156

@@ -296,10 +296,10 @@ const WebSocket = require('ws');
296296
const wss = new WebSocket.Server({ port: 8080 });
297297

298298
wss.on('connection', function connection(ws) {
299-
ws.on('message', function incoming(data) {
299+
ws.on('message', function incoming(data, isBinary) {
300300
wss.clients.forEach(function each(client) {
301301
if (client.readyState === WebSocket.OPEN) {
302-
client.send(data);
302+
client.send(data, { binary: isBinary });
303303
}
304304
});
305305
});
@@ -315,10 +315,10 @@ const WebSocket = require('ws');
315315
const wss = new WebSocket.Server({ port: 8080 });
316316

317317
wss.on('connection', function connection(ws) {
318-
ws.on('message', function incoming(data) {
318+
ws.on('message', function incoming(data, isBinary) {
319319
wss.clients.forEach(function each(client) {
320320
if (client !== ws && client.readyState === WebSocket.OPEN) {
321-
client.send(data);
321+
client.send(data, { binary: isBinary });
322322
}
323323
});
324324
});

‎bench/speed.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ if (cluster.isMaster) {
1919
});
2020

2121
wss.on('connection', (ws) => {
22-
ws.on('message', (data) => ws.send(data));
22+
ws.on('message', (data, isBinary) => {
23+
ws.send(data, { binary: isBinary });
24+
});
2325
});
2426

2527
server.listen(path ? { path } : { port }, () => cluster.fork());

‎doc/ws.md

+8-6
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,12 @@ it defaults to `/`.
300300
### Event: 'close'
301301

302302
- `code` {Number}
303-
- `reason` {String}
303+
- `reason` {Buffer}
304304

305305
Emitted when the connection is closed. `code` is a numeric value indicating the
306306
status code explaining why the connection has been closed. `reason` is a
307-
human-readable string explaining why the connection has been closed.
307+
`Buffer` containing a human-readable string explaining why the connection has
308+
been closed.
308309

309310
### Event: 'error'
310311

@@ -315,9 +316,11 @@ of the string values defined below under [WS Error Codes](#ws-error-codes).
315316

316317
### Event: 'message'
317318

318-
- `data` {String|Buffer|ArrayBuffer|Buffer[]}
319+
- `data` {Buffer|ArrayBuffer|Buffer[]}
320+
- `isBinary` {Boolean}
319321

320-
Emitted when a message is received from the server.
322+
Emitted when a message is received. `data` is the message content. `isBinary`
323+
specifies whether the message is binary or not.
321324

322325
### Event: 'open'
323326

@@ -389,8 +392,7 @@ following ways:
389392

390393
- `code` {Number} A numeric value indicating the status code explaining why the
391394
connection is being closed.
392-
- `reason` {String} A human-readable string explaining why the connection is
393-
closing.
395+
- `reason` {String|Buffer} The reason why the connection is closing.
394396

395397
Initiate a closing handshake.
396398

‎examples/ssl.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const wss = new WebSocket.Server({ server });
1414

1515
wss.on('connection', function connection(ws) {
1616
ws.on('message', function message(msg) {
17-
console.log(msg);
17+
console.log(msg.toString());
1818
});
1919
});
2020

‎lib/event-target.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,15 @@ const EventTarget = {
128128
addEventListener(type, listener, options) {
129129
if (typeof listener !== 'function') return;
130130

131-
function onMessage(data) {
132-
listener.call(this, new MessageEvent(data, this));
131+
function onMessage(data, isBinary) {
132+
listener.call(
133+
this,
134+
new MessageEvent(isBinary ? data : data.toString(), this)
135+
);
133136
}
134137

135138
function onClose(code, message) {
136-
listener.call(this, new CloseEvent(code, message, this));
139+
listener.call(this, new CloseEvent(code, message.toString(), this));
137140
}
138141

139142
function onError(error) {

‎lib/receiver.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ class Receiver extends Writable {
499499
data = fragments;
500500
}
501501

502-
this.emit('message', data);
502+
this.emit('message', data, true);
503503
} else {
504504
const buf = concat(fragments, messageLength);
505505

@@ -514,7 +514,7 @@ class Receiver extends Writable {
514514
);
515515
}
516516

517-
this.emit('message', buf.toString());
517+
this.emit('message', buf, false);
518518
}
519519
}
520520

@@ -533,7 +533,7 @@ class Receiver extends Writable {
533533
this._loop = false;
534534

535535
if (data.length === 0) {
536-
this.emit('conclude', 1005, '');
536+
this.emit('conclude', 1005, EMPTY_BUFFER);
537537
this.end();
538538
} else if (data.length === 1) {
539539
return error(
@@ -568,7 +568,7 @@ class Receiver extends Writable {
568568
);
569569
}
570570

571-
this.emit('conclude', code, buf.toString());
571+
this.emit('conclude', code, buf);
572572
this.end();
573573
}
574574
} else if (this._opcode === 0x09) {

‎lib/sender.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class Sender {
102102
* Sends a close message to the other peer.
103103
*
104104
* @param {Number} [code] The status code component of the body
105-
* @param {String} [data] The message component of the body
105+
* @param {(String|Buffer)} [data] The message component of the body
106106
* @param {Boolean} [mask=false] Specifies whether or not to mask the message
107107
* @param {Function} [cb] Callback
108108
* @public
@@ -114,7 +114,7 @@ class Sender {
114114
buf = EMPTY_BUFFER;
115115
} else if (typeof code !== 'number' || !isValidStatusCode(code)) {
116116
throw new TypeError('First argument must be a valid error code number');
117-
} else if (data === undefined || data === '') {
117+
} else if (data === undefined || !data.length) {
118118
buf = Buffer.allocUnsafe(2);
119119
buf.writeUInt16BE(code, 0);
120120
} else {
@@ -126,7 +126,12 @@ class Sender {
126126

127127
buf = Buffer.allocUnsafe(2 + length);
128128
buf.writeUInt16BE(code, 0);
129-
buf.write(data, 2);
129+
130+
if (typeof data === 'string') {
131+
buf.write(data, 2);
132+
} else {
133+
buf.set(data, 2);
134+
}
130135
}
131136

132137
if (this._deflating) {

‎lib/stream.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,11 @@ function createWebSocketStream(ws, options) {
7272
writableObjectMode: false
7373
});
7474

75-
ws.on('message', function message(msg) {
76-
if (!duplex.push(msg)) {
75+
ws.on('message', function message(msg, isBinary) {
76+
const data =
77+
!isBinary && duplex._readableState.objectMode ? msg.toString() : msg;
78+
79+
if (!duplex.push(data)) {
7780
resumeOnReceiverDrain = false;
7881
ws._socket.pause();
7982
}

‎lib/websocket.js

+8-6
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class WebSocket extends EventEmitter {
4848
this._closeCode = 1006;
4949
this._closeFrameReceived = false;
5050
this._closeFrameSent = false;
51-
this._closeMessage = '';
51+
this._closeMessage = EMPTY_BUFFER;
5252
this._closeTimer = null;
5353
this._extensions = {};
5454
this._protocol = '';
@@ -264,7 +264,8 @@ class WebSocket extends EventEmitter {
264264
* +---+
265265
*
266266
* @param {Number} [code] Status code explaining why the connection is closing
267-
* @param {String} [data] A string explaining why the connection is closing
267+
* @param {(String|Buffer)} [data] The reason why the connection is
268+
* closing
268269
* @public
269270
*/
270271
close(code, data) {
@@ -941,7 +942,7 @@ function sendAfterClose(websocket, data, cb) {
941942
* The listener of the `Receiver` `'conclude'` event.
942943
*
943944
* @param {Number} code The status code
944-
* @param {String} reason The reason for closing
945+
* @param {Buffer} reason The reason for closing
945946
* @private
946947
*/
947948
function receiverOnConclude(code, reason) {
@@ -995,11 +996,12 @@ function receiverOnFinish() {
995996
/**
996997
* The listener of the `Receiver` `'message'` event.
997998
*
998-
* @param {(String|Buffer|ArrayBuffer|Buffer[])} data The message
999+
* @param {Buffer|ArrayBuffer|Buffer[])} data The message
1000+
* @param {Boolean} isBinary Specifies whether the message is binary or not
9991001
* @private
10001002
*/
1001-
function receiverOnMessage(data) {
1002-
this[kWebSocket].emit('message', data);
1003+
function receiverOnMessage(data, isBinary) {
1004+
this[kWebSocket].emit('message', data, isBinary);
10031005
}
10041006

10051007
/**

‎test/autobahn-server.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ const wss = new WebSocket.Server({ port }, () => {
1010
});
1111

1212
wss.on('connection', (ws) => {
13-
ws.on('message', (data) => ws.send(data));
13+
ws.on('message', (data, isBinary) => {
14+
ws.send(data, { binary: isBinary });
15+
});
1416
ws.on('error', (e) => console.error(e));
1517
});

‎test/autobahn.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ function nextTest() {
1818
ws = new WebSocket(
1919
`ws://localhost:9001/runCase?case=${currentTest}&agent=ws`
2020
);
21-
ws.on('message', (data) => ws.send(data));
21+
ws.on('message', (data, isBinary) => {
22+
ws.send(data, { binary: isBinary });
23+
});
2224
ws.on('close', () => {
2325
currentTest++;
2426
process.nextTick(nextTest);

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

+34-4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const { randomBytes } = require('crypto');
99
const createWebSocketStream = require('../lib/stream');
1010
const Sender = require('../lib/sender');
1111
const WebSocket = require('..');
12+
const { EMPTY_BUFFER } = require('../lib/constants');
1213

1314
describe('createWebSocketStream', () => {
1415
it('is exposed as a property of the `WebSocket` class', () => {
@@ -58,11 +59,12 @@ describe('createWebSocketStream', () => {
5859
});
5960

6061
wss.on('connection', (ws) => {
61-
ws.on('message', (message) => {
62+
ws.on('message', (message, isBinary) => {
6263
ws.on('close', (code, reason) => {
63-
assert.ok(message.equals(chunk));
64+
assert.deepStrictEqual(message, chunk);
65+
assert.ok(isBinary);
6466
assert.strictEqual(code, 1005);
65-
assert.strictEqual(reason, '');
67+
assert.strictEqual(reason, EMPTY_BUFFER);
6668
wss.close(done);
6769
});
6870
});
@@ -229,7 +231,7 @@ describe('createWebSocketStream', () => {
229231
ws._socket.write(Buffer.from([0x85, 0x00]));
230232
ws.on('close', (code, reason) => {
231233
assert.strictEqual(code, 1002);
232-
assert.strictEqual(reason, '');
234+
assert.deepStrictEqual(reason, EMPTY_BUFFER);
233235

234236
serverClientCloseEventEmitted = true;
235237
if (duplexCloseEventEmitted) wss.close(done);
@@ -538,5 +540,33 @@ describe('createWebSocketStream', () => {
538540
});
539541
});
540542
});
543+
544+
it('converts text messages to strings in readable object mode', (done) => {
545+
const wss = new WebSocket.Server({ port: 0 }, () => {
546+
const events = [];
547+
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
548+
const duplex = createWebSocketStream(ws, { readableObjectMode: true });
549+
550+
duplex.on('data', (data) => {
551+
events.push('data');
552+
assert.strictEqual(data, 'foo');
553+
});
554+
555+
duplex.on('end', () => {
556+
events.push('end');
557+
duplex.end();
558+
});
559+
560+
duplex.on('close', () => {
561+
assert.deepStrictEqual(events, ['data', 'end']);
562+
wss.close(done);
563+
});
564+
});
565+
566+
wss.on('connection', (ws) => {
567+
ws.send('foo');
568+
ws.close();
569+
});
570+
});
541571
});
542572
});

‎test/receiver.test.js

+103-83
Large diffs are not rendered by default.

‎test/websocket-server.test.js

+6-4
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,9 @@ describe('WebSocketServer', () => {
411411

412412
const ws = new WebSocket(`ws://localhost:${server.address().port}`);
413413

414-
ws.on('message', (message) => {
415-
assert.strictEqual(message, 'hello');
414+
ws.on('message', (message, isBinary) => {
415+
assert.deepStrictEqual(message, Buffer.from('hello'));
416+
assert.ok(!isBinary);
416417
wss.close();
417418
server.close(done);
418419
});
@@ -932,8 +933,9 @@ describe('WebSocketServer', () => {
932933
});
933934

934935
wss.on('connection', (ws) => {
935-
ws.on('message', (data) => {
936-
assert.strictEqual(data, 'Hello');
936+
ws.on('message', (data, isBinary) => {
937+
assert.deepStrictEqual(data, Buffer.from('Hello'));
938+
assert.ok(!isBinary);
937939
wss.close(done);
938940
});
939941
});

‎test/websocket.test.js

+128-84
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)
Please sign in to comment.