diff --git a/src/jsc/bindings/BunObject.cpp b/src/jsc/bindings/BunObject.cpp index 04c1c4ca808b..02cee98b85fe 100644 --- a/src/jsc/bindings/BunObject.cpp +++ b/src/jsc/bindings/BunObject.cpp @@ -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)); } @@ -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())); diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 82662920a933..2f936316c8d7 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -5616,10 +5616,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 found = object->getPropertySlot(globalObject, property, slot); // Ignore exceptions from "Get" proxy traps. CLEAR_IF_EXCEPTION(scope); + if (!found) + continue; if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) { if (property == propertyNames->underscoreProto @@ -5691,7 +5692,10 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC: break; if (iterating == globalObject) break; - iterating = iterating->getPrototype(globalObject).getObject(); + JSValue proto = iterating->getPrototype(globalObject); + // Ignore exceptions from Proxy "getPrototypeOf" trap. + CLEAR_IF_EXCEPTION(scope); + iterating = proto ? proto.getObject() : nullptr; } } diff --git a/test/js/bun/util/BunObject.test.ts b/test/js/bun/util/BunObject.test.ts index 63f2ec66c9ae..b46062a6930f 100644 --- a/test/js/bun/util/BunObject.test.ts +++ b/test/js/bun/util/BunObject.test.ts @@ -1,6 +1,7 @@ import { env } from "bun"; import { hasNonReifiedStatic } from "bun:internal-for-testing"; import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; test("hasNonReifiedStatic", () => { expect(hasNonReifiedStatic(Bun), "do not eagerly initialize the Bun object. This will make Bun much slower.").toBe( true, @@ -33,3 +34,39 @@ test("await import('bun')", async () => { } expect(BunESM.default).toBe(Bun); }); + +test("a lazy property whose builtin fails to load throws from the read", async () => { + // The shell builtin ($) and bun:sql's module body (sql, SQL, postgres) call Symbol(), so + // clobbering it makes each builder throw. The read must throw that error and the slot + // must stay unreified so a later read runs the builder again. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `globalThis.Symbol = NaN; + const results = {}; + for (const name of ["$", "sql", "SQL", "postgres"]) { + const names = []; + for (let i = 0; i < 2; i++) { + try { Bun[name]; names.push("no throw"); } catch (e) { names.push(e.constructor.name); } + } + results[name] = names; + } + console.log(JSON.stringify(results));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ + $: ["TypeError", "TypeError"], + sql: ["TypeError", "TypeError"], + SQL: ["TypeError", "TypeError"], + postgres: ["TypeError", "TypeError"], + }), + stderr: "", + exitCode: 0, + }); +}); diff --git a/test/js/bun/util/inspect.test.js b/test/js/bun/util/inspect.test.js index 5c91f3dbe05f..263fef2fdf76 100644 --- a/test/js/bun/util/inspect.test.js +++ b/test/js/bun/util/inspect.test.js @@ -2,6 +2,73 @@ import { describe, expect, it } from "bun:test"; import { bunEnv, bunExe, isASAN, isWindows, normalizeBunSnapshot, tmpdirSync } from "harness"; import { join } from "path"; import util from "util"; + +it("Proxy prototype with throwing getPrototypeOf trap does not crash", () => { + const obj = {}; + Object.setPrototypeOf( + obj, + new Proxy( + {}, + { + getPrototypeOf() { + throw new Error("trap threw"); + }, + }, + ), + ); + expect(Bun.inspect(obj)).toBe("{}"); +}); + +it("Proxy prototype with throwing get trap does not crash", () => { + // A throwing `get` trap makes getPropertySlot return false with an exception + // pending; ownKeys/getOwnPropertyDescriptor make sure the names get enumerated + // so the walk reaches the trap. + const obj = {}; + Object.setPrototypeOf( + obj, + new Proxy( + {}, + { + ownKeys: () => ["x", "foo"], + getOwnPropertyDescriptor: () => ({ configurable: true, enumerable: true, value: 1 }), + get(_, p) { + if (p === "x") throw new Error("trap threw"); + return 1; + }, + }, + ), + ); + expect(Bun.inspect(obj)).toBe("{\n foo: 1,\n}"); +}); + +it("skips a lazy property whose builder throws and keeps walking", async () => { + // Same leak as the `get` trap above, without a Proxy: the Bun object's $ and + // sql builders throw once Symbol is clobbered, and the next property's builder + // must not be entered with that exception still pending. node:util is loaded + // first because Bun.env's custom inspect (Windows) needs it and it no longer + // loads once Symbol is clobbered. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `require("node:util"); + globalThis.Symbol = NaN; + const lines = Bun.inspect(Bun).split("\\n").map(line => line.trim()); + const has = key => lines.some(line => line.startsWith(key + ": ")); + console.log(JSON.stringify({ $: has("$"), sql: has("sql"), Glob: has("Glob"), write: has("write") }));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ $: false, sql: false, Glob: true, write: true }), + stderr: "", + exitCode: 0, + }); +}); + it("prototype", () => { const prototypes = [ Request.prototype,