throw on non iterable

Rename distinguishingArg to distinguishingTransferArg in postMessage
function and add a null check in StructuredSerializeOptions to throw a
TypeError when options.transfer is null, enforcing iterable requirement.

src/bun.js/bindings/webcore/JSEventEmitter.cpp

@@ -37,6 +37,7 @@
 #include <wtf/GetPtr.h>
 #include <wtf/PointerPreparations.h>
 #include <wtf/URL.h>
+#include "JSEventTarget.h"
 
 namespace WebCore {
 using namespace JSC;
@@ -224,7 +225,7 @@ void JSEventEmitter::finishCreation(VM& vm)
 
 JSObject* JSEventEmitter::createPrototype(VM& vm, JSDOMGlobalObject& globalObject)
 {
-    auto* structure = JSEventEmitterPrototype::createStructure(vm, &globalObject, globalObject.objectPrototype());
+    auto* structure = JSEventEmitterPrototype::createStructure(vm, &globalObject, JSEventTarget::prototype(vm, globalObject));
     structure->setMayBePrototype(true);
     return JSEventEmitterPrototype::create(vm, &globalObject, structure);
 }

src/bun.js/bindings/webcore/JSMessagePort.cpp

@@ -53,6 +53,7 @@
 #include <wtf/GetPtr.h>
 #include <wtf/PointerPreparations.h>
 #include <wtf/URL.h>
+#include "JSEventEmitter.h"
 
 namespace WebCore {
 using namespace JSC;
@@ -158,7 +159,7 @@ JSMessagePort::JSMessagePort(Structure* structure, JSDOMGlobalObject& globalObje
 
 JSObject* JSMessagePort::createPrototype(VM& vm, JSDOMGlobalObject& globalObject)
 {
-    auto* structure = JSMessagePortPrototype::createStructure(vm, &globalObject, JSEventTarget::prototype(vm, globalObject));
+    auto* structure = JSMessagePortPrototype::createStructure(vm, &globalObject, JSEventEmitter::prototype(vm, globalObject));
     structure->setMayBePrototype(true);
     return JSMessagePortPrototype::create(vm, &globalObject, structure);
 }
@@ -298,6 +299,17 @@ static inline JSC::EncodedJSValue jsMessagePortPrototypeFunction_postMessageOver
         if (distinguishingArg.isObject())
             RELEASE_AND_RETURN(throwScope, (jsMessagePortPrototypeFunction_postMessage2Body(lexicalGlobalObject, callFrame, castedThis)));
     }
+
+    if (argsCount >= 2) {
+        JSValue distinguishingTransferArg = callFrame->uncheckedArgument(1);
+        bool hasIterator = hasIteratorMethod(lexicalGlobalObject, distinguishingTransferArg);
+        RETURN_IF_EXCEPTION(throwScope, {});
+        if (!distinguishingTransferArg.isUndefinedOrNull() && !distinguishingTransferArg.isObject() && !hasIterator) {
+            return Bun::throwError(lexicalGlobalObject, throwScope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE,
+                "Optional transferList argument must be an iterable"_s);
+        }
+    }
+
     return argsCount < 1 ? throwVMError(lexicalGlobalObject, throwScope, createNotEnoughArgumentsError(lexicalGlobalObject)) : throwVMTypeError(lexicalGlobalObject, throwScope);
 }
 

src/bun.js/bindings/webcore/JSStructuredSerializeOptions.cpp

@@ -25,6 +25,8 @@
 #include "JSDOMConvertSequences.h"
 #include <JavaScriptCore/JSArray.h>
 #include <JavaScriptCore/JSCInlines.h>
+#include <JavaScriptCore/IteratorOperations.h>
+#include "../ErrorCode.h"
 
 namespace WebCore {
 using namespace JSC;
@@ -48,6 +50,20 @@ template<> StructuredSerializeOptions convertDictionary<StructuredSerializeOptio
         RETURN_IF_EXCEPTION(throwScope, {});
     }
     if (!transferValue.isUndefined()) {
+        if (transferValue.isNull()) {
+            Bun::throwError(&lexicalGlobalObject, throwScope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "Optional options.transfer argument must be an iterable"_s);
+            return {};
+        }
+
+        if (!transferValue.isObject()) {
+            bool hasIter = hasIteratorMethod(&lexicalGlobalObject, transferValue);
+            RETURN_IF_EXCEPTION(throwScope, {});
+            if (!hasIter) {
+                Bun::throwError(&lexicalGlobalObject, throwScope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "Optional options.transfer argument must be an iterable"_s);
+                return {};
+            }
+        }
+
         result.transfer = convert<IDLSequence<IDLObject>>(lexicalGlobalObject, transferValue);
         RETURN_IF_EXCEPTION(throwScope, {});
     } else

src/js/node/worker_threads.ts

@@ -38,82 +38,7 @@ type NodeWorkerOptions = import("node:worker_threads").WorkerOptions;
 // after their Worker exits
 let urlRevokeRegistry: FinalizationRegistry<string> | undefined = undefined;
 
-function injectFakeEmitter(Class) {
-  function messageEventHandler(event: MessageEvent) {
-    return event.data;
-  }
-
-  function errorEventHandler(event: ErrorEvent) {
-    return event.error;
-  }
-
-  const wrappedListener = Symbol("wrappedListener");
-
-  function wrapped(run, listener) {
-    const callback = function (event) {
-      return listener(run(event));
-    };
-    listener[wrappedListener] = callback;
-    return callback;
-  }
-
-  function functionForEventType(event, listener) {
-    switch (event) {
-      case "error":
-      case "messageerror": {
-        return wrapped(errorEventHandler, listener);
-      }
-
-      default: {
-        return wrapped(messageEventHandler, listener);
-      }
-    }
-  }
-
-  Class.prototype.on = function (event, listener) {
-    this.addEventListener(event, functionForEventType(event, listener));
-
-    return this;
-  };
-
-  Class.prototype.off = function (event, listener) {
-    if (listener) {
-      this.removeEventListener(event, listener[wrappedListener] || listener);
-    } else {
-      this.removeEventListener(event);
-    }
-
-    return this;
-  };
-
-  Class.prototype.once = function (event, listener) {
-    this.addEventListener(event, functionForEventType(event, listener), { once: true });
-
-    return this;
-  };
-
-  function EventClass(eventName) {
-    if (eventName === "error" || eventName === "messageerror") {
-      return ErrorEvent;
-    }
-
-    return MessageEvent;
-  }
-
-  Class.prototype.emit = function (event, ...args) {
-    this.dispatchEvent(new (EventClass(event))(event, ...args));
-
-    return this;
-  };
-
-  Class.prototype.prependListener = Class.prototype.on;
-  Class.prototype.prependOnceListener = Class.prototype.once;
-}
-
-const _MessagePort = globalThis.MessagePort;
-injectFakeEmitter(_MessagePort);
-
-const MessagePort = _MessagePort;
+const MessagePort = globalThis.MessagePort;
 
 let resourceLimits = {};
 
@@ -150,8 +75,8 @@ function fakeParentPort() {
 
   const postMessage = $newCppFunction("ZigGlobalObject.cpp", "jsFunctionPostMessage", 1);
   Object.defineProperty(fake, "postMessage", {
-    value(...args: [any, any]) {
-      return postMessage.$apply(null, args);
+    value() {
+      return postMessage.$apply(null, arguments);
     },
   });
 

test/js/node/test/parallel/test-worker-message-port.js

@@ -0,0 +1,185 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+const { MessageChannel, MessagePort } = require('worker_threads');
+
+{
+  const { port1, port2 } = new MessageChannel();
+  assert(port1 instanceof MessagePort);
+  assert(port2 instanceof MessagePort);
+
+  const input = { a: 1 };
+  port1.postMessage(input);
+  port2.on('message', common.mustCall((received) => {
+    assert.deepStrictEqual(received, input);
+    port2.close(common.mustCall());
+  }));
+}
+{
+  // Test emitting non-message events on a port
+  const { port2 } = new MessageChannel();
+  port2.addEventListener('foo', common.mustCall((received) => {
+    assert.strictEqual(received.type, 'foo');
+    assert.strictEqual(received.detail, 'bar');
+  }));
+  port2.on('foo', common.mustCall((received) => {
+    assert.strictEqual(received, 'bar');
+  }));
+  port2.emit('foo', 'bar');
+}
+{
+  const { port1, port2 } = new MessageChannel();
+
+  port1.onmessage = common.mustCall((message) => {
+    assert.strictEqual(message.data, 4);
+    assert.strictEqual(message.target, port1);
+    assert.deepStrictEqual(message.ports, []);
+    port2.close(common.mustCall());
+  });
+
+  port1.postMessage(2);
+
+  port2.onmessage = common.mustCall((message) => {
+    port2.postMessage(message.data * 2);
+  });
+}
+
+{
+  const { port1, port2 } = new MessageChannel();
+
+  const input = { a: 1 };
+  port1.postMessage(input);
+  // Check that the message still gets delivered if `port2` has its
+  // `on('message')` handler attached at a later point in time.
+  setImmediate(() => {
+    port2.on('message', common.mustCall((received) => {
+      assert.deepStrictEqual(received, input);
+      port2.close(common.mustCall());
+    }));
+  });
+}
+
+{
+  const { port1, port2 } = new MessageChannel();
+
+  const input = { a: 1 };
+
+  const dummy = common.mustNotCall();
+  // Check that the message still gets delivered if `port2` has its
+  // `on('message')` handler attached at a later point in time, even if a
+  // listener was removed previously.
+  port2.addListener('message', dummy);
+  setImmediate(() => {
+    port2.removeListener('message', dummy);
+    port1.postMessage(input);
+    setImmediate(() => {
+      port2.on('message', common.mustCall((received) => {
+        assert.deepStrictEqual(received, input);
+        port2.close(common.mustCall());
+      }));
+    });
+  });
+}
+
+{
+  const { port1, port2 } = new MessageChannel();
+  port2.on('message', common.mustCall(6));
+  port1.postMessage(1, null);
+  port1.postMessage(2, undefined);
+  port1.postMessage(3, []);
+  port1.postMessage(4, {});
+  port1.postMessage(5, { transfer: undefined });
+  port1.postMessage(6, { transfer: [] });
+
+  const err = {
+    constructor: TypeError,
+    code: 'ERR_INVALID_ARG_TYPE',
+    message: 'Optional transferList argument must be an iterable'
+  };
+
+  assert.throws(() => port1.postMessage(5, 0), err);
+  assert.throws(() => port1.postMessage(5, false), err);
+  assert.throws(() => port1.postMessage(5, 'X'), err);
+  assert.throws(() => port1.postMessage(5, Symbol('X')), err);
+
+  const err2 = {
+    constructor: TypeError,
+    code: 'ERR_INVALID_ARG_TYPE',
+    message: 'Optional options.transfer argument must be an iterable'
+  };
+
+  assert.throws(() => port1.postMessage(5, { transfer: null }), err2);
+  assert.throws(() => port1.postMessage(5, { transfer: 0 }), err2);
+  assert.throws(() => port1.postMessage(5, { transfer: false }), err2);
+  assert.throws(() => port1.postMessage(5, { transfer: {} }), err2);
+  assert.throws(() => port1.postMessage(5, {
+    transfer: { [Symbol.iterator]() { return {}; } }
+  }), err2);
+  assert.throws(() => port1.postMessage(5, {
+    transfer: { [Symbol.iterator]() { return { next: 42 }; } }
+  }), err2);
+  assert.throws(() => port1.postMessage(5, {
+    transfer: { [Symbol.iterator]() { return { next: null }; } }
+  }), err2);
+  port1.close();
+}
+
+{
+  // Make sure these ArrayBuffers end up detached, i.e. are actually being
+  // transferred because the transfer list provides them.
+  const { port1, port2 } = new MessageChannel();
+  port2.on('message', common.mustCall((msg) => {
+    assert.strictEqual(msg.ab.byteLength, 10);
+  }, 4));
+
+  {
+    const ab = new ArrayBuffer(10);
+    port1.postMessage({ ab }, [ ab ]);
+    assert.strictEqual(ab.byteLength, 0);
+  }
+
+  {
+    const ab = new ArrayBuffer(10);
+    port1.postMessage({ ab }, { transfer: [ ab ] });
+    assert.strictEqual(ab.byteLength, 0);
+  }
+
+  {
+    const ab = new ArrayBuffer(10);
+    port1.postMessage({ ab }, (function*() { yield ab; })());
+    assert.strictEqual(ab.byteLength, 0);
+  }
+
+  {
+    const ab = new ArrayBuffer(10);
+    port1.postMessage({ ab }, {
+      transfer: (function*() { yield ab; })()
+    });
+    assert.strictEqual(ab.byteLength, 0);
+  }
+
+  port1.close();
+}
+
+{
+  // Test MessageEvent#ports
+  const c1 = new MessageChannel();
+  const c2 = new MessageChannel();
+  c1.port1.postMessage({ port: c2.port2 }, [ c2.port2 ]);
+  c1.port2.addEventListener('message', common.mustCall((ev) => {
+    assert.strictEqual(ev.ports.length, 1);
+    assert.strictEqual(ev.ports[0].constructor, MessagePort);
+    c1.port1.close();
+    c2.port1.close();
+  }));
+}
+
+{
+  assert.deepStrictEqual(
+    Object.getOwnPropertyNames(MessagePort.prototype).sort(),
+    [
+      'close', 'constructor', 'hasRef', 'onmessage', 'onmessageerror',
+      'postMessage', 'ref', 'start', 'unref',
+    ]);
+}