Skip to content

Commit

Permalink
[major] Make WebSocket#addEventListener() ignore non standard events
Browse files Browse the repository at this point in the history 
Make `WebSocket.prototype.addEventListener()` a noop if the `type`
argument is not one of `'close'`, `'error'`, `'message'`, or `'open'`.
  • Loading branch information
@lpinca
lpinca committed Jul 29, 2021
1 parent 77a675c commit 9558ed1
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 58 deletions.
4 changes: 3 additions & 1 deletion doc/ws.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ handshake. This allows you to read headers from the server, for example
at most once after being added. If `true`, the listener would be
automatically removed when invoked.

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

### websocket.binaryType

Expand Down
3 changes: 2 additions & 1 deletion lib/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

module.exports = {
BINARY_TYPES: ['nodebuffer', 'arraybuffer', 'fragments'],
EMPTY_BUFFER: Buffer.alloc(0),
GUID: '258EAFA5-E914-47DA-95CA-C5AB0DC85B11',
kListener: Symbol('kListener'),
kStatusCode: Symbol('status-code'),
kWebSocket: Symbol('websocket'),
EMPTY_BUFFER: Buffer.alloc(0),
NOOP: () => {}
};
26 changes: 11 additions & 15 deletions lib/event-target.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const { kListener } = require('./constants');

