Skip to content
Closed
Show file tree
Hide file tree
Changes from 13 commits
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
13 changes: 10 additions & 3 deletions src/jsc/bindings/BunObject+exports.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,16 @@ FOR_EACH_GETTER(DECLARE_ZIG_BUN_OBJECT_GETTER);
#undef DECLARE_ZIG_BUN_OBJECT_GETTER

// definition of the C++ wrapper to call the Rust function
#define DEFINE_ZIG_BUN_OBJECT_GETTER_WRAPPER(name) static JSC::JSValue BunObject_lazyPropCb_wrap_##name(JSC::VM &vm, JSC::JSObject *object) { \
return JSC::JSValue::decode(BunObject_lazyPropCb_##name(object->globalObject(), object)); \
} \
#define DEFINE_ZIG_BUN_OBJECT_GETTER_WRAPPER(name) static JSC::JSValue BunObject_lazyPropCb_wrap_##name(JSC::VM& vm, JSC::JSObject* object) \
{ \
auto scope = DECLARE_THROW_SCOPE(vm); \
JSC::JSValue result = JSC::JSValue::decode(BunObject_lazyPropCb_##name(object->globalObject(), object)); \
if (scope.exception()) [[unlikely]] { \
(void)scope.tryClearException(); \
return JSC::jsUndefined(); \
} \
return result ? result : JSC::jsUndefined(); \
}
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

FOR_EACH_GETTER(DEFINE_ZIG_BUN_OBJECT_GETTER_WRAPPER);
#undef DEFINE_ZIG_BUN_OBJECT_GETTER_WRAPPER
Expand Down
73 changes: 52 additions & 21 deletions src/jsc/bindings/BunObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,29 +314,52 @@ static JSValue constructPluginObject(VM& vm, JSObject* bunObject)
return pluginFunction;
}

// JSC's reifyStaticProperty passes the PropertyCallback result straight to
// putDirect without checking for exceptions, and JSValue::get asserts
// (!scope.exception() || !hasSlot). So these callbacks must not leave a
// pending exception; if loading the module fails we report it and return
// jsUndefined() so the property is reified to a valid value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimmed to one line in 6ecdfcd; the reasoning is in the PR description.

static JSValue requireBunSqlModule(VM& vm, Zig::GlobalObject* globalObject, JSC::ThrowScope& scope)
{
JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql);
if (auto* exception = scope.exception()) [[unlikely]] {
(void)scope.tryClearException();
globalObject->reportUncaughtExceptionAtEventLoop(globalObject, exception);
(void)scope.tryClearException();
return {};
}
if (!sqlValue || !sqlValue.isObject()) [[unlikely]]
return {};
return sqlValue;
}
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

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));
JSValue sqlValue = requireBunSqlModule(vm, globalObject, scope);
if (!sqlValue) return jsUndefined();
JSValue result = sqlValue.getObject()->get(globalObject, vm.propertyNames->defaultKeyword);
if (scope.exception()) [[unlikely]] {
(void)scope.tryClearException();
return jsUndefined();
}
return result ? result : jsUndefined();
}

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, {});
JSValue sqlValue = requireBunSqlModule(vm, globalObject, scope);
if (!sqlValue) 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());
if (scope.exception()) [[unlikely]] {
(void)scope.tryClearException();
return jsUndefined();
}
return result ? result : jsUndefined();
}

extern "C" JSC::EncodedJSValue JSPasswordObject__create(JSGlobalObject*);
Expand Down Expand Up @@ -365,25 +388,33 @@ static JSValue constructBunShell(VM& vm, JSObject* bunObject)
JSC::JSFunction* createShellFn = JSC::JSFunction::create(vm, globalObject, shellCreateBunShellTemplateFunctionCodeGenerator(vm), globalObject);

auto scope = DECLARE_THROW_SCOPE(vm);
auto reportAndClear = [&]() {
auto* exception = scope.exception();
(void)scope.tryClearException();
if (exception) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, exception);
(void)scope.tryClearException();
};
auto args = JSC::MarkedArgumentBuffer();
args.append(createShellInterpreterFunction);
args.append(createParsedShellScript);
JSC::JSValue shell = JSC::call(globalObject, createShellFn, args, "BunShell"_s);
RETURN_IF_EXCEPTION(scope, {});

if (!shell.isObject()) [[unlikely]] {
throwTypeError(globalObject, scope, "Internal error: BunShell constructor did not return an object"_s);
return {};
if (scope.exception()) [[unlikely]] {
reportAndClear();
return jsUndefined();
}

if (!shell || !shell.isObject()) [[unlikely]]
return jsUndefined();

auto* bunShell = shell.getObject();

auto ShellError = bunShell->get(globalObject, JSC::Identifier::fromString(vm, "ShellError"_s));
RETURN_IF_EXCEPTION(scope, {});
if (!ShellError.isObject()) [[unlikely]] {
throwTypeError(globalObject, scope, "Internal error: BunShell.ShellError is not an object"_s);
return {};
if (scope.exception()) [[unlikely]] {
reportAndClear();
return jsUndefined();
}
if (!ShellError || !ShellError.isObject()) [[unlikely]]
return jsUndefined();

bunShell->putDirectNativeFunction(vm, globalObject, Identifier::fromString(vm, "braces"_s), 1, Generated::BunObject::jsBraces, ImplementationVisibility::Public, NoIntrinsic, JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly | 0);
bunShell->putDirectNativeFunction(vm, globalObject, Identifier::fromString(vm, "escape"_s), 1, BunObject_callback_shellEscape, ImplementationVisibility::Public, NoIntrinsic, JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly | 0);
Expand Down
10 changes: 7 additions & 3 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5225,10 +5225,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
Expand Down Expand Up @@ -5300,7 +5301,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;
Comment thread
robobun marked this conversation as resolved.
}
}

