Skip to content

Commit a421eb5

Browse files
lpincalpinca
committed
[major] Make Websocket#addEventListener() ignore non standard events
Make `Websocket.prototype.addEventListener()` a noop if the `type` argument is not one of `'close'`, `'error'`, `'message'`, or `'open'`.
1 parent 77a675c commit a421eb5

File tree

5 files changed

+53
-58
lines changed

5 files changed

+53
-58
lines changed

‎doc/ws.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,9 @@ handshake. This allows you to read headers from the server, for example
370370
at most once after being added. If `true`, the listener would be
371371
automatically removed when invoked.
372372

373-
Register an event listener emulating the `EventTarget` interface.
373+
Register an event listener emulating the `EventTarget` interface. This method
374+
does nothing if `type` is not one of `'close'`, `'error'`, `'message'`, or
375+
`'open'`.
374376

375377
### websocket.binaryType
376378

‎lib/constants.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22

33
module.exports = {
44
BINARY_TYPES: ['nodebuffer', 'arraybuffer', 'fragments'],
5+
EMPTY_BUFFER: Buffer.alloc(0),
56
GUID: '258EAFA5-E914-47DA-95CA-C5AB0DC85B11',
7+
kListener: Symbol('kListener'),
68
kStatusCode: Symbol('status-code'),
79
kWebSocket: Symbol('websocket'),
8-
EMPTY_BUFFER: Buffer.alloc(0),
910
NOOP: () => {}
1011
};

‎lib/event-target.js‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
'use strict';
22

