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 (nodejs#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: nodejs#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]>
eliphazbouye · Jun 20, 2024 · 2996ada · 2996ada
1 parent 227f0f6
commit 2996ada
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 120 deletions.
diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml
@@ -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

diff --git a/lib/internal/bootstrap/web/exposed-window-or-worker.js b/lib/internal/bootstrap/web/exposed-window-or-worker.js
@@ -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']);
@@ -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/

diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js
@@ -20,7 +20,6 @@ const {
 } = primordials;
 
 const {
-  kEmptyObject,
   kEnumerableProperty,
   setOwnProperty,
 } = require('internal/util');
@@ -38,7 +37,6 @@ const {
   moveMessagePortToContext,
   receiveMessageOnPort: receiveMessageOnPort_,
   stopMessagePort,
-  checkMessagePort,
   DOMException,
 } = internalBinding('messaging');
 const {
@@ -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 =
@@ -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
@@ -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,
@@ -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,
@@ -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) {
@@ -546,7 +454,6 @@ module.exports = {
   moveMessagePortToContext,
   MessagePort,
   MessageChannel,
-  MessageEvent,
   receiveMessageOnPort,
   setupPortReferencing,
   ReadableWorkerStdio,

diff --git a/test/parallel/test-messageevent-brandcheck.js b/test/parallel/test-messageevent-brandcheck.js
@@ -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);
 });
diff --git a/test/parallel/test-worker-message-event.js b/test/parallel/test-worker-message-event.js
@@ -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/,
   });
 }