Skip to content

Commit

Permalink
lib: replace MessageEvent with undici's
Browse files Browse the repository at this point in the history
undici's MessageEvent is better tested and has a complete WebIDL
implementation for validation. Not only this, but it's also used in
Node's  current WebSocket implementation. There are a large number of
webidl-related issues in the current MessageEvent, such as not
implementing `MessageEvent.prototype.initMessageEvent`, not validating
arguments passed to its constructor
(#51771), not validating the values
passed to the constructor (such as not validating that `ports` is a
sequence, not converting origin to a USVString, etc.), and other issues.

fixup

PR-URL: #52370
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
  • Loading branch information
KhafraDev authored Apr 29, 2024
1 parent 7c3dce0 commit 3136fb0
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 120 deletions.
2 changes: 1 addition & 1 deletion lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ rules:
- name: MessageChannel
message: Use `const { MessageChannel } = require('internal/worker/io');` instead of the global.
- name: MessageEvent
message: Use `const { MessageEvent } = require('internal/worker/io');` instead of the global.
message: Use `const { MessageEvent } = require('internal/deps/undici/undici');` instead of the global.
- name: MessagePort
message: Use `const { MessagePort } = require('internal/worker/io');` instead of the global.
- name: Navigator
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/web/exposed-window-or-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']);
// https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts
exposeLazyInterfaces(globalThis, 'internal/worker/io', ['BroadcastChannel']);
exposeLazyInterfaces(globalThis, 'internal/worker/io', [
'MessageChannel', 'MessagePort', 'MessageEvent',
'MessageChannel', 'MessagePort',
]);
// https://www.w3.org/TR/FileAPI/#dfn-Blob
exposeLazyInterfaces(globalThis, 'internal/blob', ['Blob']);
Expand Down Expand Up @@ -84,7 +84,7 @@ ObjectDefineProperty(globalThis, 'fetch', {
// https://fetch.spec.whatwg.org/#request-class
// https://fetch.spec.whatwg.org/#response-class
exposeLazyInterfaces(globalThis, 'internal/deps/undici/undici', [
'FormData', 'Headers', 'Request', 'Response',
'FormData', 'Headers', 'Request', 'Response', 'MessageEvent',
]);

// https://websockets.spec.whatwg.org/
Expand Down
107 changes: 7 additions & 100 deletions lib/internal/worker/io.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const {
} = primordials;

const {
kEmptyObject,
kEnumerableProperty,
setOwnProperty,
} = require('internal/util');
Expand All @@ -38,7 +37,6 @@ const {
moveMessagePortToContext,
receiveMessageOnPort: receiveMessageOnPort_,
stopMessagePort,
checkMessagePort,
DOMException,
} = internalBinding('messaging');
const {
Expand All @@ -59,25 +57,19 @@ const {
const { inspect } = require('internal/util/inspect');
const {
codes: {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_THIS,
ERR_MISSING_ARGS,
},
} = require('internal/errors');

const kData = Symbol('kData');
const kHandle = Symbol('kHandle');
const kIncrementsPortRef = Symbol('kIncrementsPortRef');
const kLastEventId = Symbol('kLastEventId');
const kName = Symbol('kName');
const kOrigin = Symbol('kOrigin');
const kOnMessage = Symbol('kOnMessage');
const kOnMessageError = Symbol('kOnMessageError');
const kPort = Symbol('kPort');
const kPorts = Symbol('kPorts');
const kWaitingStreams = Symbol('kWaitingStreams');
const kWritableCallbacks = Symbol('kWritableCallbacks');
const kSource = Symbol('kSource');
const kStartedReading = Symbol('kStartedReading');
const kStdioWantsMoreDataCallback = Symbol('kStdioWantsMoreDataCallback');
const kCurrentlyReceivingPorts =
Expand All @@ -93,6 +85,11 @@ const messageTypes = {
LOAD_SCRIPT: 'loadScript',
};

let messageEvent;
function lazyMessageEvent() {
return messageEvent ??= require('internal/deps/undici/undici').MessageEvent;
}

// We have to mess with the MessagePort prototype a bit, so that a) we can make
// it inherit from NodeEventTarget, even though it is a C++ class, and b) we do
// not provide methods that are not present in the Browser and not documented
Expand All @@ -119,95 +116,6 @@ MessagePort.prototype.hasRef = function() {
return !!FunctionPrototypeCall(MessagePortPrototype.hasRef, this);
};

function validateMessagePort(port, name) {
if (!checkMessagePort(port))
throw new ERR_INVALID_ARG_TYPE(name, 'MessagePort', port);
}

function isMessageEvent(value) {
return value != null && kData in value;
}

class MessageEvent extends Event {
constructor(type, {
data = null,
origin = '',
lastEventId = '',
source = null,
ports = [],
} = kEmptyObject) {
super(type);
this[kData] = data;
this[kOrigin] = `${origin}`;
this[kLastEventId] = `${lastEventId}`;
this[kSource] = source;
this[kPorts] = [...ports];

if (this[kSource] !== null)
validateMessagePort(this[kSource], 'init.source');
for (let i = 0; i < this[kPorts].length; i++)
validateMessagePort(this[kPorts][i], `init.ports[${i}]`);
}
}

ObjectDefineProperties(MessageEvent.prototype, {
data: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kData];
},
enumerable: true,
configurable: true,
set: undefined,
},
origin: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kOrigin];
},
enumerable: true,
configurable: true,
set: undefined,
},
lastEventId: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kLastEventId];
},
enumerable: true,
configurable: true,
set: undefined,
},
source: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kSource];
},
enumerable: true,
configurable: true,
set: undefined,
},
ports: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kPorts];
},
enumerable: true,
configurable: true,
set: undefined,
},
});

