Skip to content

Commit 6413769

Browse files
KhafraDevtargos
authored andcommitted
lib: replace MessageEvent with undici's
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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
1 parent af60fbb commit 6413769

File tree

5 files changed

+20
-120
lines changed

5 files changed

+20
-120
lines changed

lib/.eslintrc.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ rules:
6767
- name: MessageChannel
6868
message: Use `const { MessageChannel } = require('internal/worker/io');` instead of the global.
6969
- name: MessageEvent
70-
message: Use `const { MessageEvent } = require('internal/worker/io');` instead of the global.
70+
message: Use `const { MessageEvent } = require('internal/deps/undici/undici');` instead of the global.
7171
- name: MessagePort
7272
message: Use `const { MessagePort } = require('internal/worker/io');` instead of the global.
7373
- name: Navigator

lib/internal/bootstrap/web/exposed-window-or-worker.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']);
3939
// https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts
4040
exposeLazyInterfaces(globalThis, 'internal/worker/io', ['BroadcastChannel']);
4141
exposeLazyInterfaces(globalThis, 'internal/worker/io', [
42-
'MessageChannel', 'MessagePort', 'MessageEvent',
42+
'MessageChannel', 'MessagePort',
4343
]);
4444
// https://www.w3.org/TR/FileAPI/#dfn-Blob
4545
exposeLazyInterfaces(globalThis, 'internal/blob', ['Blob']);
@@ -78,7 +78,7 @@ ObjectDefineProperty(globalThis, 'fetch', {
7878
// https://fetch.spec.whatwg.org/#request-class
7979
// https://fetch.spec.whatwg.org/#response-class
8080
exposeLazyInterfaces(globalThis, 'internal/deps/undici/undici', [
81-
'FormData', 'Headers', 'Request', 'Response',
81+
'FormData', 'Headers', 'Request', 'Response', 'MessageEvent',
8282
]);
8383

8484
// https://websockets.spec.whatwg.org/

lib/internal/worker/io.js

+7-100
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ const {
2020
} = primordials;
2121

2222
const {
23-
kEmptyObject,
2423
kEnumerableProperty,
2524
setOwnProperty,
2625
} = require('internal/util');
@@ -38,7 +37,6 @@ const {
3837
moveMessagePortToContext,
3938
receiveMessageOnPort: receiveMessageOnPort_,
4039
stopMessagePort,
41-
checkMessagePort,
4240
DOMException,
4341
} = internalBinding('messaging');
4442
const {
@@ -59,25 +57,19 @@ const {
5957
const { inspect } = require('internal/util/inspect');
6058
const {
6159
codes: {
62-
ERR_INVALID_ARG_TYPE,
6360
ERR_INVALID_THIS,
6461
ERR_MISSING_ARGS,
6562
},
6663
} = require('internal/errors');
6764

68-
const kData = Symbol('kData');
6965
const kHandle = Symbol('kHandle');
7066
const kIncrementsPortRef = Symbol('kIncrementsPortRef');
71-
const kLastEventId = Symbol('kLastEventId');
7267
const kName = Symbol('kName');
73-
const kOrigin = Symbol('kOrigin');
7468
const kOnMessage = Symbol('kOnMessage');
7569
const kOnMessageError = Symbol('kOnMessageError');
7670
const kPort = Symbol('kPort');
77-
const kPorts = Symbol('kPorts');
7871
const kWaitingStreams = Symbol('kWaitingStreams');
7972
const kWritableCallbacks = Symbol('kWritableCallbacks');
80-
const kSource = Symbol('kSource');
8173
const kStartedReading = Symbol('kStartedReading');
8274
const kStdioWantsMoreDataCallback = Symbol('kStdioWantsMoreDataCallback');
8375
const kCurrentlyReceivingPorts =
@@ -93,6 +85,11 @@ const messageTypes = {
9385
LOAD_SCRIPT: 'loadScript',
9486
};
9587

88+
let messageEvent;
89+
function lazyMessageEvent() {
90+
return messageEvent ??= require('internal/deps/undici/undici').MessageEvent;
91+
}
92+
9693
// We have to mess with the MessagePort prototype a bit, so that a) we can make
9794
// it inherit from NodeEventTarget, even though it is a C++ class, and b) we do
9895
// not provide methods that are not present in the Browser and not documented
@@ -119,95 +116,6 @@ MessagePort.prototype.hasRef = function() {
119116
return !!FunctionPrototypeCall(MessagePortPrototype.hasRef, this);
120117
};
121118

122-
function validateMessagePort(port, name) {
123-
if (!checkMessagePort(port))
124-
throw new ERR_INVALID_ARG_TYPE(name, 'MessagePort', port);
125-
}
126-
127-
function isMessageEvent(value) {
128-
return value != null && kData in value;
129-
}
130-
131-
class MessageEvent extends Event {
132-
constructor(type, {
133-
data = null,
134-
origin = '',
135-
lastEventId = '',
136-
source = null,
137-
ports = [],
138-
} = kEmptyObject) {
139-
super(type);
140-
this[kData] = data;
141-
this[kOrigin] = `${origin}`;
142-
this[kLastEventId] = `${lastEventId}`;
143-
this[kSource] = source;
144-
this[kPorts] = [...ports];
145-
146-
if (this[kSource] !== null)
147-
validateMessagePort(this[kSource], 'init.source');
148-
for (let i = 0; i < this[kPorts].length; i++)
149-
validateMessagePort(this[kPorts][i], `init.ports[${i}]`);
150-
}
151-
}
152-
153-
ObjectDefineProperties(MessageEvent.prototype, {
154-
data: {
155-
__proto__: null,
156-
get() {
157-
if (!isMessageEvent(this))
158-
throw new ERR_INVALID_THIS('MessageEvent');
159-
return this[kData];
160-
},
161-
enumerable: true,
162-
configurable: true,
163-
set: undefined,
164-
},
165-
origin: {
166-
__proto__: null,
167-
get() {
168-
if (!isMessageEvent(this))
169-
throw new ERR_INVALID_THIS('MessageEvent');
170-
return this[kOrigin];
171-
},
172-
enumerable: true,
173-
configurable: true,
174-
set: undefined,
175-
},
176-
lastEventId: {
177-
__proto__: null,
178-
get() {
179-
if (!isMessageEvent(this))
180-
throw new ERR_INVALID_THIS('MessageEvent');
181-
return this[kLastEventId];
182-
},
183-
enumerable: true,
184-
configurable: true,
185-
set: undefined,
186-
},
187-
source: {
188-
__proto__: null,
189-
get() {
190-
if (!isMessageEvent(this))
191-
throw new ERR_INVALID_THIS('MessageEvent');
192-
return this[kSource];
193-
},
194-
enumerable: true,
195-
configurable: true,
196-
set: undefined,
197-
},
198-
ports: {
199-
__proto__: null,
200-
get() {
201-
if (!isMessageEvent(this))
202-
throw new ERR_INVALID_THIS('MessageEvent');
203-
return this[kPorts];
204-
},
205-
enumerable: true,
206-
configurable: true,
207-
set: undefined,
208-
},
209-
});
210-
211119
const originalCreateEvent = EventTarget.prototype[kCreateEvent];
212120
ObjectDefineProperty(
213121
MessagePort.prototype,
@@ -220,7 +128,7 @@ ObjectDefineProperty(
220128
}
221129
const ports = this[kCurrentlyReceivingPorts];
222130
this[kCurrentlyReceivingPorts] = undefined;
223-
return new MessageEvent(type, { data, ports });
131+
return new (lazyMessageEvent())(type, { data, ports });
224132
},
225133
configurable: false,
226134
writable: false,
@@ -413,7 +321,7 @@ function receiveMessageOnPort(port) {
413321
}
414322

415323
function onMessageEvent(type, data) {
416-
this.dispatchEvent(new MessageEvent(type, { data }));
324+
this.dispatchEvent(new (lazyMessageEvent())(type, { data }));
417325
}
418326

419327
function isBroadcastChannel(value) {
@@ -546,7 +454,6 @@ module.exports = {
546454
moveMessagePortToContext,
547455
MessagePort,
548456
MessageChannel,
549-
MessageEvent,
550457
receiveMessageOnPort,
551458
setupPortReferencing,
552459
ReadableWorkerStdio,
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,14 @@
1-
// Flags: --expose-internals
21
'use strict';
32

43
require('../common');
54
const assert = require('assert');
65

7-
const {
8-
MessageEvent,
9-
} = require('internal/worker/io');
10-
116
[
127
'data',
138
'origin',
149
'lastEventId',
1510
'source',
1611
'ports',
1712
].forEach((i) => {
18-
assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), {
19-
code: 'ERR_INVALID_THIS',
20-
});
13+
assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), TypeError);
2114
});

test/parallel/test-worker-message-event.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -61,31 +61,31 @@ const dummyPort = new MessageChannel().port1;
6161
assert.throws(() => {
6262
new MessageEvent('message', { source: 1 });
6363
}, {
64-
code: 'ERR_INVALID_ARG_TYPE',
65-
message: /The "init\.source" property must be an instance of MessagePort/,
64+
name: 'TypeError',
65+
message: /Expected 1 to be an instance of MessagePort/,
6666
});
6767
assert.throws(() => {
6868
new MessageEvent('message', { source: {} });
6969
}, {
70-
code: 'ERR_INVALID_ARG_TYPE',
71-
message: /The "init\.source" property must be an instance of MessagePort/,
70+
name: 'TypeError',
71+
message: /Expected {} to be an instance of MessagePort/,
7272
});
7373
assert.throws(() => {
7474
new MessageEvent('message', { ports: 0 });
7575
}, {
76-
message: /ports is not iterable/,
76+
message: /Sequence: Value of type Number is not an Object/,
7777
});
7878
assert.throws(() => {
7979
new MessageEvent('message', { ports: [ null ] });
8080
}, {
81-
code: 'ERR_INVALID_ARG_TYPE',
82-
message: /The "init\.ports\[0\]" property must be an instance of MessagePort/,
81+
name: 'TypeError',
82+
message: /Expected null to be an instance of MessagePort/,
8383
});
8484
assert.throws(() => {
8585
new MessageEvent('message', { ports: [ {} ] });
8686
}, {
87-
code: 'ERR_INVALID_ARG_TYPE',
88-
message: /The "init\.ports\[0\]" property must be an instance of MessagePort/,
87+
name: 'TypeError',
88+
message: /Expected {} to be an instance of MessagePort/,
8989
});
9090
}
9191

0 commit comments

Comments
 (0)