diff --git a/Cargo.lock b/Cargo.lock index f2dadefeaf0..decbb2a2bc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7781,6 +7781,7 @@ dependencies = [ "urlencoding", "uuid", "v8", + "wasmbin", "wasmtime", "wasmtime-internal-fiber", ] diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 3ec0b44e251..80b0aeec1cb 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -164,6 +164,7 @@ pretty_assertions.workspace = true jsonwebtoken.workspace = true axum.workspace = true fs_extra.workspace = true +wasmbin.workspace = true [lints] workspace = true diff --git a/crates/core/src/host/mod.rs b/crates/core/src/host/mod.rs index 0daa9c359bc..8db68931c27 100644 --- a/crates/core/src/host/mod.rs +++ b/crates/core/src/host/mod.rs @@ -16,6 +16,8 @@ mod module_common; #[allow(clippy::too_many_arguments)] pub mod module_host; pub mod scheduler; +#[cfg(test)] +pub(crate) mod test_utils; pub mod wasmtime; // Visible for integration testing. diff --git a/crates/core/src/host/test_utils.rs b/crates/core/src/host/test_utils.rs new file mode 100644 index 00000000000..cb75e8af2eb --- /dev/null +++ b/crates/core/src/host/test_utils.rs @@ -0,0 +1,45 @@ +use crate::database_logger::DatabaseLogger; +use crate::db::relational_db::tests_utils::TestDB; +use crate::energy::NullEnergyMonitor; +use crate::host::Scheduler; +use crate::messages::control_db::{Database, HostType}; +use crate::module_host_context::ModuleCreationContext; +use crate::replica_context::ReplicaContext; +use crate::subscription::module_subscription_actor::ModuleSubscriptions; +use spacetimedb_lib::{Hash, Identity}; +use std::sync::Arc; + +pub(crate) fn module_creation_context_for_test( + host_type: HostType, +) -> (ModuleCreationContext, tokio::runtime::Runtime) { + let TestDB { db, .. } = TestDB::in_memory().expect("failed to make test db"); + let (subscriptions, runtime) = ModuleSubscriptions::for_test_new_runtime(db.clone()); + let logger = { + let _rt = runtime.enter(); + Arc::new(DatabaseLogger::in_memory(64 * 1024)) + }; + let replica_ctx = Arc::new(ReplicaContext { + database: Database { + id: 0, + database_identity: Identity::ZERO, + owner_identity: Identity::ZERO, + host_type, + initial_program: Hash::ZERO, + }, + replica_id: 0, + logger, + subscriptions, + relational_db: db, + }); + let (scheduler, _starter) = Scheduler::open(replica_ctx.relational_db.clone()); + + ( + ModuleCreationContext { + replica_ctx, + scheduler, + program_hash: Hash::ZERO, + energy_monitor: Arc::new(NullEnergyMonitor), + }, + runtime, + ) +} diff --git a/crates/core/src/host/v8/mod.rs b/crates/core/src/host/v8/mod.rs index e21a08e5747..b03fd8eb83a 100644 --- a/crates/core/src/host/v8/mod.rs +++ b/crates/core/src/host/v8/mod.rs @@ -942,12 +942,15 @@ fn extract_description<'scope>( mod test { use super::to_value::test::with_scope; use super::*; + use crate::host::test_utils::module_creation_context_for_test; use crate::host::v8::error::{ErrorOrException, ExceptionThrown}; use crate::host::wasm_common::module_host_actor::ReducerOp; use crate::host::ArgsTuple; + use crate::messages::control_db::HostType; use spacetimedb_lib::{ConnectionId, Identity}; use spacetimedb_primitives::ReducerId; use spacetimedb_schema::reducer_name::ReducerName; + use std::sync::Arc; fn with_module_catch( code: &str, @@ -1054,4 +1057,51 @@ js error Uncaught Error: foobar .map_err(|e| e.to_string()); assert_eq!(raw_mod, Ok(RawModuleDef::V9(<_>::default()))); } + + #[test] + fn fail_multiple_describe_module_exports() { + let code = r#" + import { register_hooks } from "spacetime:sys@1.0"; + register_hooks({ + __call_reducer__: function() {}, + __describe_module__: function() { + return new Uint8Array([0]); + }, + __describe_module_v10__: function() { + return new Uint8Array([0]); + }, + }) + "#; + let err = with_scope(|scope| eval_user_module_catch(scope, code)) + .expect_err("register_hooks should fail") + .to_string(); + assert!(err.contains("cannot register both `__describe_module__` and `__describe_module_v10__` hooks")); + } + + #[test] + fn startup_worker_fails_multiple_describe_module_exports() { + let code: Arc = Arc::from( + r#" + import { register_hooks } from "spacetime:sys@1.0"; + register_hooks({ + __call_reducer__: function() {}, + __describe_module__: function() { + return new Uint8Array([0]); + }, + __describe_module_v10__: function() { + return new Uint8Array([0]); + }, + }) + "#, + ); + + let err = with_scope(|scope| { + let (mcc, _runtime) = module_creation_context_for_test(HostType::Js); + match startup_instance_worker(scope, code, Either::Right(mcc)) { + Ok(_) => panic!("startup should fail"), + Err(err) => err.to_string(), + } + }); + assert!(err.contains("module cannot register both `__describe_module__` and `__describe_module_v10__` hooks")); + } } diff --git a/crates/core/src/host/v8/syscall/hooks.rs b/crates/core/src/host/v8/syscall/hooks.rs index 7a9207ee091..daf305665b6 100644 --- a/crates/core/src/host/v8/syscall/hooks.rs +++ b/crates/core/src/host/v8/syscall/hooks.rs @@ -11,6 +11,10 @@ use crate::host::v8::error::Throwable; use crate::host::v8::error::TypeError; use crate::host::v8::from_value::cast; use crate::host::v8::string::StringConst; +use crate::host::wasm_common::{DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10}; + +const DESCRIBE_MODULE_DUNDER_STR: &StringConst = &StringConst::new(DESCRIBE_MODULE_DUNDER); +const DESCRIBE_MODULE_DUNDER_V10_STR: &StringConst = &StringConst::new(DESCRIBE_MODULE_DUNDER_V10); /// Returns the hook function `name` on `hooks_obj`. pub(super) fn get_hook_function<'scope>( @@ -23,6 +27,23 @@ pub(super) fn get_hook_function<'scope>( cast!(scope, object, Function, "module function hook `{}`", name.as_str()).map_err(|e| e.throw(scope)) } +/// Ensures a module doesn't define both `__describe_module__` and `__describe_module_v10__`. +pub(super) fn validate_describe_hooks<'scope>( + scope: &mut PinScope<'scope, '_>, + hooks_obj: Local<'_, Object>, +) -> ExcResult<()> { + let has_v9_or_earlier = !property(scope, hooks_obj, DESCRIBE_MODULE_DUNDER_STR.string(scope))?.is_undefined(); + let has_v10 = !property(scope, hooks_obj, DESCRIBE_MODULE_DUNDER_V10_STR.string(scope))?.is_undefined(); + if has_v9_or_earlier && has_v10 { + return Err(TypeError(format!( + "module cannot register both `{}` and `{}` hooks", + DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10 + )) + .throw(scope)); + } + Ok(()) +} + /// Registers all the module function `hooks` /// and sets the given `AbiVersion` to `abi`. pub(super) fn set_hook_slots( diff --git a/crates/core/src/host/v8/syscall/v1.rs b/crates/core/src/host/v8/syscall/v1.rs index 113d436fba7..bf1762d6a69 100644 --- a/crates/core/src/host/v8/syscall/v1.rs +++ b/crates/core/src/host/v8/syscall/v1.rs @@ -14,7 +14,7 @@ use super::common::{ procedure_start_mut_tx, row_iter_bsatn_close, table_id_from_name, volatile_nonatomic_schedule_immediate, }; use super::hooks::HookFunctions; -use super::hooks::{get_hook_function, set_hook_slots}; +use super::hooks::{get_hook_function, set_hook_slots, validate_describe_hooks}; use super::{AbiVersion, FnRet, ModuleHookKey}; use crate::host::instance_env::InstanceEnv; use crate::host::wasm_common::instrumentation::span; @@ -311,9 +311,11 @@ fn with_span<'scope, T, E: From>( /// /// Throws a `TypeError` if: /// - `hooks` is not an object that has functions `__describe_module__` and `__call_reducer__`. +/// - `hooks` contains both `__describe_module__` and `__describe_module_v10__`. fn register_hooks_v1_0<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionCallbackArguments<'_>) -> FnRet<'scope> { // Convert `hooks` to an object. let hooks = cast!(scope, args.get(0), Object, "hooks object").map_err(|e| e.throw(scope))?; + validate_describe_hooks(scope, hooks)?; let describe_module = get_hook_function(scope, hooks, str_from_ident!(__describe_module__))?; let call_reducer = get_hook_function(scope, hooks, str_from_ident!(__call_reducer__))?; diff --git a/crates/core/src/host/v8/syscall/v2.rs b/crates/core/src/host/v8/syscall/v2.rs index d160dcb2c50..3374a5153b6 100644 --- a/crates/core/src/host/v8/syscall/v2.rs +++ b/crates/core/src/host/v8/syscall/v2.rs @@ -16,7 +16,7 @@ use super::common::{ procedure_start_mut_tx, row_iter_bsatn_close, table_id_from_name, volatile_nonatomic_schedule_immediate, }; use super::hooks::HookFunctions; -use super::hooks::{get_hook_function, set_hook_slots}; +use super::hooks::{get_hook_function, set_hook_slots, validate_describe_hooks}; use super::{AbiVersion, ModuleHookKey}; use crate::host::instance_env::InstanceEnv; use crate::host::wasm_common::instrumentation::span; @@ -280,9 +280,11 @@ fn with_nothing<'scope, O: JsReturnValue>( /// /// Throws a `TypeError` if: /// - `hooks` is not an object that has functions `__describe_module__` and `__call_reducer__`. +/// - `hooks` contains both `__describe_module__` and `__describe_module_v10__`. fn register_hooks_v2_0<'scope>(scope: &mut PinScope<'scope, '_>, args: FunctionCallbackArguments<'_>) -> ExcResult<()> { // Convert `hooks` to an object. let hooks = cast!(scope, args.get(0), Object, "hooks object").map_err(|e| e.throw(scope))?; + validate_describe_hooks(scope, hooks)?; let describe_module = get_hook_function(scope, hooks, str_from_ident!(__describe_module__))?; let call_reducer = get_hook_function(scope, hooks, str_from_ident!(__call_reducer__))?; diff --git a/crates/core/src/host/wasm_common.rs b/crates/core/src/host/wasm_common.rs index 9e58b8e67e1..33fc5ed422a 100644 --- a/crates/core/src/host/wasm_common.rs +++ b/crates/core/src/host/wasm_common.rs @@ -42,16 +42,28 @@ pub fn detect_raw_def_version(module: &M) -> Result( + get_export: impl Fn(&str) -> Option, +) -> Result { + let has_v9_or_earlier = get_export(DESCRIBE_MODULE_DUNDER).is_some(); + let has_v10 = get_export(DESCRIBE_MODULE_DUNDER_V10).is_some(); + + match (has_v9_or_earlier, has_v10) { + (true, false) => Ok(RawModuleDefVersion::V9OrEarlier), + (false, true) => Ok(RawModuleDefVersion::V10), + (false, false) => Err(module_host_actor::DescribeError::Signature(anyhow::anyhow!( "module does not export a {} or {} function", DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10 - ))) + ))), + (true, true) => Err(module_host_actor::DescribeError::Signature(anyhow::anyhow!( + "module exports both {} and {}; expected exactly one describe function", + DESCRIBE_MODULE_DUNDER, + DESCRIBE_MODULE_DUNDER_V10 + ))), } } /// Returns the describe dunder symbol for a given module version. diff --git a/crates/core/src/host/wasmtime/wasmtime_module.rs b/crates/core/src/host/wasmtime/wasmtime_module.rs index 71aab2c1ffb..45e3337ec83 100644 --- a/crates/core/src/host/wasmtime/wasmtime_module.rs +++ b/crates/core/src/host/wasmtime/wasmtime_module.rs @@ -684,6 +684,14 @@ fn get_memory_size(store: &Store) -> usize { mod tests { use super::*; use crate::energy::EnergyQuanta; + use crate::host::test_utils::module_creation_context_for_test; + use crate::host::wasm_common::module_host_actor::WasmModuleHostActor; + use crate::host::wasm_common::{DESCRIBE_MODULE_DUNDER, DESCRIBE_MODULE_DUNDER_V10}; + use crate::messages::control_db::HostType; + use wasmbin::builtins::Blob; + use wasmbin::indices::{FuncId, TypeId}; + use wasmbin::sections::{Export, ExportDesc, FuncBody}; + use wasmbin::types::{FuncType, ValueType}; #[test] fn test_fuel() { @@ -698,4 +706,59 @@ mod tests { let used = EnergyQuanta::from(budget) - remaining; assert_eq!(used, EnergyQuanta::new(10)); } + + fn module_with_both_describe_exports_wasm() -> Vec { + let module = wasmbin::Module { + sections: vec![ + vec![FuncType { + params: vec![ValueType::I32], + results: vec![], + }] + .into(), + vec![TypeId::from(0), TypeId::from(0)].into(), + vec![ + Export { + name: DESCRIBE_MODULE_DUNDER.to_owned(), + desc: ExportDesc::Func(FuncId::from(0)), + }, + Export { + name: DESCRIBE_MODULE_DUNDER_V10.to_owned(), + desc: ExportDesc::Func(FuncId::from(1)), + }, + ] + .into(), + vec![ + Blob::from(FuncBody { + locals: vec![], + expr: vec![], + }), + Blob::from(FuncBody { + locals: vec![], + expr: vec![], + }), + ] + .into(), + ], + }; + module.encode_into(Vec::new()).expect("failed to encode test module") + } + + #[test] + fn wasm_module_host_actor_fails_multiple_describe_module_exports() { + let wasm = module_with_both_describe_exports_wasm(); + let engine = wasmtime::Engine::new(&wasmtime::Config::new()).unwrap(); + let module = wasmtime::Module::new(&engine, &wasm).expect("failed to compile test module"); + let linker = wasmtime::Linker::new(&engine); + let instance_pre = linker + .instantiate_pre(&module) + .expect("failed to prepare instantiation"); + let module = WasmtimeModule::new(instance_pre); + let (mcc, _runtime) = module_creation_context_for_test(HostType::Wasm); + + let err = match WasmModuleHostActor::new(mcc, module) { + Ok(_) => panic!("initialization should fail"), + Err(err) => err.to_string(), + }; + assert!(err.contains("module exports both __describe_module__ and __describe_module_v10__")); + } }