diff --git a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp index 3125ee8411a..26f98a9f49d 100644 --- a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp +++ b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp @@ -26,6 +26,7 @@ #include "config.h" #include "SerializedScriptValue.h" +#include "BunBuiltinNames.h" #include "BunString.h" // #include "BlobRegistry.h" // #include "ByteArrayPixelBuffer.h" @@ -1586,6 +1587,16 @@ class CloneSerializer : public CloneBase { VM& vm = m_lexicalGlobalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); + // node:worker_threads.markAsUncloneable: if the object was tagged with + // our private-name marker, refuse to clone it. Checked before the + // isArray/object-type dispatch below so marked arrays are caught too. + if (value.isObject()) { + if (JSC::asObject(value)->getDirect(vm, WebCore::builtinNames(vm).isUncloneablePrivateName())) { + code = SerializationReturnCode::DataCloneError; + return true; + } + } + if (isArray(value)) return false; diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 8db7b462b33..6a2a0ca831b 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -64,6 +64,9 @@ #include "CloseEvent.h" #include "JSMessagePort.h" #include "JSBroadcastChannel.h" +#include "BunBuiltinNames.h" +#include +#include namespace WebCore { @@ -672,4 +675,49 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionPostMessage, return JSValue::encode(jsUndefined()); } +// node:worker_threads.markAsUncloneable(obj) +// +// Marks `obj` so that any subsequent structured-clone attempt (structuredClone, +// MessagePort.postMessage, Worker workerData, BroadcastChannel.postMessage) +// throws a DOMException with name "DataCloneError" when `obj` appears as a +// value. +// +// Per the Node.js spec (https://nodejs.org/api/worker_threads.html#workermarkasuncloneableobject): +// - No-op for primitives (non-object / non-function) and null. +// - No effect on ArrayBuffer, SharedArrayBuffer, or any Buffer/TypedArray/DataView. +// - Cannot be undone. +JSC_DEFINE_HOST_FUNCTION(jsFunctionMarkAsUncloneable, + (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + JSC::VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + + if (callFrame->argumentCount() < 1) + return JSC::JSValue::encode(JSC::jsUndefined()); + + JSC::JSValue value = callFrame->argument(0); + + // Primitives (including null/undefined) are a no-op per the Node spec. + // Node's JS implementation checks `typeof obj === "object" || typeof obj === "function"` + // and that obj !== null. isObject() covers both cases in JSC. + if (!value.isObject()) + return JSC::JSValue::encode(JSC::jsUndefined()); + + JSC::JSObject* object = JSC::asObject(value); + + // "This has no effect on ArrayBuffer, or any Buffer like objects." — Node docs. + // We implement that by refusing to tag these types, so the serializer's + // existing special-case clone paths for them remain intact. + if (object->inherits() || object->inherits()) + return JSC::JSValue::encode(JSC::jsUndefined()); + + // putDirect with a private-name identifier: invisible to Object.keys, JSON, + // for-in, Reflect.ownKeys — but cheap to read from native code. + object->putDirect(vm, WebCore::builtinNames(vm).isUncloneablePrivateName(), JSC::jsBoolean(true), + JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly); + RETURN_IF_EXCEPTION(scope, {}); + + return JSC::JSValue::encode(JSC::jsUndefined()); +} + } // namespace WebCore diff --git a/src/bun.js/bindings/webcore/Worker.h b/src/bun.js/bindings/webcore/Worker.h index bbc73053ddf..017838eac0e 100644 --- a/src/bun.js/bindings/webcore/Worker.h +++ b/src/bun.js/bindings/webcore/Worker.h @@ -125,5 +125,6 @@ class Worker final : public ThreadSafeRefCounted, public EventTargetWith JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject); JSC_DECLARE_HOST_FUNCTION(jsFunctionPostMessage); +JSC_DECLARE_HOST_FUNCTION(jsFunctionMarkAsUncloneable); } // namespace WebCore diff --git a/src/js/builtins/BunBuiltinNames.h b/src/js/builtins/BunBuiltinNames.h index c97c72e3fae..b0acacc08d6 100644 --- a/src/js/builtins/BunBuiltinNames.h +++ b/src/js/builtins/BunBuiltinNames.h @@ -152,6 +152,7 @@ using namespace JSC; macro(isAbsolute) \ macro(isDisturbed) \ macro(isPaused) \ + macro(isUncloneable) \ macro(isWindows) \ macro(join) \ macro(kind) \ diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index c6d0633dde9..a1cf074d2f9 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -219,6 +219,8 @@ function markAsUntransferable() { throwNotImplemented("worker_threads.markAsUntransferable"); } +const markAsUncloneable = $newCppFunction("Worker.cpp", "jsFunctionMarkAsUncloneable", 1); + function moveMessagePortToContext() { throwNotImplemented("worker_threads.moveMessagePortToContext"); } @@ -418,6 +420,7 @@ export default { getHeapSnapshot() { return {}; }, + markAsUncloneable, markAsUntransferable, moveMessagePortToContext, receiveMessageOnPort, diff --git a/test/js/node/worker_threads/markAsUncloneable.test.ts b/test/js/node/worker_threads/markAsUncloneable.test.ts new file mode 100644 index 00000000000..c08ca317745 --- /dev/null +++ b/test/js/node/worker_threads/markAsUncloneable.test.ts @@ -0,0 +1,282 @@ +import { bunEnv, bunExe, tempDir } from "harness"; +import { test, expect, describe } from "bun:test"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { BroadcastChannel, markAsUncloneable, MessageChannel, Worker } from "node:worker_threads"; + +// Helper: assert the given thunk throws a DataCloneError DOMException. +function expectDataCloneError(thunk: () => void) { + let err: unknown; + try { + thunk(); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(Error); + // Node uses `DOMException` with name "DataCloneError". + expect((err as { name?: string }).name).toBe("DataCloneError"); +} + +describe("node:worker_threads.markAsUncloneable", () => { + test("is a function with arity 1", () => { + expect(typeof markAsUncloneable).toBe("function"); + expect(markAsUncloneable.length).toBe(1); + }); + + test("is exported on the default export too", () => { + const wt = require("node:worker_threads"); + expect(wt.markAsUncloneable).toBe(markAsUncloneable); + }); + + test("is exported on the bare 'worker_threads' specifier too", () => { + const bare = require("worker_threads"); + expect(bare.markAsUncloneable).toBe(markAsUncloneable); + }); + + test("returns undefined", () => { + expect(markAsUncloneable({})).toBeUndefined(); + }); + + test("is a no-op on primitives and null/undefined", () => { + expect(() => markAsUncloneable(1)).not.toThrow(); + expect(() => markAsUncloneable(0)).not.toThrow(); + expect(() => markAsUncloneable(Number.NaN)).not.toThrow(); + expect(() => markAsUncloneable(true)).not.toThrow(); + expect(() => markAsUncloneable(false)).not.toThrow(); + expect(() => markAsUncloneable("x")).not.toThrow(); + expect(() => markAsUncloneable("")).not.toThrow(); + expect(() => markAsUncloneable(null)).not.toThrow(); + expect(() => markAsUncloneable(undefined)).not.toThrow(); + expect(() => markAsUncloneable(Symbol())).not.toThrow(); + expect(() => markAsUncloneable(0n)).not.toThrow(); + // Zero-arg: also a no-op per Node spec (arg defaults to undefined). + expect(() => (markAsUncloneable as () => void)()).not.toThrow(); + }); + + test("accepts a function as the argument (typeof === 'function')", () => { + const fn = () => {}; + expect(() => markAsUncloneable(fn)).not.toThrow(); + expectDataCloneError(() => structuredClone(fn)); + }); + + test("structuredClone(marked) throws DataCloneError", () => { + const obj = { foo: "bar" }; + markAsUncloneable(obj); + expectDataCloneError(() => structuredClone(obj)); + }); + + test("structuredClone catches marked object nested inside an array", () => { + const inner = { secret: 1 }; + markAsUncloneable(inner); + const outer = [1, 2, inner, 4]; + expectDataCloneError(() => structuredClone(outer)); + }); + + test("structuredClone catches marked object nested inside an object", () => { + const inner = { secret: 1 }; + markAsUncloneable(inner); + const outer = { a: 1, wrapper: { nested: inner } }; + expectDataCloneError(() => structuredClone(outer)); + }); + + test("structuredClone catches a marked array at the root", () => { + const arr: unknown[] = [1, 2, 3]; + markAsUncloneable(arr); + expectDataCloneError(() => structuredClone(arr)); + }); + + test("structuredClone catches a marked array nested inside an object", () => { + const arr: unknown[] = [1, 2, 3]; + markAsUncloneable(arr); + const outer = { items: arr }; + expectDataCloneError(() => structuredClone(outer)); + }); + + test("structuredClone catches marked object nested inside a Map value", () => { + const inner = { secret: 1 }; + markAsUncloneable(inner); + const map = new Map([["k", inner]]); + expectDataCloneError(() => structuredClone(map)); + }); + + test("structuredClone catches marked object nested inside a Set", () => { + const inner = { secret: 1 }; + markAsUncloneable(inner); + const set = new Set([inner]); + expectDataCloneError(() => structuredClone(set)); + }); + + test("MessageChannel: port1.postMessage(marked) throws DataCloneError", () => { + const { port1, port2 } = new MessageChannel(); + try { + const obj = { x: 1 }; + markAsUncloneable(obj); + expectDataCloneError(() => port1.postMessage(obj)); + } finally { + port1.close(); + port2.close(); + } + }); + + test("BroadcastChannel.postMessage(marked) throws DataCloneError", () => { + const bc = new BroadcastChannel("markAsUncloneable-test"); + try { + const obj = { x: 1 }; + markAsUncloneable(obj); + expectDataCloneError(() => bc.postMessage(obj)); + } finally { + bc.close(); + } + }); + + test("new Worker with marked workerData throws DataCloneError", () => { + const obj: Record = { x: 1 }; + markAsUncloneable(obj); + expectDataCloneError( + () => + new Worker("postMessage('hi')", { + eval: true, + workerData: obj, + }), + ); + }); + + test("ArrayBuffer is unaffected by markAsUncloneable (Node spec no-op)", () => { + const buf = new ArrayBuffer(8); + markAsUncloneable(buf); + const cloned = structuredClone(buf); + expect(cloned).toBeInstanceOf(ArrayBuffer); + expect(cloned.byteLength).toBe(8); + expect(cloned).not.toBe(buf); + }); + + test("SharedArrayBuffer is unaffected by markAsUncloneable (Node spec no-op)", () => { + const sab = new SharedArrayBuffer(8); + markAsUncloneable(sab); + // Node spec: markAsUncloneable has no effect on SharedArrayBuffer. We + // only assert that cloning does NOT throw DataCloneError — Bun's + // structuredClone handling of SAB itself is outside the scope of this PR, + // so we catch any thrown error and only fail on a DataCloneError name. + let err: unknown; + try { + structuredClone(sab); + } catch (e) { + err = e; + } + expect((err as { name?: string } | undefined)?.name).not.toBe("DataCloneError"); + }); + + test("Buffer is unaffected by markAsUncloneable (Node spec no-op)", () => { + const b = Buffer.from("hello"); + markAsUncloneable(b); + const cloned = structuredClone(b); + expect(cloned).toBeInstanceOf(Uint8Array); + expect(Buffer.from(cloned).toString("utf8")).toBe("hello"); + }); + + test("Uint8Array / TypedArrays are unaffected by markAsUncloneable (Node spec no-op)", () => { + const u8 = new Uint8Array([1, 2, 3, 4]); + markAsUncloneable(u8); + const cloned = structuredClone(u8); + expect(Array.from(cloned)).toEqual([1, 2, 3, 4]); + }); + + test("DataView is unaffected by markAsUncloneable (Node spec no-op)", () => { + const view = new DataView(new ArrayBuffer(4)); + view.setUint32(0, 0xdeadbeef); + markAsUncloneable(view); + const cloned = structuredClone(view); + expect(cloned).toBeInstanceOf(DataView); + expect(cloned.getUint32(0)).toBe(0xdeadbeef); + }); + + test("marking is irreversible: a second clone still throws", () => { + const obj = { z: 1 }; + markAsUncloneable(obj); + expectDataCloneError(() => structuredClone(obj)); + expectDataCloneError(() => structuredClone(obj)); + }); + + test("marked object remains usable locally", () => { + const obj: Record = { a: 1, b: "two", c: [3, 4] }; + markAsUncloneable(obj); + + expect(obj.a).toBe(1); + expect(obj.b).toBe("two"); + obj.d = true; + expect(obj.d).toBe(true); + delete obj.a; + expect("a" in obj).toBe(false); + }); + + test("the uncloneable marker is hidden from enumeration APIs", () => { + const obj = { visible: 42 }; + markAsUncloneable(obj); + + expect(Object.keys(obj)).toEqual(["visible"]); + expect(Object.getOwnPropertyNames(obj)).toEqual(["visible"]); + expect(Object.getOwnPropertySymbols(obj)).toEqual([]); + expect(Reflect.ownKeys(obj)).toEqual(["visible"]); + expect(JSON.stringify(obj)).toBe('{"visible":42}'); + + const seen: string[] = []; + for (const key in obj) seen.push(key); + expect(seen).toEqual(["visible"]); + }); + + test("child worker: markAsUncloneable works inside a worker thread too", async () => { + using dir = tempDir("markAsUncloneable-child-worker", { + "worker.mjs": ` + import { parentPort, markAsUncloneable } from "node:worker_threads"; + const obj = { fromChild: true }; + markAsUncloneable(obj); + let threw = false; + let name = ""; + try { + structuredClone(obj); + } catch (e) { + threw = true; + name = e.name; + } + parentPort.postMessage({ threw, name }); + `, + }); + + // Build the file:// URL via pathToFileURL in the parent scope so paths + // with spaces or Windows drive letters (file:///C:/…) are handled + // correctly, then inject the finished URL string into the -e script. + const workerUrl = pathToFileURL(join(String(dir), "worker.mjs")).href; + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const w = new Worker(${JSON.stringify(workerUrl)}); + w.on("message", m => { + console.log(JSON.stringify(m)); + w.terminate(); + }); + w.on("error", e => { + console.error("worker error:", e); + process.exit(1); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // stderr may contain benign debug-build / ASAN warnings; we only care + // that no error from the worker itself made it through. + expect(stderr).not.toContain("worker error:"); + expect(stdout.trim()).toBe(JSON.stringify({ threw: true, name: "DataCloneError" })); + // Surface full stderr if the child exited non-zero for any other reason, + // so CI logs show the actual diff rather than just "expected 0 got N". + if (exitCode !== 0) expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 555c205c6fc..5c8adaf1c21 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -7,6 +7,7 @@ import wt, { BroadcastChannel, getEnvironmentData, isMainThread, + markAsUncloneable, markAsUntransferable, MessageChannel, MessagePort, @@ -35,6 +36,7 @@ test("support eval in worker", async () => { test("all worker_threads module properties are present", () => { expect(wt).toHaveProperty("getEnvironmentData"); expect(wt).toHaveProperty("isMainThread"); + expect(wt).toHaveProperty("markAsUncloneable"); expect(wt).toHaveProperty("markAsUntransferable"); expect(wt).toHaveProperty("moveMessagePortToContext"); expect(wt).toHaveProperty("parentPort"); @@ -51,6 +53,7 @@ test("all worker_threads module properties are present", () => { expect(getEnvironmentData).toBeFunction(); expect(isMainThread).toBeBoolean(); + expect(markAsUncloneable).toBeFunction(); expect(markAsUntransferable).toBeFunction(); expect(moveMessagePortToContext).toBeFunction(); expect(parentPort).toBeNull(); diff --git a/test/regression/issue/29423.test.ts b/test/regression/issue/29423.test.ts new file mode 100644 index 00000000000..9605de1ed0a --- /dev/null +++ b/test/regression/issue/29423.test.ts @@ -0,0 +1,122 @@ +// Regression test for https://github.com/oven-sh/bun/issues/29423. +// +// Before markAsUncloneable was implemented, every undici 8.0.3+ constructor +// that called webidl.util.markAsUncloneable (CacheStorage, Response, Request, +// Headers, FormData, WebSocket, EventSource) crashed at module-load time with: +// TypeError: webidl.util.markAsUncloneable is not a function +// +// undici 8.0.3 removed the runtime feature probe (nodejs/undici#4968) and +// 8.1.0's lib/web/webidl/index.js does +// const { markAsUncloneable } = require("node:worker_threads"); +// unconditionally at module load. +// +// This test exercises the real npm package (not Bun's built-in `undici` shim, +// which intercepts the bare `"undici"` specifier). Deep subpath imports bypass +// the shim, so we drive the webidl module-load through them. + +import { test, expect } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const kUndiciVersion = "^8.1.0"; + +// Pulls undici from the real npm registry, so give the install enough time to +// either succeed or fail gracefully (the "install failed → skip" branch below +// only runs if the child exits; the default 5s per-test timeout isn't enough +// for a cold registry fetch). +const kTestTimeoutMs = 120_000; + +test( + "undici 8.1+ loads webidl without crashing (regression for missing markAsUncloneable)", + async () => { + using dir = tempDir("markAsUncloneable-undici-regression", { + "package.json": JSON.stringify({ + name: "markasuncloneable-regression", + version: "0.0.0", + private: true, + dependencies: { undici: kUndiciVersion }, + }), + }); + + // Install undici from npm into the temp dir. If we can't reach the registry + // (offline CI / sandbox), skip the test instead of failing — the smoke test + // is only meaningful with the real package. + await using install = Bun.spawn({ + cmd: [bunExe(), "install", "--no-save"], + cwd: String(dir), + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [installStdout, installStderr, installExit] = await Promise.all([ + install.stdout.text(), + install.stderr.text(), + install.exited, + ]); + + if (installExit !== 0) { + console.warn( + `[markAsUncloneable-undici-regression] Skipping: 'bun install undici@${kUndiciVersion}' failed (exit ${installExit}).\nstdout: ${installStdout}\nstderr: ${installStderr}`, + ); + return; + } + + // Resolve the installed undici version and confirm we got >=8.1 (otherwise + // the regression does not apply). Read package.json directly so we don't + // depend on any particular `bun pm ls` output format. + let major: number; + let minor: number; + let patch: number; + try { + const pkg = JSON.parse(readFileSync(join(String(dir), "node_modules", "undici", "package.json"), "utf8")) as { + version: string; + }; + const match = pkg.version.match(/^(\d+)\.(\d+)\.(\d+)/); + if (!match) { + console.warn( + `[markAsUncloneable-undici-regression] Skipping: could not parse undici version '${pkg.version}'.`, + ); + return; + } + major = Number(match[1]); + minor = Number(match[2]); + patch = Number(match[3]); + } catch (err) { + console.warn(`[markAsUncloneable-undici-regression] Skipping: could not read undici package.json: ${err}`); + return; + } + if (major < 8 || (major === 8 && minor < 1)) { + console.warn( + `[markAsUncloneable-undici-regression] Skipping: resolved undici@${major}.${minor}.${patch} is too old.`, + ); + return; + } + + // Now drive the actual repro: deep-subpath require of cachestorage.js pulls + // in lib/web/webidl/index.js, which calls markAsUncloneable at module-load + // time. If markAsUncloneable is missing, the require throws synchronously. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + // Keep this aligned with the user-visible repro in the PR body. + `require("undici/lib/web/cache/cachestorage.js"); console.log("ok");`, + ], + cwd: String(dir), + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // The specific pre-fix symptom we're guarding against. Print both streams + // on failure so diagnosis is easy. + expect(stderr).not.toContain("markAsUncloneable is not a function"); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }, + kTestTimeoutMs, +);