Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e2df61c
Fix null deref in Bun.sql/Bun.SQL lazy property callbacks
robobun May 4, 2026
c5c1f5c
Also fix Bun.$ PropertyCallback and forEachProperty with Proxy protot…
robobun May 4, 2026
f2190e1
[autofix.ci] apply automated fixes
autofix-ci[bot] May 4, 2026
6630bf6
Use test.concurrent.each for independent subprocess tests
robobun May 4, 2026
da65188
Retrigger CI (previous build had expired agents)
robobun May 4, 2026
aad7f3e
PropertyCallbacks: catch and clear exceptions instead of propagating
robobun May 4, 2026
4639d3c
Merge remote-tracking branch 'origin/main' into farm/8928bc04/fix-sql…
robobun May 12, 2026
e0ef811
Tighten 19650 regression test to assert positive stdout marker
robobun May 12, 2026
756fa7e
Merge remote-tracking branch 'origin/main' into farm/8928bc04/fix-sql…
robobun May 20, 2026
0e7406d
Merge remote-tracking branch 'origin/main' into farm/8928bc04/fix-sql…
robobun May 21, 2026
ae3fc29
Address review: fix get-trap test coverage, report Bun.redis init errors
robobun May 21, 2026
9dffcd1
Merge origin/main into farm/8928bc04/fix-sql-lazy-init-null-deref
robobun Jun 5, 2026
43f57e5
Merge origin/main into farm/8928bc04/fix-sql-lazy-init-null-deref
robobun Jul 12, 2026
10d35e0
test: add issue URL comment to 19650.test.ts
robobun Jul 12, 2026
bf98514
Merge remote-tracking branch 'origin/main' into farm/8928bc04/fix-sql…
robobun Aug 14, 2026
873cddc
test: Bun.redis getter is Rust-backed
robobun Aug 14, 2026
6ecdfcd
Shorten comments on the lazy property fold helpers
robobun Aug 14, 2026
5414053
Drop the Bun object lazy-init changes; JSC now propagates a throwing …
robobun Aug 14, 2026
230e894
test: drain stderr in the lazy property subprocess test
robobun Aug 14, 2026
2a20b8d
test: pin inspect output for the Proxy prototype cases; fix comment o…
robobun Aug 14, 2026
bafa86f
test: inspect keeps walking when a lazy property builder throws mid-walk
robobun Aug 15, 2026
bfb3414
test: load node:util before clobbering Symbol so Bun.env's custom ins…
robobun Aug 15, 2026
ff102fa
Merge remote-tracking branch 'origin/main' into farm/8928bc04/fix-sql…
robobun Aug 17, 2026
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
24 changes: 18 additions & 6 deletions src/jsc/bindings/BunObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,16 @@
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());
if (auto* exception = scope.exception()) {
globalObject->reportUncaughtExceptionAtEventLoop(globalObject, exception);
if (!scope.exception()) throwException(globalObject, scope, exception);
}
#endif
RETURN_IF_EXCEPTION(scope, {});
RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, vm.propertyNames->defaultKeyword));
RETURN_IF_EXCEPTION(scope, jsUndefined());
if (!sqlValue || !sqlValue.isObject()) [[unlikely]] return jsUndefined();

Check notice on line 326 in src/jsc/bindings/BunObject.cpp

View check run for this annotation

Claude / Claude Code Review

constructBunShell has the same empty-JSValue PropertyCallback bug

`constructBunShell` (the `$` entry in the same `bunObjectTable`) has the identical bug: it calls `JSC::call(...)` and then `RETURN_IF_EXCEPTION(scope, {})` / `return {}` after `throwTypeError`, so a stack-overflow during shell init will hand an empty JSValue to `reifyStaticProperty` → `putDirect` and null-deref in `isGetterSetter()`. This is **pre-existing** and not touched by this PR, but since you've established the fix pattern here it'd be nice to apply the same `jsUndefined()` treatment to t
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
JSValue result = sqlValue.getObject()->get(globalObject, vm.propertyNames->defaultKeyword);
RETURN_IF_EXCEPTION(scope, jsUndefined());
return result ? result : jsUndefined();
}

static JSValue constructBunSQLObject(VM& vm, JSObject* bunObject)
Expand All @@ -329,11 +335,17 @@
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());
if (auto* exception = scope.exception()) {
globalObject->reportUncaughtExceptionAtEventLoop(globalObject, exception);
if (!scope.exception()) throwException(globalObject, scope, exception);
}
#endif
RETURN_IF_EXCEPTION(scope, {});
RETURN_IF_EXCEPTION(scope, jsUndefined());
if (!sqlValue || !sqlValue.isObject()) [[unlikely]] return jsUndefined();
auto clientData = WebCore::clientData(vm);
RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, clientData->builtinNames().SQLPublicName()));
JSValue result = sqlValue.getObject()->get(globalObject, clientData->builtinNames().SQLPublicName());
RETURN_IF_EXCEPTION(scope, jsUndefined());
return result ? result : jsUndefined();
}

extern "C" JSC::EncodedJSValue JSPasswordObject__create(JSGlobalObject*);
Expand Down
26 changes: 26 additions & 0 deletions test/js/sql/sql-lazy-init-stack-overflow.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { test, expect } from "bun:test";
import { bunEnv, bunExe } from "harness";

// The lazy property callbacks for Bun.sql / Bun.SQL load the "bun:sql" internal
// module on first access. If that load throws (e.g. stack overflow), the callback
// previously returned an empty JSValue which JSC's reifyStaticProperty passed
// straight into putDirect, crashing on a null JSCell dereference.
test.each(["sql", "SQL"] as const)("accessing Bun.%s near stack overflow does not crash", async key => {

Check warning on line 8 in test/js/sql/sql-lazy-init-stack-overflow.test.ts

View check run for this annotation

Claude / Claude Code Review

Use test.concurrent.each per test/CLAUDE.md convention

nit: Per `test/CLAUDE.md`, tests that spawn independent subprocesses should use `test.concurrent.each` (or `describe.concurrent`) so they run in parallel. The two iterations here have no shared state, so this can be `test.concurrent.each(["sql", "SQL"] as const)(...)`.
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
const src = `
function F() {
try { new F(); } catch {}
Bun.${key};
}
try { new F(); } catch {}
Bun.gc(true);
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stdout: "ignore",
stderr: "ignore",
});
const exitCode = await proc.exited;
expect(proc.signalCode).toBeNull();
expect([0, 1]).toContain(exitCode);
});
Loading