diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 983a348f73a5..26cab48f0202 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -5643,10 +5643,11 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC: } JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get); - if (!object->getPropertySlot(globalObject, property, slot)) - continue; - // Ignore exceptions from "Get" proxy traps. + bool hasProperty = object->getPropertySlot(globalObject, property, slot); + // Ignore exceptions from "Get" proxy traps and lazy property initializers. CLEAR_IF_EXCEPTION(scope); + if (!hasProperty) + continue; if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) { if (property == propertyNames->underscoreProto @@ -5718,7 +5719,11 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC: break; if (iterating == globalObject) break; - iterating = iterating->getPrototype(globalObject).getObject(); + JSValue prototype = iterating->getPrototype(globalObject); + // Empty when a Proxy "getPrototypeOf" trap threw; the exception is cleared below. + if (!prototype) [[unlikely]] + break; + iterating = prototype.getObject(); } } diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index 6b90a8f89d58..54b0930c31f7 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -2077,7 +2077,10 @@ extern "C" napi_status napi_get_all_property_names( if (key_mode == napi_key_include_prototypes) { // Climb up the prototype chain to find inherited properties while (!owner->getOwnPropertyDescriptor(globalObject, propKey, desc)) { - JSObject* proto = owner->getPrototype(globalObject).getObject(); + NAPI_RETURN_IF_EXCEPTION(env); + JSValue prototype = owner->getPrototype(globalObject); + NAPI_RETURN_IF_EXCEPTION(env); + JSObject* proto = prototype.getObject(); if (!proto) { break; } @@ -2085,6 +2088,7 @@ extern "C" napi_status napi_get_all_property_names( } } else { owner->getOwnPropertyDescriptor(globalObject, propKey, desc); + NAPI_RETURN_IF_EXCEPTION(env); } // V8 never applies ONLY_WRITABLE/ONLY_CONFIGURABLE to Proxy keys diff --git a/src/jsc/modules/NodeUtilTypesModule.cpp b/src/jsc/modules/NodeUtilTypesModule.cpp index 46050f5ed8d1..dcbe91751052 100644 --- a/src/jsc/modules/NodeUtilTypesModule.cpp +++ b/src/jsc/modules/NodeUtilTypesModule.cpp @@ -901,6 +901,8 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionIsError, PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry, &vm); bool has = object->getPropertySlot(globalObject, vm.propertyNames->toStringTagSymbol, slot); scope.assertNoException(); + // A VMInquiry slot forbids entering the VM while it is alive, and getPrototype() below may run a Proxy trap. + slot.disallowVMEntry.reset(); if (has) { if (slot.isValue()) { JSValue value = slot.getValue(globalObject, vm.propertyNames->toStringTagSymbol); @@ -914,6 +916,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionIsError, } JSValue proto = object->getPrototype(globalObject); + RETURN_IF_EXCEPTION(scope, {}); if (proto.isCell() && (proto.inherits() || proto.asCell()->type() == ErrorInstanceType || proto.inherits())) return JSValue::encode(jsBoolean(true)); } diff --git a/test/js/bun/util/inspect.test.js b/test/js/bun/util/inspect.test.js index 5c91f3dbe05f..f67c385c44ab 100644 --- a/test/js/bun/util/inspect.test.js +++ b/test/js/bun/util/inspect.test.js @@ -928,3 +928,62 @@ describe.skipIf(!isASAN)("object mutated while being formatted", () => { expect(exitCode).toBe(0); }); }); + +describe.concurrent("exceptions thrown while walking properties", () => { + async function run(fixture) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + it("a lazy property initializer that throws only hides that property", async () => { + // Bun.$ is initialized on first access and reads process.env while doing so. + // The exception it throws here used to stay pending while the following + // properties were visited, so they were dropped from the output (and debug + // builds asserted when the next initializer ran). + const { stdout, stderr, exitCode } = await run(` + // Formatting a value with a custom inspect function (process.env on Windows) + // loads node:util's inspect helpers, which read process.env. Load them + // before breaking process.env so only the Bun.$ initializer throws. + Bun.inspect({ [Bun.inspect.custom]() { return ""; } }); + Object.defineProperty(process, "env", { get() { throw new Error("boom"); } }); + const s = Bun.inspect(Bun); + console.log(/^ \\$:/m.test(s), /^ Archive:/m.test(s), /^ version:/m.test(s)); + `); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "false true true\n", stderr: "", exitCode: 0 }); + }); + + it("a get trap that throws for one inherited property only hides that property", async () => { + // Same leak through a Proxy: the pending exception made the remaining + // properties look missing and then the prototype step dereferenced the + // empty value returned by the proxy's getPrototype. + const { stdout, stderr, exitCode } = await run(` + const proto = new Proxy({ a: 1, b: 2, c: 3 }, { + get(target, key, receiver) { + if (key === "a") throw new Error("trap"); + return Reflect.get(target, key, receiver); + }, + }); + console.log(Bun.inspect(Object.create(proto))); + `); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "{\n b: 2,\n c: 3,\n}\n", stderr: "", exitCode: 0 }); + }); + + it("a throwing getPrototypeOf trap in the prototype chain stops the walk", async () => { + const { stdout, stderr, exitCode } = await run(` + const proto = new Proxy({ inherited: 1 }, { getPrototypeOf() { throw new Error("trap"); } }); + console.log(Bun.inspect(Object.create(proto))); + console.log(Object.create(proto)); + `); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: "{\n inherited: 1,\n}\n{\n inherited: 1,\n}\n", + stderr: "", + exitCode: 0, + }); + }); +}); diff --git a/test/js/node/util/util.test.js b/test/js/node/util/util.test.js index 7689c1230321..b9d38274c846 100644 --- a/test/js/node/util/util.test.js +++ b/test/js/node/util/util.test.js @@ -23,7 +23,7 @@ import assert from "assert"; import { describe, expect, it } from "bun:test"; -import "harness"; +import { bunEnv, bunExe } from "harness"; import util from "util"; // const context = require('vm').runInNewContext; // TODO: Use a vm polyfill @@ -152,6 +152,27 @@ describe("util", () => { let err8 = new Error3(); strictEqual(util.isError(err8), true); }); + + it("propagates an exception from a getPrototypeOf trap", async () => { + // Used to crash the process, so the call runs in a child. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const proxy = new Proxy({}, { getPrototypeOf() { throw new Error("getPrototypeOf trap"); } }); + try { + console.log("returned", require("util").isError(proxy)); + } catch (e) { + console.log("threw", e.message); + }`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "threw getPrototypeOf trap\n", stderr: "", exitCode: 0 }); + }); }); describe("isObject", () => { diff --git a/test/napi/napi-app/module.js b/test/napi/napi-app/module.js index b0aece155716..e59c627ca75d 100644 --- a/test/napi/napi-app/module.js +++ b/test/napi/napi-app/module.js @@ -321,6 +321,40 @@ nativeTests.test_get_all_property_names_proxy_and_string_wrapper = () => { show("frozen writable:", apn(Object.freeze({ a: 1, b: 2 }), napi_key_writable)); }; +nativeTests.test_get_all_property_names_throwing_trap = () => { + const napi_key_include_prototypes = 0; + const napi_key_own_only = 1; + const napi_key_enumerable = 1 << 1; + const napi_key_keep_numbers = 0; + + // The enumerable filter makes both engines ask the proxy for descriptors. + const makeProxy = () => + new Proxy( + { k: 1 }, + { + getOwnPropertyDescriptor() { + throw new Error("descriptor trap"); + }, + }, + ); + for (const [label, obj, mode] of [ + ["own_only", makeProxy(), napi_key_own_only], + ["include_prototypes", Object.create(makeProxy()), napi_key_include_prototypes], + ]) { + try { + const { status, keys } = nativeTests.get_all_property_names( + obj, + mode, + napi_key_enumerable, + napi_key_keep_numbers, + ); + console.log(`${label}: returned status=${status} keys=${JSON.stringify(keys)}`); + } catch (e) { + console.log(`${label}: threw ${e.message}`); + } + } +}; + nativeTests.test_set_property = () => { const objects = [ {}, diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 9bede65a492e..962a2ecfc659 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -880,6 +880,11 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { expect(output).toContain(`plain writable: status=0 keys=["w","nc"]`); expect(output).toContain(`frozen writable: status=0 keys=[]`); }); + it("propagates an exception thrown by a Proxy trap while filtering", async () => { + const output = await checkSameOutput("test_get_all_property_names_throwing_trap", []); + expect(output).toContain("own_only: threw descriptor trap"); + expect(output).toContain("include_prototypes: threw descriptor trap"); + }); }); describe("napi_value <=> integer conversion", () => {