Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/jsc/bindings/BunObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,6 @@ static JSValue defaultBunSQLObject(VM& vm, JSObject* bunObject)
auto scope = DECLARE_THROW_SCOPE(vm);
auto* globalObject = defaultGlobalObject(bunObject->globalObject());
JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql);
#if BUN_DEBUG
if (scope.exception()) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, scope.exception());
#endif
RETURN_IF_EXCEPTION(scope, {});
RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, vm.propertyNames->defaultKeyword));
}
Expand All @@ -332,9 +329,6 @@ static JSValue constructBunSQLObject(VM& vm, JSObject* bunObject)
auto scope = DECLARE_THROW_SCOPE(vm);
auto* globalObject = defaultGlobalObject(bunObject->globalObject());
JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql);
#if BUN_DEBUG
if (scope.exception()) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, scope.exception());
#endif
RETURN_IF_EXCEPTION(scope, {});
auto clientData = WebCore::clientData(vm);
RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, clientData->builtinNames().SQLPublicName()));
Expand Down
12 changes: 9 additions & 3 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5627,10 +5627,11 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
}

JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
if (!object->getPropertySlot(globalObject, property, slot))
continue;
bool hasProperty = object->getPropertySlot(globalObject, property, slot);
// Ignore exceptions from "Get" proxy traps.
CLEAR_IF_EXCEPTION(scope);
if (!hasProperty)
continue;

if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
if (property == propertyNames->underscoreProto
Expand Down Expand Up @@ -5702,7 +5703,12 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
break;
if (iterating == globalObject)
break;
iterating = iterating->getPrototype(globalObject).getObject();
JSValue prototype = iterating->getPrototype(globalObject);
// Ignore exceptions from "getPrototypeOf" proxy traps.
CLEAR_IF_EXCEPTION(scope);
if (!prototype) [[unlikely]]
break;
iterating = prototype.getObject();
Comment thread
claude[bot] marked this conversation as resolved.
}
}

Expand Down
9 changes: 6 additions & 3 deletions src/jsc/bindings/napi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2077,14 +2077,17 @@ 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();
if (!proto) {
NAPI_RETURN_IF_EXCEPTION(env);
JSValue proto = owner->getPrototype(globalObject);
NAPI_RETURN_IF_EXCEPTION(env);
if (!proto.isObject()) {
break;
}
owner = proto;
owner = asObject(proto);
}
} else {
owner->getOwnPropertyDescriptor(globalObject, propKey, desc);
NAPI_RETURN_IF_EXCEPTION(env);
}

// V8 never applies ONLY_WRITABLE/ONLY_CONFIGURABLE to Proxy keys
Expand Down
78 changes: 78 additions & 0 deletions test/js/bun/util/inspect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -928,3 +928,81 @@ describe.skipIf(!isASAN)("object mutated while being formatted", () => {
expect(exitCode).toBe(0);
});
});

