From 2eb727a30c941004c9df89a8d3d7da7030a30593 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 2 Sep 2026 06:48:55 +0000 Subject: [PATCH 1/6] bake: check for exceptions in the production build's module loader hooks and helpers --- src/runtime/bake/BakeGlobalObject.cpp | 33 ++---- src/runtime/bake/BakeSourceProvider.cpp | 83 ++++++++++---- src/runtime/bake/production.rs | 139 +++++++++++++----------- test/bake/dev/production.test.ts | 113 ++++++++++++++++++- test/no-validate-exceptions.txt | 1 - 5 files changed, 262 insertions(+), 107 deletions(-) diff --git a/src/runtime/bake/BakeGlobalObject.cpp b/src/runtime/bake/BakeGlobalObject.cpp index fea61849fa79..14fc054ab720 100644 --- a/src/runtime/bake/BakeGlobalObject.cpp +++ b/src/runtime/bake/BakeGlobalObject.cpp @@ -22,37 +22,30 @@ 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 what JSModuleLoader::importModule + // itself does. 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 promise; - } BunString refererBunString = Bun::toString(refererString); BunString keyBunString = Bun::toString(keyString); BunString result = BakeProdResolve(global, &refererBunString, &keyBunString); RETURN_IF_EXCEPTION(scope, nullptr); - return JSC::importModule(global, JSC::Identifier::fromString(vm, result.transferToWTFString()), - JSC::Identifier(), WTF::move(parameters), nullptr); + RELEASE_AND_RETURN(scope, JSC::importModule(global, JSC::Identifier::fromString(vm, result.transferToWTFString()), 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, @@ -95,7 +88,7 @@ JSC::Identifier bakeModuleLoaderResolve(JSC::JSGlobalObject* jsGlobal, } } - return Zig::GlobalObject::moduleLoaderResolve(jsGlobal, loader, key, referrer, WTF::move(origin), useImportMap); + RELEASE_AND_RETURN(scope, Zig::GlobalObject::moduleLoaderResolve(jsGlobal, loader, key, referrer, WTF::move(origin), useImportMap)); } static JSC::JSPromise* rejectedInternalPromise(JSC::JSGlobalObject* globalObject, JSC::JSValue value) @@ -172,14 +165,12 @@ JSC::JSPromise* bakeModuleLoaderFetch(JSC::JSGlobalObject* globalObject, #endif JSString* bakePrefixRemovedString = jsNontrivialString(vm, bakePrefixRemoved); JSValue bakePrefixRemovedJsvalue = bakePrefixRemovedString; - return Zig::GlobalObject::moduleLoaderFetch(globalObject, loader, bakePrefixRemovedJsvalue, referrer, WTF::move(parameters), WTF::move(script)); + RELEASE_AND_RETURN(scope, Zig::GlobalObject::moduleLoaderFetch(globalObject, loader, bakePrefixRemovedJsvalue, referrer, WTF::move(parameters), WTF::move(script))); } return rejectedInternalPromise(globalObject, createTypeError(globalObject, "BakeGlobalObject does not have per-thread data configured"_s)); } - auto result = Zig::GlobalObject::moduleLoaderFetch(globalObject, loader, key, referrer, WTF::move(parameters), WTF::move(script)); - RETURN_IF_EXCEPTION(scope, rejectedInternalPromise(globalObject, scope.exception()->value())); - return result; + RELEASE_AND_RETURN(scope, Zig::GlobalObject::moduleLoaderFetch(globalObject, loader, key, referrer, WTF::move(parameters), WTF::move(script))); } GlobalObject* GlobalObject::create(JSC::VM& vm, JSC::Structure* structure, diff --git a/src/runtime/bake/BakeSourceProvider.cpp b/src/runtime/bake/BakeSourceProvider.cpp index 75995e3a6aab..95eb15bcbc88 100644 --- a/src/runtime/bake/BakeSourceProvider.cpp +++ b/src/runtime/bake/BakeSourceProvider.cpp @@ -24,6 +24,9 @@ extern "C" BunString BakeSourceProvider__getSourceSlice(SourceProvider* provider return Bun::toStringView(provider->source()); } +// The EncodedJSValue-returning entry points below return empty if and only if +// an exception is pending (Rust calls them through `jsc::from_js_host_call`). + 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 +54,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(JSC::JSGlobalObject* 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 +124,71 @@ 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); +// `keyValue` must name a module whose evaluation promise has already settled. +// 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 keyIdent = JSC::Identifier::fromString(vm, key->value(global)); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSString* key = uncheckedDowncast(keyValue); + String keyString = key->value(global); + RETURN_IF_EXCEPTION(scope, nullptr); + + auto keyIdent = JSC::Identifier::fromString(vm, keyString); 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 +// `bun_core::ffi::FfiSlice`: a borrowed `&[u8]` passed by value. +struct BakeModuleNamespaceKey { + const unsigned char* ptr; + size_t len; +}; + +// `moduleNamespaceValue` must be a namespace object from BakeGetModuleNamespace. extern "C" JSC::EncodedJSValue BakeGetOnModuleNamespace( JSC::JSGlobalObject* global, - JSC::JSModuleNamespaceObject* moduleNamespace, - const unsigned char* key, - size_t keyLength + JSC::EncodedJSValue moduleNamespaceValue, + BakeModuleNamespaceKey key ) { auto& vm = JSC::getVM(global); - const auto propertyString = String(StringImpl::createWithoutCopying({ key, keyLength })); + auto scope = DECLARE_THROW_SCOPE(vm); + + auto* moduleNamespace = uncheckedDowncast(JSC::JSValue::decode(moduleNamespaceValue)); + const auto propertyString = String(StringImpl::createWithoutCopying({ key.ptr, key.len })); 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); } } // namespace Bake diff --git a/src/runtime/bake/production.rs b/src/runtime/bake/production.rs index 8e69effd2005..f3e05c7703dd 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!( @@ -846,17 +847,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.", @@ -866,34 +860,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 }; @@ -1200,13 +1183,13 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result< } /// unsafe function, must be run outside of the event loop -/// quits the process on exception +/// fails with `JSError` on exception fn load_module( vm: *mut VirtualMachine, 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!(), @@ -1233,38 +1216,64 @@ 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 if and only if it +/// threw, so every call goes through `from_js_host_call`. +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; -} - -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; + safe fn BakeLoadModuleByKey(global: &JSGlobalObject, key: JSValue) -> JSValue; + safe fn BakeGetModuleNamespace(global: &JSGlobalObject, key: JSValue) -> JSValue; + safe fn BakeGetDefaultExportFromModule(global: &JSGlobalObject, key: JSValue) -> JSValue; + // `key` is borrowed for the call (C++ builds an `Identifier` from it). + safe fn BakeGetOnModuleNamespace( + global: &JSGlobalObject, + module_namespace: JSValue, + key: bun_core::ffi::FfiSlice<'_, u8>, + ) -> JSValue; + } + + /// Starts loading and evaluating the module that `key` (a JSString) names + /// and returns its `JSInternalPromise`. + pub(super) fn bake_load_module_by_key( + global: &JSGlobalObject, + key: JSValue, + ) -> JsResult { + jsc::from_js_host_call(global, || BakeLoadModuleByKey(global, key)) + } + + /// The promise from [`bake_load_module_by_key`] for `key` must have settled. + pub(super) fn bake_get_module_namespace( + global: &JSGlobalObject, + key: JSValue, + ) -> JsResult { + jsc::from_js_host_call(global, || BakeGetModuleNamespace(global, key)) + } + + /// The module `key` names must already be evaluated. + pub(super) fn bake_get_default_export_from_module( + global: &JSGlobalObject, + key: JSValue, + ) -> JsResult { + jsc::from_js_host_call(global, || BakeGetDefaultExportFromModule(global, key)) + } + + /// Reads `property` off a module namespace object returned by + /// [`bake_get_module_namespace`]. + pub(super) fn bake_get_on_module_namespace( + global: &JSGlobalObject, + module_namespace: JSValue, + property: &[u8], + ) -> JsResult { + jsc::from_js_host_call(global, || { + BakeGetOnModuleNamespace(global, module_namespace, property.into()) + }) } - // 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 994f8d412240..7b7c8d8139c4 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, isWindows, tempDir } from "harness"; import path from "path"; import { tempDirWithBakeDeps } from "../bake-harness"; @@ -659,4 +659,115 @@ export default function IndexPage() { // Verify NO JavaScript imports are included in the HTML expect(htmlContent).not.toContain('