From cb749736c8f7ea49c6f7add9b73a99946734b4f1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:12:08 +0000 Subject: [PATCH 1/7] bake: check for exceptions in the production build's module helpers BakeGetModuleNamespace, BakeGetDefaultExportFromModule, BakeGetOnModuleNamespace and BakeLoadModuleByKey called throwing JSC APIs (getModuleNamespaceObject, JSObject::get, loadAndEvaluateModule) without a throw scope or an exception check, and production.rs used their results without checking either, so every bun build --app aborted under BUN_JSC_validateExceptionChecks=1 while loading the config file. bakeModuleLoaderImportModule returned JSC::importModule's result from inside a scope it never released, which aborted the same way once a bundled module called import(). BakeLoadInitialServerCode returned the runtime init function's (undefined) result when it threw, so its Rust caller could not see the exception. Give each helper a throw scope, return empty iff an exception is pending, wrap them in from_js_host_call on the Rust side and propagate the error. Take production.test.ts off the exception validation skip list and add validated builds covering the config and prerender paths. --- src/runtime/bake/BakeGlobalObject.cpp | 21 ++-- src/runtime/bake/BakeSourceProvider.cpp | 66 ++++++++--- src/runtime/bake/production.rs | 148 ++++++++++++++---------- test/bake/dev/production.test.ts | 95 ++++++++++++++- test/no-validate-exceptions.txt | 1 - 5 files changed, 240 insertions(+), 91 deletions(-) diff --git a/src/runtime/bake/BakeGlobalObject.cpp b/src/runtime/bake/BakeGlobalObject.cpp index 766b100614fe..3baabb02962f 100644 --- a/src/runtime/bake/BakeGlobalObject.cpp +++ b/src/runtime/bake/BakeGlobalObject.cpp @@ -22,35 +22,36 @@ bakeModuleLoaderImportModule(JSC::JSGlobalObject* global, bool deferred) { UNUSED_PARAM(deferred); + auto& vm = JSC::getVM(global); + auto scope = DECLARE_THROW_SCOPE(vm); + + // Returning nullptr with an exception pending is fine: globalFuncImportModule rejects the import() promise with it. WTF::String keyString = moduleNameValue->getString(global); + RETURN_IF_EXCEPTION(scope, nullptr); if (keyString.startsWith("bake:/"_s)) { - auto& vm = JSC::getVM(global); - return JSC::importModule(global, JSC::Identifier::fromString(vm, keyString), - JSC::Identifier(), WTF::move(parameters), nullptr); + RELEASE_AND_RETURN(scope, JSC::importModule(global, JSC::Identifier::fromString(vm, keyString), + JSC::Identifier(), WTF::move(parameters), nullptr)); } if (!sourceOrigin.isNull() && sourceOrigin.string().startsWith("bake:/"_s)) { - auto& vm = JSC::getVM(global); - auto scope = DECLARE_THROW_SCOPE(vm); - WTF::String refererString = sourceOrigin.string(); - WTF::String keyString = moduleNameValue->getString(global); if (!keyString) { auto promise = JSC::JSPromise::create(vm, global->promiseStructure()); promise->reject(vm, JSC::createError(global, "import() requires a string"_s)); + RETURN_IF_EXCEPTION(scope, nullptr); return promise; } BunString result = BakeProdResolve(global, Bun::toString(refererString), Bun::toString(keyString)); RETURN_IF_EXCEPTION(scope, nullptr); - return JSC::importModule(global, JSC::Identifier::fromString(vm, result.toWTFString()), - JSC::Identifier(), WTF::move(parameters), nullptr); + RELEASE_AND_RETURN(scope, JSC::importModule(global, JSC::Identifier::fromString(vm, result.toWTFString()), + JSC::Identifier(), WTF::move(parameters), nullptr)); } // TODO: make static cast instead of jscast - return uncheckedDowncast(global)->moduleLoaderImportModule(global, moduleLoader, moduleNameValue, WTF::move(parameters), sourceOrigin, false); + RELEASE_AND_RETURN(scope, uncheckedDowncast(global)->moduleLoaderImportModule(global, moduleLoader, moduleNameValue, WTF::move(parameters), sourceOrigin, false)); } JSC::Identifier bakeModuleLoaderResolve(JSC::JSGlobalObject* jsGlobal, diff --git a/src/runtime/bake/BakeSourceProvider.cpp b/src/runtime/bake/BakeSourceProvider.cpp index b337c9f3f218..2963d7c56480 100644 --- a/src/runtime/bake/BakeSourceProvider.cpp +++ b/src/runtime/bake/BakeSourceProvider.cpp @@ -24,6 +24,8 @@ extern "C" BunString BakeSourceProvider__getSourceSlice(SourceProvider* provider return Bun::toStringView(provider->source()); } +// Rust calls the EncodedJSValue-returning functions below via `jsc::from_js_host_call`: empty return <=> exception pending. + extern "C" JSC::EncodedJSValue BakeLoadInitialServerCode(JSC::JSGlobalObject* global, BunString source, bool separateSSRGraph) { auto& vm = JSC::getVM(global); auto scope = DECLARE_THROW_SCOPE(vm); @@ -51,11 +53,24 @@ extern "C" JSC::EncodedJSValue BakeLoadInitialServerCode(JSC::JSGlobalObject* gl args.append(JSC::jsBoolean(separateSSRGraph)); // separateSSRGraph args.append(Zig::ImportMetaObject::create(global, "bake://server-runtime.js"_s)); // importMeta - RELEASE_AND_RETURN(scope, JSC::JSValue::encode(JSC::profiledCall(global, JSC::ProfilingReason::API, fn, callData, JSC::jsUndefined(), args))); + // `JSC::call` returns undefined (not empty) when the callee throws. + JSC::JSValue result = JSC::profiledCall(global, JSC::ProfilingReason::API, fn, callData, JSC::jsUndefined(), args); + RETURN_IF_EXCEPTION(scope, {}); + return JSC::JSValue::encode(result); } -extern "C" JSC::JSPromise* BakeLoadModuleByKey(GlobalObject* global, JSC::JSString* key) { - return JSC::loadAndEvaluateModule(global, key->getString(global), nullptr, nullptr); +extern "C" JSC::EncodedJSValue BakeLoadModuleByKey(GlobalObject* global, JSC::EncodedJSValue keyValue) { + auto& vm = JSC::getVM(global); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSString* key = uncheckedDowncast(JSC::JSValue::decode(keyValue)); + String keyString = key->getString(global); + RETURN_IF_EXCEPTION(scope, {}); + + JSC::JSPromise* promise = JSC::loadAndEvaluateModule(global, keyString, nullptr, nullptr); + RETURN_IF_EXCEPTION(scope, {}); + ASSERT(promise); + return JSC::JSValue::encode(promise); } extern "C" JSC::EncodedJSValue BakeLoadServerHmrPatch(GlobalObject* global, BunString source) { @@ -108,42 +123,63 @@ extern "C" JSC::EncodedJSValue BakeLoadServerHmrPatchWithSourceMap(GlobalObject* return JSC::JSValue::encode(result); } -extern "C" JSC::EncodedJSValue BakeGetModuleNamespace( - JSC::JSGlobalObject* global, - JSC::JSValue keyValue -) { - JSC::JSString* key = uncheckedDowncast(keyValue); +// Returns nullptr if and only if an exception is pending. +static JSC::JSModuleNamespaceObject* getModuleNamespace(JSC::JSGlobalObject* global, JSC::JSValue keyValue) { auto& vm = JSC::getVM(global); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSString* key = uncheckedDowncast(keyValue); auto keyIdent = JSC::Identifier::fromString(vm, key->value(global)); + RETURN_IF_EXCEPTION(scope, nullptr); + auto* entry = global->moduleLoader()->registryEntry(keyIdent); - ASSERT(entry); // should have called BakeLoadServerCode and wait for that promise + ASSERT(entry); // should have called BakeLoadModuleByKey and waited for that promise auto* module = entry ? entry->record() : nullptr; ASSERT(module); JSC::JSModuleNamespaceObject* namespaceObject = global->moduleLoader()->getModuleNamespaceObject(global, module); + RETURN_IF_EXCEPTION(scope, nullptr); ASSERT(namespaceObject); - return JSC::JSValue::encode(namespaceObject); + return namespaceObject; +} + +extern "C" JSC::EncodedJSValue BakeGetModuleNamespace( + JSC::JSGlobalObject* global, + JSC::EncodedJSValue keyValue +) { + return JSC::JSValue::encode(getModuleNamespace(global, JSC::JSValue::decode(keyValue))); } extern "C" JSC::EncodedJSValue BakeGetDefaultExportFromModule( JSC::JSGlobalObject* global, - JSC::JSValue keyValue + JSC::EncodedJSValue keyValue ) { auto& vm = JSC::getVM(global); - return JSC::JSValue::encode(uncheckedDowncast(JSC::JSValue::decode(BakeGetModuleNamespace(global, keyValue)))->get(global, vm.propertyNames->defaultKeyword)); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSModuleNamespaceObject* namespaceObject = getModuleNamespace(global, JSC::JSValue::decode(keyValue)); + RETURN_IF_EXCEPTION(scope, {}); + + JSC::JSValue defaultExport = namespaceObject->get(global, vm.propertyNames->defaultKeyword); + RETURN_IF_EXCEPTION(scope, {}); + return JSC::JSValue::encode(defaultExport); } -// There were issues when trying to use JSValue.get from zig extern "C" JSC::EncodedJSValue BakeGetOnModuleNamespace( JSC::JSGlobalObject* global, - JSC::JSModuleNamespaceObject* moduleNamespace, + JSC::EncodedJSValue moduleNamespaceValue, const unsigned char* key, size_t keyLength ) { auto& vm = JSC::getVM(global); + auto scope = DECLARE_THROW_SCOPE(vm); + + auto* moduleNamespace = uncheckedDowncast(JSC::JSValue::decode(moduleNamespaceValue)); const auto propertyString = String(StringImpl::createWithoutCopying({ key, keyLength })); const auto identifier = JSC::Identifier::fromString(vm, propertyString); const auto property = JSC::PropertyName(identifier); - return JSC::JSValue::encode(moduleNamespace->get(global, property)); + JSC::JSValue value = moduleNamespace->get(global, property); + RETURN_IF_EXCEPTION(scope, {}); + return JSC::JSValue::encode(value); } extern "C" JSC::EncodedJSValue BakeRegisterProductionChunk(JSC::JSGlobalObject* global, BunString virtualPathName, BunString source) { diff --git a/src/runtime/bake/production.rs b/src/runtime/bake/production.rs index 81c32d956928..977d202156a9 100644 --- a/src/runtime/bake/production.rs +++ b/src/runtime/bake/production.rs @@ -355,10 +355,11 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result< { Unwrapped::Pending => unreachable!(), Unwrapped::Fulfilled(_) => { - let default = BakeGetDefaultExportFromModule( + let default = c::bake_get_default_export_from_module( global, config_entry_point_string.to_js(global).map_err(js_err)?, - ); + ) + .map_err(js_err)?; if !default.is_object() { return Err(js_err(global.throw_invalid_arguments(format_args!( @@ -843,17 +844,10 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result< let server_file = router_type.server_file; let server_entry_point = pt.load_bundled_module(server_file)?; - let server_render_func = 'brk: { - let Some(raw) = bake_get_on_module_namespace(global, server_entry_point, b"prerender") - else { - break 'brk None; - }; - if !raw.is_callable() { - break 'brk None; - } - break 'brk Some(raw); - }; - let Some(server_render_func) = server_render_func else { + let server_render_func = + c::bake_get_on_module_namespace(global, server_entry_point, b"prerender") + .map_err(js_err)?; + if !server_render_func.is_callable() { bun_core::err_generic!("Framework does not support static site generation"); bun_core::note!( "The file {} is missing the \"prerender\" export, which defines how to generate static files.", @@ -863,34 +857,23 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result< )) ); Global::crash(); - }; + } let server_param_func = if router.dynamic_routes.count() > 0 { - let f = 'brk: { - let Some(raw) = - bake_get_on_module_namespace(global, server_entry_point, b"getParams") - else { - break 'brk None; - }; - if !raw.is_callable() { - break 'brk None; - } - break 'brk Some(raw); - }; - match f { - Some(f) => f, - None => { - bun_core::err_generic!("Framework does not support static site generation"); - bun_core::note!( - "The file {} is missing the \"getParams\" export, which defines how to generate static files.", - bun_core::fmt::quote(resolve_path::relative( - cwd, - pt.input_file(server_file).abs_path() - )) - ); - Global::crash(); - } + let f = c::bake_get_on_module_namespace(global, server_entry_point, b"getParams") + .map_err(js_err)?; + if !f.is_callable() { + bun_core::err_generic!("Framework does not support static site generation"); + bun_core::note!( + "The file {} is missing the \"getParams\" export, which defines how to generate static files.", + bun_core::fmt::quote(resolve_path::relative( + cwd, + pt.input_file(server_file).abs_path() + )) + ); + Global::crash(); } + f } else { JSValue::NULL }; @@ -1225,7 +1208,7 @@ fn load_module( global: &JSGlobalObject, key: JSValue, ) -> crate::Result { - let promise_value = BakeLoadModuleByKey(global, key); + let promise_value = c::bake_load_module_by_key(global, key).map_err(js_err)?; let promise: *mut jsc::JSInternalPromise = match promise_value.as_any_promise().unwrap() { AnyPromise::Internal(p) => p, AnyPromise::Normal(_) => unreachable!(), @@ -1252,38 +1235,75 @@ fn load_module( let jsc_vm = vm_ref.as_mut().jsc_vm_mut(); match jsc::JSInternalPromise::opaque_mut(promise).unwrap(jsc_vm, UnwrapMode::MarkHandled) { Unwrapped::Pending => unreachable!(), - Unwrapped::Fulfilled(_) => Ok(BakeGetModuleNamespace(global, key)), + Unwrapped::Fulfilled(_) => c::bake_get_module_namespace(global, key).map_err(js_err), Unwrapped::Rejected(err) => Err(js_err(vm_ref.global().throw_value(err))), } } -// extern apis: +/// BakeSourceProvider.cpp entry points; each returns empty iff it threw, as +/// `from_js_host_call` requires. +mod c { + use super::*; -unsafe extern "C" { - safe fn BakeGetDefaultExportFromModule(global: &JSGlobalObject, key: JSValue) -> JSValue; - safe fn BakeGetModuleNamespace(global: &JSGlobalObject, key: JSValue) -> JSValue; - safe fn BakeLoadModuleByKey(global: &JSGlobalObject, key: JSValue) -> JSValue; -} + /// Returns the promise for loading and evaluating the module named by the `key` JSString. + pub(super) fn bake_load_module_by_key( + global: &JSGlobalObject, + key: JSValue, + ) -> JsResult { + unsafe extern "C" { + safe fn BakeLoadModuleByKey(global: &JSGlobalObject, key: JSValue) -> JSValue; + } + jsc::from_js_host_call(global, || BakeLoadModuleByKey(global, key)) + } -fn bake_get_on_module_namespace( - global: &JSGlobalObject, - module: JSValue, - property: &[u8], -) -> Option { - unsafe extern "C" { - // PRECONDITION: `ptr` must be readable for `len` bytes (C++ builds an - // `Identifier` from the slice). Cannot be `safe fn` — raw ptr+len pair - // carries a caller-side validity precondition. - #[link_name = "BakeGetOnModuleNamespace"] - fn f(global: *const JSGlobalObject, module: JSValue, ptr: *const u8, len: usize) - -> JSValue; + /// The module's promise from [`bake_load_module_by_key`] must already be fulfilled. + pub(super) fn bake_get_module_namespace( + global: &JSGlobalObject, + key: JSValue, + ) -> JsResult { + unsafe extern "C" { + safe fn BakeGetModuleNamespace(global: &JSGlobalObject, key: JSValue) -> JSValue; + } + jsc::from_js_host_call(global, || BakeGetModuleNamespace(global, key)) + } + + /// The module's evaluation promise must already be fulfilled. + pub(super) fn bake_get_default_export_from_module( + global: &JSGlobalObject, + key: JSValue, + ) -> JsResult { + unsafe extern "C" { + safe fn BakeGetDefaultExportFromModule( + global: &JSGlobalObject, + key: JSValue, + ) -> JSValue; + } + jsc::from_js_host_call(global, || BakeGetDefaultExportFromModule(global, key)) + } + + /// `module_namespace` comes from [`bake_get_module_namespace`]. + pub(super) fn bake_get_on_module_namespace( + global: &JSGlobalObject, + module_namespace: JSValue, + property: &[u8], + ) -> JsResult { + unsafe extern "C" { + // PRECONDITION: `ptr` must be readable for `len` bytes (C++ builds an + // `Identifier` from the slice). Cannot be `safe fn` — raw ptr+len pair + // carries a caller-side validity precondition. + fn BakeGetOnModuleNamespace( + global: &JSGlobalObject, + module_namespace: JSValue, + ptr: *const u8, + len: usize, + ) -> JSValue; + } + // SAFETY: `property.as_ptr()`/`len()` describe a borrowed `&[u8]` that + // outlives the call, which discharges the ptr+len precondition above. + jsc::from_js_host_call(global, || unsafe { + BakeGetOnModuleNamespace(global, module_namespace, property.as_ptr(), property.len()) + }) } - // SAFETY: `global` is a live `&JSGlobalObject`, `module` is a stack-held - // `JSValue`, and `property.as_ptr()`/`len()` describe a valid borrowed - // `&[u8]` for the call duration — discharges the ptr+len precondition above. - let result: JSValue = unsafe { f(global, module, property.as_ptr(), property.len()) }; - debug_assert!(!result.is_empty()); - Some(result) } // Renders all routes for static site generation by calling the JavaScript implementation. diff --git a/test/bake/dev/production.test.ts b/test/bake/dev/production.test.ts index 1335e3244c00..5386e00b3a7e 100644 --- a/test/bake/dev/production.test.ts +++ b/test/bake/dev/production.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { existsSync } from "fs"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, tempDir } from "harness"; import path from "path"; import { tempDirWithBakeDeps } from "../bake-harness"; @@ -594,4 +594,97 @@ export default function IndexPage() { // Verify NO JavaScript imports are included in the HTML expect(htmlContent).not.toContain('