// The slow property walk (objects with static tables, proxies in the prototype
// chain) looks each property up with getPropertySlot. When that lookup threw,
// the exception stayed pending while the walk moved on to the next property,
// and the prototype step dereferenced the empty value a throwing getPrototype
// returns. Run in a child: before the fix these abort or segfault.
describe.concurrent("property lookup throws while formatting", () => {
async function run(code) {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: {
...bunEnv,
// Skip symbolizing a failure report; symbolization of the debug
// binary takes longer than the test timeout.
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "symbolize=0"].filter(Boolean).join(":"),
},
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
}

it("lazy static property whose builder throws", async () => {
// Formatting Bun reifies its static table, and the builders of `$`, `sql`,
// `postgres` and `SQL` call into JS. Right after a stack overflow unwinds,
// native code may still run but JSC refuses to enter JS, so at the first
// depth where Bun.inspect(Bun) gets through those builders throw, and the
// entry after the last of them is only printed if the walk kept going
// (debug builds used to abort instead).
//
// process.env and util.inspect are set up before recursing: creating
// either one enters JS too, and failing inside their lazy initializers
// aborts the process regardless of this fix (Bun.env is a Proxy with a
// custom inspect function on Windows).
expect(
await run(`
void process.env;
Bun.inspect({ [Symbol.for("nodejs.util.inspect.custom")]() { return ""; } });
const names = Object.getOwnPropertyNames(Bun);
const afterSQL = names[names.indexOf("SQL") + 1];
let result;
function recurse() {
try { recurse(); } catch {}
if (result === undefined) {
try { result = Bun.inspect(Bun); } catch {}
}
}
recurse();
console.log(result.includes("\\n " + afterSQL + ":"));
`),
Comment thread
claude[bot] marked this conversation as resolved.
).toEqual({ stdout: "true\n", stderr: "", exitCode: 0 });
});

it("proxy get trap in the prototype chain throws", async () => {
expect(
await run(`
const proto = new Proxy({}, {
ownKeys() { return ["a", "b"]; },
get(target, key) {
if (key === "a") throw new Error("a");
return key === "b" ? 2 : undefined;
},
});
console.log(Bun.inspect(Object.create(proto)));
`),
).toEqual({ stdout: "{\n b: 2,\n}\n", stderr: "", exitCode: 0 });
});

it("proxy getPrototypeOf trap in the prototype chain throws", async () => {
expect(
await run(`
const proto = new Proxy({}, { getPrototypeOf() { throw new Error("x"); } });
console.log(Bun.inspect(Object.create(proto)));
`),
).toEqual({ stdout: "{}\n", stderr: "", exitCode: 0 });
});
});
63 changes: 63 additions & 0 deletions test/napi/napi-app/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,69 @@ nativeTests.test_get_all_property_names_proxy_and_string_wrapper = () => {
show("frozen writable:", apn(Object.freeze({ a: 1, b: 2 }), napi_key_writable));
};

// A trap that throws while the keys are being filtered must surface as the
// thrown exception, both when the proxy is the object itself and when it is
// found while climbing the prototype chain.
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;

const proxy = new Proxy(
{},
{
ownKeys: () => ["x"],
getOwnPropertyDescriptor() {
throw new Error("getOwnPropertyDescriptor trap");
},
},
);
for (const [label, object, mode] of [
["own_only proxy:", proxy, napi_key_own_only],
["include_prototypes proxy-proto:", Object.create(proxy), napi_key_include_prototypes],
]) {
try {
const r = nativeTests.get_all_property_names(object, mode, napi_key_enumerable, napi_key_keep_numbers);
console.log(label, "returned status=" + r.status, "keys=" + JSON.stringify(r.keys));
} catch (e) {
console.log(label, "threw", e.message);
}
}
};

// Bun walks the chain once to collect the keys and again to filter them, so a
// getPrototypeOf trap can succeed while collecting and throw while filtering.
// Node only walks once and never sees the second call, so this is not compared
// against it.
nativeTests.test_get_all_property_names_throwing_get_prototype_of = () => {
const napi_key_include_prototypes = 0;
const napi_key_enumerable = 1 << 1;
const napi_key_keep_numbers = 0;

let calls = 0;
const proxy = new Proxy(
{},
{
getPrototypeOf() {
if (++calls > 1) throw new Error("getPrototypeOf trap");
return { z: 1 };
},
},
);
try {
const r = nativeTests.get_all_property_names(
Object.create(proxy),
napi_key_include_prototypes,
napi_key_enumerable,
napi_key_keep_numbers,
);
console.log("returned status=" + r.status, "keys=" + JSON.stringify(r.keys));
} catch (e) {
console.log("threw", e.message);
}
};

nativeTests.test_set_property = () => {
const objects = [
{},
Expand Down
11 changes: 11 additions & 0 deletions test/napi/napi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,17 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => {
expect(output).toContain(`plain writable: status=0 keys=["w","nc"]`);
expect(output).toContain(`frozen writable: status=0 keys=[]`);
});
it("reports a trap that throws while filtering instead of leaving it pending", async () => {
const output = await checkSameOutput("test_get_all_property_names_throwing_trap", []);
expect(output.split(/\r?\n/)).toEqual([
"own_only proxy: threw getOwnPropertyDescriptor trap",
"include_prototypes proxy-proto: threw getOwnPropertyDescriptor trap",
]);
});
it("reports a getPrototypeOf trap that throws while filtering", async () => {
const output = await runOn(bunExe(), "test_get_all_property_names_throwing_get_prototype_of", []);
expect(output.trim()).toBe("threw getPrototypeOf trap");
});
});

describe("napi_value <=> integer conversion", () => {
Expand Down