3+
const { kListener } = require('./constants');
4+
35
/**
46
* Class representing an event.
57
*
@@ -126,8 +128,6 @@ const EventTarget = {
126128
* @public
127129
*/
128130
addEventListener(type, listener, options) {
129-
if (typeof listener !== 'function') return;
130-
131131
function onMessage(data, isBinary) {
132132
listener.call(
133133
this,
@@ -150,35 +150,31 @@ const EventTarget = {
150150
const method = options && options.once ? 'once' : 'on';
151151

152152
if (type === 'message') {
153-
onMessage._listener = listener;
153+
onMessage[kListener] = listener;
154154
this[method](type, onMessage);
155155
} else if (type === 'close') {
156-
onClose._listener = listener;
156+
onClose[kListener] = listener;
157157
this[method](type, onClose);
158158
} else if (type === 'error') {
159-
onError._listener = listener;
159+
onError[kListener] = listener;
160160
this[method](type, onError);
161161
} else if (type === 'open') {
162-
onOpen._listener = listener;
162+
onOpen[kListener] = listener;
163163
this[method](type, onOpen);
164-
} else {
165-
this[method](type, listener);
166164
}
167165
},
168166

169167
/**
170168
* Remove an event listener.
171169
*
172170
* @param {String} type A string representing the event type to remove
173-
* @param {Function} listener The listener to remove
171+
* @param {Function} handler The listener to remove
174172
* @public
175173
*/
176-
removeEventListener(type, listener) {
177-
const listeners = this.listeners(type);
178-
179-
for (let i = 0; i < listeners.length; i++) {
180-
if (listeners[i] === listener || listeners[i]._listener === listener) {
181-
this.removeListener(type, listeners[i]);
174+
removeEventListener(type, handler) {
175+
for (const listener of this.listeners(type)) {
176+
if (listener === handler || listener[kListener] === handler) {
177+
this.removeListener(type, listener);
182178
}
183179
}
184180
}

‎lib/websocket.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const {
1515
BINARY_TYPES,
1616
EMPTY_BUFFER,
1717
GUID,
18+
kListener,
1819
kStatusCode,
1920
kWebSocket,
2021
NOOP
@@ -522,22 +523,23 @@ Object.defineProperty(WebSocket.prototype, 'CLOSED', {
522523
Object.defineProperty(WebSocket.prototype, `on${method}`, {
523524
enumerable: true,
524525
get() {
525-
const listeners = this.listeners(method);
526-
for (let i = 0; i < listeners.length; i++) {
527-
if (listeners[i]._listener) return listeners[i]._listener;
526+
for (const listener of this.listeners(method)) {
527+
if (listener[kListener]) return listener[kListener];
528528
}
529529

530530
return undefined;
531531
},
532-
set(listener) {
533-
const listeners = this.listeners(method);
534-
for (let i = 0; i < listeners.length; i++) {
532+
set(handler) {
533+
for (const listener of this.listeners(method)) {
535534
//
536535
// Remove only the listeners added via `addEventListener`.
537536
//
538-
if (listeners[i]._listener) this.removeListener(method, listeners[i]);
537+
if (listener[kListener]) this.removeListener(method, listener);
539538
}
540-
this.addEventListener(method, listener);
539+
540+
if (typeof handler !== 'function') return;
541+
542+
this.addEventListener(method, handler);
541543
}
542544
});
543545
});

‎test/websocket.test.js‎

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const fs = require('fs');
1111
const { URL } = require('url');
1212

1313
const WebSocket = require('..');
14-
const { EMPTY_BUFFER, GUID, NOOP } = require('../lib/constants');
14+
const { EMPTY_BUFFER, GUID, kListener, NOOP } = require('../lib/constants');
1515

1616
class CustomAgent extends http.Agent {
1717
addRequest() {}
@@ -2157,88 +2157,82 @@ describe('WebSocket', () => {
21572157

21582158
assert.strictEqual(listeners.length, 2);
21592159
assert.strictEqual(listeners[0], NOOP);
2160-
assert.strictEqual(listeners[1]._listener, NOOP);
2160+
assert.strictEqual(listeners[1][kListener], NOOP);
21612161

21622162
ws.onclose = NOOP;
21632163

21642164
listeners = ws.listeners('close');
21652165

21662166
assert.strictEqual(listeners.length, 2);
21672167
assert.strictEqual(listeners[0], NOOP);
2168-
assert.strictEqual(listeners[1]._listener, NOOP);
2168+
assert.strictEqual(listeners[1][kListener], NOOP);
21692169
});
21702170

2171-
it('adds listeners for custom events with `addEventListener`', () => {
2171+
it('supports the `addEventListener` method', () => {
2172+
const events = [];
21722173
const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() });
21732174

2174-
ws.addEventListener('foo', NOOP);
2175-
assert.strictEqual(ws.listeners('foo')[0], NOOP);
2175+
ws.addEventListener('foo', () => {});
2176+
assert.strictEqual(ws.listenerCount('foo'), 0);
21762177

2177-
//
2178-
// Fails silently when the `listener` is not a function.
2179-
//
2180-
ws.addEventListener('bar', {});
2181-
assert.strictEqual(ws.listeners('bar').length, 0);
2182-
});
2178+
ws.addEventListener('open', () => {
2179+
events.push('open');
2180+
assert.strictEqual(ws.listenerCount('open'), 1);
2181+
});
21832182

2184-
it('allows to add one time listeners with `addEventListener`', (done) => {
2185-
const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() });
2183+
assert.strictEqual(ws.listenerCount('open'), 1);
21862184

21872185
ws.addEventListener(
2188-
'foo',
2186+
'message',
21892187
() => {
2190-
assert.strictEqual(ws.listenerCount('foo'), 0);
2191-
done();
2188+
events.push('message');
2189+
assert.strictEqual(ws.listenerCount('message'), 0);
21922190
},
21932191
{ once: true }
21942192
);
21952193

2196-
assert.strictEqual(ws.listenerCount('foo'), 1);
2197-
ws.emit('foo');
2194+
assert.strictEqual(ws.listenerCount('message'), 1);
2195+
2196+
ws.emit('open');
2197+
ws.emit('message', EMPTY_BUFFER, false);
2198+
2199+
assert.deepStrictEqual(events, ['open', 'message']);
21982200
});
21992201

22002202
it('supports the `removeEventListener` method', () => {
22012203
const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() });
22022204

22032205
ws.addEventListener('message', NOOP);
22042206
ws.addEventListener('open', NOOP);
2205-
ws.addEventListener('foo', NOOP);
22062207

2207-
assert.strictEqual(ws.listeners('message')[0]._listener, NOOP);
2208-
assert.strictEqual(ws.listeners('open')[0]._listener, NOOP);
2209-
assert.strictEqual(ws.listeners('foo')[0], NOOP);
2208+
assert.strictEqual(ws.listeners('message')[0][kListener], NOOP);
2209+
assert.strictEqual(ws.listeners('open')[0][kListener], NOOP);
22102210

22112211
ws.removeEventListener('message', () => {});
22122212

2213-
assert.strictEqual(ws.listeners('message')[0]._listener, NOOP);
2213+
assert.strictEqual(ws.listeners('message')[0][kListener], NOOP);
22142214

22152215
ws.removeEventListener('message', NOOP);
22162216
ws.removeEventListener('open', NOOP);
2217-
ws.removeEventListener('foo', NOOP);
22182217

22192218
assert.strictEqual(ws.listenerCount('message'), 0);
22202219
assert.strictEqual(ws.listenerCount('open'), 0);
2221-
assert.strictEqual(ws.listenerCount('foo'), 0);
22222220

22232221
ws.addEventListener('message', NOOP, { once: true });
22242222
ws.addEventListener('open', NOOP, { once: true });
2225-
ws.addEventListener('foo', NOOP, { once: true });
22262223

2227-
assert.strictEqual(ws.listeners('message')[0]._listener, NOOP);
2228-
assert.strictEqual(ws.listeners('open')[0]._listener, NOOP);
2229-
assert.strictEqual(ws.listeners('foo')[0], NOOP);
2224+
assert.strictEqual(ws.listeners('message')[0][kListener], NOOP);
2225+
assert.strictEqual(ws.listeners('open')[0][kListener], NOOP);
22302226

22312227
ws.removeEventListener('message', () => {});
22322228

2233-
assert.strictEqual(ws.listeners('message')[0]._listener, NOOP);
2229+
assert.strictEqual(ws.listeners('message')[0][kListener], NOOP);
22342230

22352231
ws.removeEventListener('message', NOOP);
22362232
ws.removeEventListener('open', NOOP);
2237-
ws.removeEventListener('foo', NOOP);
22382233

22392234
assert.strictEqual(ws.listenerCount('message'), 0);
22402235
assert.strictEqual(ws.listenerCount('open'), 0);
2241-
assert.strictEqual(ws.listenerCount('foo'), 0);
22422236
});
22432237

22442238
it('wraps text data in a `MessageEvent`', (done) => {

0 commit comments

Comments
 (0)