/**
* Class representing an event.
*
Expand Down Expand Up @@ -126,8 +128,6 @@ const EventTarget = {
* @public
*/
addEventListener(type, listener, options) {
if (typeof listener !== 'function') return;

function onMessage(data, isBinary) {
listener.call(
this,
Expand All @@ -150,35 +150,31 @@ const EventTarget = {
const method = options && options.once ? 'once' : 'on';

if (type === 'message') {
onMessage._listener = listener;
onMessage[kListener] = listener;
this[method](type, onMessage);
} else if (type === 'close') {
onClose._listener = listener;
onClose[kListener] = listener;
this[method](type, onClose);
} else if (type === 'error') {
onError._listener = listener;
onError[kListener] = listener;
this[method](type, onError);
} else if (type === 'open') {
onOpen._listener = listener;
onOpen[kListener] = listener;
this[method](type, onOpen);
} else {
this[method](type, listener);
}
},

/**
* Remove an event listener.
*
* @param {String} type A string representing the event type to remove
* @param {Function} listener The listener to remove
* @param {Function} handler The listener to remove
* @public
*/
removeEventListener(type, listener) {
const listeners = this.listeners(type);

for (let i = 0; i < listeners.length; i++) {
if (listeners[i] === listener || listeners[i]._listener === listener) {
this.removeListener(type, listeners[i]);
removeEventListener(type, handler) {
for (const listener of this.listeners(type)) {
if (listener === handler || listener[kListener] === handler) {
this.removeListener(type, listener);
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions lib/websocket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const {
BINARY_TYPES,
EMPTY_BUFFER,
GUID,
kListener,
kStatusCode,
kWebSocket,
NOOP
Expand Down Expand Up @@ -522,22 +523,23 @@ Object.defineProperty(WebSocket.prototype, 'CLOSED', {
Object.defineProperty(WebSocket.prototype, `on${method}`, {
enumerable: true,
get() {
const listeners = this.listeners(method);
for (let i = 0; i < listeners.length; i++) {
if (listeners[i]._listener) return listeners[i]._listener;
for (const listener of this.listeners(method)) {
if (listener[kListener]) return listener[kListener];
}

return undefined;
},
set(listener) {
const listeners = this.listeners(method);
for (let i = 0; i < listeners.length; i++) {
set(handler) {
for (const listener of this.listeners(method)) {
//
// Remove only the listeners added via `addEventListener`.
//
if (listeners[i]._listener) this.removeListener(method, listeners[i]);
if (listener[kListener]) this.removeListener(method, listener);
}
this.addEventListener(method, listener);

if (typeof handler !== 'function') return;

this.addEventListener(method, handler);
}
});
});
Expand Down
60 changes: 27 additions & 33 deletions test/websocket.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const fs = require('fs');
const { URL } = require('url');

const WebSocket = require('..');
const { EMPTY_BUFFER, GUID, NOOP } = require('../lib/constants');
const { EMPTY_BUFFER, GUID, kListener, NOOP } = require('../lib/constants');

class CustomAgent extends http.Agent {
addRequest() {}
Expand Down Expand Up @@ -2157,88 +2157,82 @@ describe('WebSocket', () => {

assert.strictEqual(listeners.length, 2);
assert.strictEqual(listeners[0], NOOP);
assert.strictEqual(listeners[1]._listener, NOOP);
assert.strictEqual(listeners[1][kListener], NOOP);

ws.onclose = NOOP;

listeners = ws.listeners('close');

assert.strictEqual(listeners.length, 2);
assert.strictEqual(listeners[0], NOOP);
assert.strictEqual(listeners[1]._listener, NOOP);
assert.strictEqual(listeners[1][kListener], NOOP);
});

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

ws.addEventListener('foo', NOOP);
assert.strictEqual(ws.listeners('foo')[0], NOOP);
ws.addEventListener('foo', () => {});
assert.strictEqual(ws.listenerCount('foo'), 0);

//
// Fails silently when the `listener` is not a function.
//
ws.addEventListener('bar', {});
assert.strictEqual(ws.listeners('bar').length, 0);
});
ws.addEventListener('open', () => {
events.push('open');
assert.strictEqual(ws.listenerCount('open'), 1);
});

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

ws.addEventListener(
'foo',
'message',
() => {
assert.strictEqual(ws.listenerCount('foo'), 0);
done();
events.push('message');
assert.strictEqual(ws.listenerCount('message'), 0);
},
{ once: true }
);

assert.strictEqual(ws.listenerCount('foo'), 1);
ws.emit('foo');
assert.strictEqual(ws.listenerCount('message'), 1);

ws.emit('open');
ws.emit('message', EMPTY_BUFFER, false);

assert.deepStrictEqual(events, ['open', 'message']);
});

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

ws.addEventListener('message', NOOP);
ws.addEventListener('open', NOOP);
ws.addEventListener('foo', NOOP);

assert.strictEqual(ws.listeners('message')[0]._listener, NOOP);
assert.strictEqual(ws.listeners('open')[0]._listener, NOOP);
assert.strictEqual(ws.listeners('foo')[0], NOOP);
assert.strictEqual(ws.listeners('message')[0][kListener], NOOP);
assert.strictEqual(ws.listeners('open')[0][kListener], NOOP);

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

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

ws.removeEventListener('message', NOOP);
ws.removeEventListener('open', NOOP);
ws.removeEventListener('foo', NOOP);

assert.strictEqual(ws.listenerCount('message'), 0);
assert.strictEqual(ws.listenerCount('open'), 0);
assert.strictEqual(ws.listenerCount('foo'), 0);

ws.addEventListener('message', NOOP, { once: true });
ws.addEventListener('open', NOOP, { once: true });
ws.addEventListener('foo', NOOP, { once: true });

assert.strictEqual(ws.listeners('message')[0]._listener, NOOP);
assert.strictEqual(ws.listeners('open')[0]._listener, NOOP);
assert.strictEqual(ws.listeners('foo')[0], NOOP);
assert.strictEqual(ws.listeners('message')[0][kListener], NOOP);
assert.strictEqual(ws.listeners('open')[0][kListener], NOOP);

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

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

ws.removeEventListener('message', NOOP);
ws.removeEventListener('open', NOOP);
ws.removeEventListener('foo', NOOP);

assert.strictEqual(ws.listenerCount('message'), 0);
assert.strictEqual(ws.listenerCount('open'), 0);
assert.strictEqual(ws.listenerCount('foo'), 0);
});

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

0 comments on commit 9558ed1

Please sign in to comment.