const originalCreateEvent = EventTarget.prototype[kCreateEvent];
ObjectDefineProperty(
MessagePort.prototype,
Expand All @@ -220,7 +128,7 @@ ObjectDefineProperty(
}
const ports = this[kCurrentlyReceivingPorts];
this[kCurrentlyReceivingPorts] = undefined;
return new MessageEvent(type, { data, ports });
return new (lazyMessageEvent())(type, { data, ports });
},
configurable: false,
writable: false,
Expand Down Expand Up @@ -413,7 +321,7 @@ function receiveMessageOnPort(port) {
}

function onMessageEvent(type, data) {
this.dispatchEvent(new MessageEvent(type, { data }));
this.dispatchEvent(new (lazyMessageEvent())(type, { data }));
}

function isBroadcastChannel(value) {
Expand Down Expand Up @@ -546,7 +454,6 @@ module.exports = {
moveMessagePortToContext,
MessagePort,
MessageChannel,
MessageEvent,
receiveMessageOnPort,
setupPortReferencing,
ReadableWorkerStdio,
Expand Down
9 changes: 1 addition & 8 deletions test/parallel/test-messageevent-brandcheck.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
// Flags: --expose-internals
'use strict';

require('../common');
const assert = require('assert');

const {
MessageEvent,
} = require('internal/worker/io');

[
'data',
'origin',
'lastEventId',
'source',
'ports',
].forEach((i) => {
assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), {
code: 'ERR_INVALID_THIS',
});
assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), TypeError);
});
18 changes: 9 additions & 9 deletions test/parallel/test-worker-message-event.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,31 +61,31 @@ const dummyPort = new MessageChannel().port1;
assert.throws(() => {
new MessageEvent('message', { source: 1 });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.source" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected 1 to be an instance of MessagePort/,
});
assert.throws(() => {
new MessageEvent('message', { source: {} });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.source" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected {} to be an instance of MessagePort/,
});
assert.throws(() => {
new MessageEvent('message', { ports: 0 });
}, {
message: /ports is not iterable/,
message: /Sequence: Value of type Number is not an Object/,
});
assert.throws(() => {
new MessageEvent('message', { ports: [ null ] });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.ports\[0\]" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected null to be an instance of MessagePort/,
});
assert.throws(() => {
new MessageEvent('message', { ports: [ {} ] });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.ports\[0\]" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected {} to be an instance of MessagePort/,
});
}

Expand Down

0 comments on commit 3136fb0

Please sign in to comment.