Expand Down
19 changes: 15 additions & 4 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1966,11 +1966,18 @@ pub(crate) fn get_valkey_default_client(global_this: &JSGlobalObject, _: &JSObje

let valkey = match JSValkeyClient::create_no_js_no_pubsub(global_this, &[JSValue::UNDEFINED]) {
Ok(p) => p,
Err(jsc::JsError::Thrown) | Err(jsc::JsError::Terminated) => return JSValue::ZERO,
Err(err @ (jsc::JsError::Thrown | jsc::JsError::Terminated)) => {
// PropertyCallback contract: the C++ wrapper clears any pending
// exception and reifies the slot to `undefined`, so report here
// (like s3_default_client) or the diagnostic is silently dropped.
global_this.report_active_exception_as_unhandled(err);
return JSValue::UNDEFINED;
}
Err(err) => {
let _ =
global_this.throw_error(crate::Error::from(err), "Failed to create Redis client");
return JSValue::ZERO;
global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown);
return JSValue::UNDEFINED;
}
};

Expand All @@ -1982,11 +1989,15 @@ pub(crate) fn get_valkey_default_client(global_this: &JSGlobalObject, _: &JSObje
valkey_ref.this_value.set(jsc::JsRef::init_weak(as_js));
match SubscriptionCtx::init(valkey_ref) {
Ok(ctx) => valkey_ref._subscription_ctx.set(ctx),
Err(jsc::JsError::Thrown) | Err(jsc::JsError::Terminated) => return JSValue::ZERO,
Err(err @ (jsc::JsError::Thrown | jsc::JsError::Terminated)) => {
global_this.report_active_exception_as_unhandled(err);
return JSValue::UNDEFINED;
}
Err(err) => {
let _ =
global_this.throw_error(crate::Error::from(err), "Failed to create Redis client");
return JSValue::ZERO;
global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown);
return JSValue::UNDEFINED;
}
}

Expand Down
42 changes: 42 additions & 0 deletions test/js/bun/util/inspect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,48 @@ import { describe, expect, it } from "bun:test";
import { bunEnv, bunExe, 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)).not.toThrow();
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
});

it("Proxy prototype with throwing get trap does not crash", () => {
// The `get` trap must throw so ProxyObject's performGet throws and
// getPropertySlot returns false with a pending exception — exercising
// the CLEAR_IF_EXCEPTION before the `if (!found) continue` in
// forEachPropertyImpl. ownKeys/getOwnPropertyDescriptor ensure the
// property names are enumerated so the loop reaches the get 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;
},
},
),
);
const out = Bun.inspect(obj);
expect(out).toContain("foo");
});

it("prototype", () => {
const prototypes = [
Request.prototype,
Expand Down
76 changes: 76 additions & 0 deletions test/regression/issue/19650.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";

// Several lazy PropertyCallback entries on the Bun object (Bun.$, Bun.sql,
// Bun.SQL, ...) run JS on first access. If that JS throws (e.g. a stack
// overflow), the callback previously returned an empty JSValue, which JSC's
// reifyStaticProperty passes straight to putDirect without an exception
// check, crashing on a null JSCell dereference.

Check warning on line 8 in test/regression/issue/19650.test.ts

View check run for this annotation

Claude / Claude Code Review

Regression test missing standard issue-URL comment

Consider prepending `// https://github.com/oven-sh/bun/issues/19650` at the top of this file — regression tests under `test/regression/issue/` conventionally start with the issue URL so future readers can find the original report. Not blocking.
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
//
// Each subprocess must write "OK" to stdout after the pathological access
// pattern completes. Without the fix, release builds segfault and ASAN/UBSan
// builds abort on a null-pointer member call, so "OK" is never written.
test.concurrent.each(["$", "sql", "SQL", "postgres"] as const)(
"accessing Bun.%s near stack overflow does not crash",
async key => {
const src = `
function F() {
try { new F(); } catch {}
Bun[${JSON.stringify(key)}];
}
try { new F(); } catch {}
Bun.gc(true);
process.stdout.write("OK");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stdout: "pipe",
stderr: "ignore",
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
expect({ stdout, signalCode: proc.signalCode }).toEqual({ stdout: "OK", signalCode: null });
expect([0, 1]).toContain(exitCode);
},
);

test.concurrent.each(["$", "sql", "SQL", "postgres"] as const)(
"accessing Bun.%s after clobbering Symbol does not crash",
async key => {
// process.stdout is lazily initialized via internal:primordials which
// reads globalThis.Symbol, so take a reference before clobbering it.
const src = `
const stdout = process.stdout;
globalThis.Symbol = NaN;
try { Bun[${JSON.stringify(key)}]; } catch {}
try { Bun[${JSON.stringify(key)}]; } catch {}
Bun.gc(true);
stdout.write("OK");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stdout: "pipe",
stderr: "ignore",
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
expect({ stdout, signalCode: proc.signalCode }).toEqual({ stdout: "OK", signalCode: null });
expect([0, 1]).toContain(exitCode);
},
);

// Bun.redis is a Zig-backed lazy PropertyCallback whose init reads REDIS_URL
// and throws a plain TypeError for an empty/invalid value. Same crash path
// as above; additionally verify the diagnostic isn't silently swallowed.
test.concurrent("accessing Bun.redis with invalid REDIS_URL does not crash and reports the error", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", 'process.stdout.write(String(Bun.redis)); process.stdout.write("OK")'],
env: { ...bunEnv, REDIS_URL: "" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, signalCode: proc.signalCode }).toEqual({ stdout: "undefinedOK", signalCode: null });
expect(stderr).toContain("Invalid URL");
expect([0, 1]).toContain(exitCode);
});
Loading