diff --git a/crates/plugins/src/host.rs b/crates/plugins/src/host.rs index 1b0250399..5d7802d3c 100644 --- a/crates/plugins/src/host.rs +++ b/crates/plugins/src/host.rs @@ -122,7 +122,7 @@ pub const CALL_LIMIT: i32 = -9; /// What `vc_call_result` returns: no call has a result yet. pub const CALL_NO_RESULT: i32 = -10; /// What `vc_call` returns: nothing can be called now (while the module is being loaded, or in a -/// run that isn't a filter's run on a document). +/// run that isn't a filter's run on a document). In a run, this fails it too. pub const CALL_NO_CONTEXT: i32 = -11; /// Most `vc_call` calls one run makes. @@ -370,8 +370,8 @@ pub(crate) struct Run { storage: Option>, storage_calls: usize, storage_bytes: usize, - /// The commands `vc_call` may run (the manifest's `commandAllowlist`), when the run has - /// something to run them ([`Run::with_calls`]); `None` otherwise. + /// The commands `vc_call` may run (the manifest's `commandAllowlist`), once the module has + /// loaded and when the run has something to run them ([`Run::allow_calls`]); `None` otherwise. allowed: Option>, /// How many `vc_call` calls the run made, and the bytes of arguments and results they moved. calls: usize, @@ -416,10 +416,9 @@ impl Run { self } - /// The run, with `vc_call` running the commands in `allowed`. - pub(crate) fn with_calls(mut self, allowed: Vec) -> Self { + /// From now on, `vc_call` runs the commands in `allowed`. + pub(crate) fn allow_calls(&mut self, allowed: Vec) { self.allowed = Some(allowed); - self } /// Records the run's first failed call, `e`, and returns `code`. @@ -434,7 +433,8 @@ impl Run { fn call_request(&mut self, memory: &[u8], id: (usize, usize), args: (usize, usize)) -> std::result::Result<(), i32> { self.call_result = None; if self.allowed.is_none() { - return Err(CALL_NO_CONTEXT); + let why = "vc_call: no command can be called here (while the module loads, or in a run that can't make calls)"; + return Err(self.fail(CALL_NO_CONTEXT, Error::Abi(why.into()))); } let shown = || { id.0.checked_add(id.1) diff --git a/crates/plugins/src/runtime.rs b/crates/plugins/src/runtime.rs index 019aae8ae..f0bd20662 100644 --- a/crates/plugins/src/runtime.rs +++ b/crates/plugins/src/runtime.rs @@ -372,11 +372,12 @@ impl Plugin { total.ok_or_else(|| Error::Limit(SELECTION_TOO_LARGE.into()))?; let deadline = Deadline::new(limits.wall_time_ms); let m = &self.manifest; - let mut run = Run::new(&m.id, m.kind != Kind::Effect, true, deadline).with_storage(crate::registry::storage(&m.id)); - if bridge.is_some() { - run = run.with_calls(m.command_allowlist.clone()); - } + let run = Run::new(&m.id, m.kind != Kind::Effect, true, deadline).with_storage(crate::registry::storage(&m.id)); let mut live = self.instantiate(run, &m.permissions(), report)?; + // Calls can be made from `vc_run` on, not from the start function. + if bridge.is_some() { + live.store.data_mut().run.allow_calls(m.command_allowlist.clone()); + } let out = self.run_live(&mut live, input, params, limits, &deadline, bridge); let state = &mut live.store.data_mut().run; *report = std::mem::take(&mut state.report); diff --git a/crates/plugins/tests/call.rs b/crates/plugins/tests/call.rs index 71f4ca8d1..d362dd379 100644 --- a/crates/plugins/tests/call.rs +++ b/crates/plugins/tests/call.rs @@ -188,13 +188,25 @@ fn cancel_stops_a_run_at_its_next_call() { } /// Without a bridge (a run that isn't a filter's run on a document), and while the module is being -/// loaded, `vc_call` returns [`CALL_NO_CONTEXT`] and the run goes on. +/// loaded, `vc_call` returns [`CALL_NO_CONTEXT`]; in a run, that fails the run as any failed call +/// does, also when the plug-in goes on. #[test] fn without_a_bridge_a_call_returns_no_context() { + let no_context = |out: vectorcraft_plugins::Result>| { + let e = out.unwrap_err(); + assert!(matches!(e, Error::Abi(_)) && e.to_string().contains("no command can be called here"), "{e}"); + }; + // The start function's call gets the code, and the module loads; a run, with a bridge or + // without one, then fails on it, and the bridge is never asked. let start = format!("(func $start {}) (start $start)", expect(&select_all(), CALL_NO_CONTEXT)); - let src = tpl::with_imports(&manifest("test.call.none"), &format!("{CALLS} {start}"), &expect(&select_all(), CALL_NO_CONTEXT), INPUT); + let src = tpl::with_imports(&manifest("test.call.none"), &format!("{CALLS} {start}"), "", INPUT); let p = Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap(); - assert_eq!(p.run(INPUT.as_bytes(), b"{}").unwrap(), INPUT.as_bytes()); + no_context(p.run(INPUT.as_bytes(), b"{}")); + let (out, seen) = run_with(&p, |_| Ok(json!({}))); + no_context(out); + assert!(seen.is_empty()); + // A call from `vc_run` without a bridge. + no_context(calling("test.call.none", &expect(&select_all(), CALL_NO_CONTEXT)).run(INPUT.as_bytes(), b"{}")); } /// A module that imports `vc_call` or `vc_call_result` needs the permission `commands`, which needs diff --git a/docs/plugins.md b/docs/plugins.md index 31185ab1e..12476362c 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -406,11 +406,10 @@ arguments as a JSON object, the params the command takes from any request: `vc_call` returns `-1` when the id or the arguments don't lie inside the module's memory, `-2` when they are not UTF-8, `-3` when the arguments are not a JSON object, `-4` when `commandAllowlist` doesn't list the id, `-5` when no command has the id, `-6` when a plug-in may not call the command, `-7` when the command can't run now (nothing is selected), -`-8` when the command ran and failed, `-9` when a limit below is passed, and `-11` when the run has no document to -call commands on. +`-8` when the command ran and failed, `-9` when a limit below is passed, and `-11` when nothing can be called (from +the module's start function, or in a run that isn't a filter's run on a document). -- Every code but `-11` also fails the run, whatever the plug-in does next, and the error names the call and why it - failed. The run's calls and its output are one undo step, named after the plug-in or its command. A run that fails, +- Every code also fails the run, whatever the plug-in does next, and the error names the call and why it failed. The run's calls and its output are one undo step, named after the plug-in or its command. A run that fails, traps, runs out of budget or is cancelled leaves every document, its selection and its history as they were. - Each call runs on the document the run is for, with its params checked as any request's are, and sees what the calls before it changed: a `select.all` followed by `object.move` moves everything. The filter's own input is the one it