From 4f0680541e3a88776745a483359bb89f8dd382ab Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Tue, 6 Oct 2026 14:50:44 +0300 Subject: [PATCH] fix(runtime): deliver a posted undefined as undefined, not null Every delivery path built its MessageEvent through the public constructor, whose init dictionary turns an undefined data into null. Delivery now goes through an internal factory that stores the payload as given. The native messageerror paths pass null so that event keeps its default data, and a worker's messageerror carries the deserialization failure as its data, as a port's already does. --- .../app/tests/messaging/describeDataWorker.js | 10 ++ .../messaging/parentPortDescribeWorker.js | 4 + .../main/assets/app/tests/testMessaging.js | 92 +++++++++++++++++++ test-app/runtime/src/main/cpp/Messaging.cpp | 8 +- .../runtime/src/main/cpp/WorkerEvents.cpp | 11 ++- .../src/main/cpp/js/broadcast-channel.js | 12 +-- .../src/main/cpp/js/message-channel.js | 13 ++- .../runtime/src/main/cpp/js/message-event.js | 19 +++- .../src/main/cpp/js/node-worker-threads.js | 12 +-- .../runtime/src/main/cpp/js/worker-events.js | 13 ++- 10 files changed, 164 insertions(+), 30 deletions(-) create mode 100644 test-app/app/src/main/assets/app/tests/messaging/describeDataWorker.js create mode 100644 test-app/app/src/main/assets/app/tests/messaging/parentPortDescribeWorker.js diff --git a/test-app/app/src/main/assets/app/tests/messaging/describeDataWorker.js b/test-app/app/src/main/assets/app/tests/messaging/describeDataWorker.js new file mode 100644 index 000000000..0486a1b5b --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/messaging/describeDataWorker.js @@ -0,0 +1,10 @@ +// Reports what `event.data` was before echoing it, so the parent can tell a +// value lost on the way in from one lost on the way back. +function describe(value) { + return value === undefined ? "undefined" : value === null ? "null" : typeof value; +} + +onmessage = function (event) { + postMessage({ received: describe(event.data) }); + postMessage(event.data); +}; diff --git a/test-app/app/src/main/assets/app/tests/messaging/parentPortDescribeWorker.js b/test-app/app/src/main/assets/app/tests/messaging/parentPortDescribeWorker.js new file mode 100644 index 000000000..ab121b9bf --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/messaging/parentPortDescribeWorker.js @@ -0,0 +1,4 @@ +var parentPort = require("node:worker_threads").parentPort; +parentPort.on("message", function (value) { + parentPort.postMessage({ received: value === undefined ? "undefined" : typeof value }); +}); diff --git a/test-app/app/src/main/assets/app/tests/testMessaging.js b/test-app/app/src/main/assets/app/tests/testMessaging.js index c06a46037..77a45ffc6 100644 --- a/test-app/app/src/main/assets/app/tests/testMessaging.js +++ b/test-app/app/src/main/assets/app/tests/testMessaging.js @@ -204,6 +204,98 @@ describe("Messaging runtime edges", function () { }); }); + // The MessageEvent constructor defaults an undefined `data` to null, as Web + // IDL requires of its init dictionary. A delivered message is not built + // from a dictionary: it carries whatever the payload deserialized to. + describe("undefined payloads", function () { + var payloads = [undefined, null, false, 0, ""]; + // Closed after every spec, so one that failed or timed out leaves no + // worker or channel behind. terminate() and close() are idempotent. + var cleanups = []; + afterEach(function () { + cleanups.forEach(function (cleanup) { cleanup(); }); + cleanups = []; + }); + + function failOnWorkerError(done) { + return function (error) { + fail("worker error: " + error.message); + done(); + }; + } + + function expectPayloads(events) { + expect(events.map(function (event) { return event.data; })).toEqual(payloads); + expect(events[0].data).toBeUndefined(); + expect("data" in events[0]).toBe(true); + } + + it("arrive unchanged on a MessagePort", function (done) { + var channel = new MessageChannel(); + // The receiver first: closing only the sender queues the close + // behind the messages, which would still be delivered. + cleanups.push(function () { channel.port2.close(); channel.port1.close(); }); + var events = []; + channel.port2.onmessage = function (event) { + events.push(event); + if (events.length === payloads.length) { + expectPayloads(events); + done(); + } + }; + payloads.forEach(function (payload) { channel.port1.postMessage(payload); }); + }); + + it("arrive unchanged on a BroadcastChannel", function (done) { + var sender = new BroadcastChannel("undefined-payloads"); + var receiver = new BroadcastChannel("undefined-payloads"); + cleanups.push(function () { sender.close(); receiver.close(); }); + var events = []; + receiver.onmessage = function (event) { + events.push(event); + if (events.length === payloads.length) { + expectPayloads(events); + done(); + } + }; + payloads.forEach(function (payload) { sender.postMessage(payload); }); + }); + + it("arrive unchanged in a worker and back on its Worker object", function (done) { + var worker = new Worker("./messaging/describeDataWorker.js"); + cleanups.push(function () { worker.terminate(); }); + var events = []; + worker.onmessage = function (event) { + events.push(event); + if (events.length === 2) { + expect(events[0].data).toEqual({ received: "undefined" }); + expect(events[1].data).toBeUndefined(); + expect("data" in events[1]).toBe(true); + done(); + } + }; + worker.onerror = failOnWorkerError(done); + worker.postMessage(undefined); + }); + + it("arrive unchanged on a node:worker_threads parentPort", function (done) { + var wt = require("node:worker_threads"); + var worker = new wt.Worker("~/tests/messaging/parentPortDescribeWorker.js"); + cleanups.push(function () { worker.terminate(); }); + worker.on("message", function (value) { + expect(value).toEqual({ received: "undefined" }); + done(); + }); + worker.on("error", failOnWorkerError(done)); + worker.postMessage(undefined); + }); + + it("still default to null in a constructed MessageEvent", function () { + expect(new MessageEvent("message").data).toBeNull(); + expect(new MessageEvent("message", { data: undefined }).data).toBeNull(); + }); + }); + describe("worker error reporting", function () { // A worker boots on its own thread, so the first error arrives whenever // the runner gets to it; specs wait for it and only then settle for diff --git a/test-app/runtime/src/main/cpp/Messaging.cpp b/test-app/runtime/src/main/cpp/Messaging.cpp index 0d0f873f1..ba32a2309 100644 --- a/test-app/runtime/src/main/cpp/Messaging.cpp +++ b/test-app/runtime/src/main/cpp/Messaging.cpp @@ -682,7 +682,13 @@ void NativeMessagePort::Drain() { if (tc.HasTerminated() || !tc.CanContinue()) { return; } - payload = tc.HasCaught() ? tc.Exception() : v8::Undefined(isolate).As(); + // Null rather than undefined when there is nothing to carry, which + // includes a thrown undefined: delivery stores the payload as given, + // and an event's `data` defaults to null. + payload = tc.HasCaught() ? tc.Exception() : Local(); + if (payload.IsEmpty() || payload->IsUndefined()) { + payload = v8::Null(isolate); + } tc.Reset(); } } diff --git a/test-app/runtime/src/main/cpp/WorkerEvents.cpp b/test-app/runtime/src/main/cpp/WorkerEvents.cpp index aea42802a..5d7129559 100644 --- a/test-app/runtime/src/main/cpp/WorkerEvents.cpp +++ b/test-app/runtime/src/main/cpp/WorkerEvents.cpp @@ -79,9 +79,16 @@ void WorkerEvents::EmitMessage(Isolate* isolate, Local receiver, return; } // HTML: a message that cannot be read still reaches its target, as - // a `messageerror` event carrying nothing. + // a `messageerror` event. Its `data` is the failure, the same as a + // port's (NativeMessagePort::Drain) and as what Node hands + // `worker.on("messageerror")`; null when nothing was thrown, since + // delivery stores the payload as given and a bare undefined would + // surface as such. + data = tc.HasCaught() ? tc.Exception() : Local(); + if (data.IsEmpty() || data->IsUndefined()) { + data = v8::Null(isolate); + } tc.Reset(); - data = v8::Undefined(isolate); ports = Local(); type = "messageerror"; } diff --git a/test-app/runtime/src/main/cpp/js/broadcast-channel.js b/test-app/runtime/src/main/cpp/js/broadcast-channel.js index 3f4aa9980..7030b4ffd 100644 --- a/test-app/runtime/src/main/cpp/js/broadcast-channel.js +++ b/test-app/runtime/src/main/cpp/js/broadcast-channel.js @@ -24,12 +24,12 @@ const { adoptPort } = require("internal/message-channel"); const addEventListener = EventTarget.prototype.addEventListener; const dispatchEvent = EventTarget.prototype.dispatchEvent; -let MessageEvent; -function getMessageEvent() { - if (MessageEvent === undefined) { - ({ MessageEvent } = require("internal/message-event")); +let createMessageEvent; +function getCreateMessageEvent() { + if (createMessageEvent === undefined) { + ({ createMessageEvent } = require("internal/message-event")); } - return MessageEvent; + return createMessageEvent; } let DOMException; @@ -64,7 +64,7 @@ class BroadcastChannel extends EventTarget { FunctionPrototypeCall( dispatchEvent, channel, - new (getMessageEvent())(event.type, { data: event.data }) + getCreateMessageEvent()(event.type, event.data) ); }; FunctionPrototypeCall(addEventListener, port, "message", relay); diff --git a/test-app/runtime/src/main/cpp/js/message-channel.js b/test-app/runtime/src/main/cpp/js/message-channel.js index 7d49cac85..0c3811462 100644 --- a/test-app/runtime/src/main/cpp/js/message-channel.js +++ b/test-app/runtime/src/main/cpp/js/message-channel.js @@ -53,12 +53,12 @@ const { const addEventListener = EventTarget.prototype.addEventListener; const dispatchEvent = EventTarget.prototype.dispatchEvent; -let MessageEvent; -function getMessageEvent() { - if (MessageEvent === undefined) { - ({ MessageEvent } = require("internal/message-event")); +let createMessageEvent; +function getCreateMessageEvent() { + if (createMessageEvent === undefined) { + ({ createMessageEvent } = require("internal/message-event")); } - return MessageEvent; + return createMessageEvent; } // WebIDL sequence. Entries are handed to the native transfer-list @@ -253,11 +253,10 @@ function emitMessage(data, ports, type) { ArrayPrototypePush(list, adoptPort(ports[i])); } } - const MessageEventCtor = getMessageEvent(); FunctionPrototypeCall( dispatchEvent, this, - new MessageEventCtor(type, { data, ports: list }) + getCreateMessageEvent()(type, data, list) ); } diff --git a/test-app/runtime/src/main/cpp/js/message-event.js b/test-app/runtime/src/main/cpp/js/message-event.js index 0ea1e99b1..cfabd968a 100644 --- a/test-app/runtime/src/main/cpp/js/message-event.js +++ b/test-app/runtime/src/main/cpp/js/message-event.js @@ -53,6 +53,12 @@ function toPortSequence(value) { return list; } +// Builds the event a delivered message dispatches. The constructor cannot +// serve delivery: per Web IDL its init dictionary treats `data: undefined` as +// absent and defaults it to null, whereas a message that deserialized to +// undefined has to arrive as undefined. +let createMessageEvent; + class MessageEvent extends Event { #data; #origin; @@ -134,6 +140,17 @@ class MessageEvent extends Event { this.#source = source; this.#ports = ports === null ? [] : toPortSequence(ports); } + + static { + createMessageEvent = (type, data, ports) => { + // Null-prototype init: the constructor reads every dictionary member, + // and one this object lacks would otherwise be looked up on + // Object.prototype, which app code can change. + const event = new MessageEvent(type, { __proto__: null, ports }); + event.#data = data; + return event; + }; + } } // Class members are non-enumerable; the IDL attributes and operations are not. @@ -150,4 +167,4 @@ ObjectDefineProperty(MessageEvent.prototype, SymbolToStringTag, { configurable: true, }); -module.exports = { MessageEvent }; +module.exports = { MessageEvent, createMessageEvent }; diff --git a/test-app/runtime/src/main/cpp/js/node-worker-threads.js b/test-app/runtime/src/main/cpp/js/node-worker-threads.js index da60893b3..7e876056b 100644 --- a/test-app/runtime/src/main/cpp/js/node-worker-threads.js +++ b/test-app/runtime/src/main/cpp/js/node-worker-threads.js @@ -47,12 +47,12 @@ const { globalEventTarget, } = require("internal/events"); -let MessageEvent; -function getMessageEvent() { - if (MessageEvent === undefined) { - ({ MessageEvent } = require("internal/message-event")); +let createMessageEvent; +function getCreateMessageEvent() { + if (createMessageEvent === undefined) { + ({ createMessageEvent } = require("internal/message-event")); } - return MessageEvent; + return createMessageEvent; } const g = globalThis; @@ -316,7 +316,7 @@ if (!isMainThread) { FunctionPrototypeCall( dispatchEvent, parentPort, - new (getMessageEvent())(event.type, { data: event.data, ports: event.ports }) + getCreateMessageEvent()(event.type, event.data, event.ports) ); }; FunctionPrototypeCall(addEventListener, globalEventTarget, "message", relay); diff --git a/test-app/runtime/src/main/cpp/js/worker-events.js b/test-app/runtime/src/main/cpp/js/worker-events.js index 73536fc44..48c174f5d 100644 --- a/test-app/runtime/src/main/cpp/js/worker-events.js +++ b/test-app/runtime/src/main/cpp/js/worker-events.js @@ -20,12 +20,12 @@ const { const g = globalThis; -let MessageEvent; -function getMessageEvent() { - if (MessageEvent === undefined) { - ({ MessageEvent } = require("internal/message-event")); +let createMessageEvent; +function getCreateMessageEvent() { + if (createMessageEvent === undefined) { + ({ createMessageEvent } = require("internal/message-event")); } - return MessageEvent; + return createMessageEvent; } // ErrorEvent is installed by the error-events builtin, which @@ -48,8 +48,7 @@ function getErrorEvent() { // is what feeds the worker's onerror chain — the worker scope's handler // first, then the parent's — which the cross-runtime worker suite asserts. function emitMessage(data, ports, type) { - const MessageEventCtor = getMessageEvent(); - dispatchEventRethrowing(this, new MessageEventCtor(type, { data, ports })); + dispatchEventRethrowing(this, getCreateMessageEvent()(type, data, ports)); } // The parent-side error delivery callout, invoked by native with the Worker