makepad/platform/script/tests/parse_error_capture.rs
Kevin Boos 6dd0b2c133
Splash: a host-services bridge for mini-apps, plus three VM/parser fixes (#1181)
* Splash: a host-services bridge so isolates can ask for brokered capabilities

Mini-apps are sandboxed hard (fs/run/res stripped, net gated), which also
means they can't do anything real. This adds the one doorway back: a
mod.host module in every isolate whose host.request(service, args, cb)
queues {app_tag, heap_key, req_id, service, args_json} on a thread-local
the EMBEDDING HOST drains and answers (splash_host_respond re-enters the
isolate under the normal budget and calls the callback with {ok, data,
error}). No policy lives in makepad: an undrained request never resolves,
tags are host-assigned (Splash::set_host_tag) so scripts can't spoof who
they are, and host.capabilities() just echoes whatever grant list the
host last pushed (set_host_caps). Callbacks are rooted ScriptFnRefs keyed
by heap, GC'd with the isolate alongside the storage-jail roots.

Also: call_script_fn_with_strings (string args must be minted in the
callee's own heap), 'let host = mod.host' in both Splash prefixes (line
offsets documented per prefix; the net prefix was already one line off),
and mod.cx.quit is now nil'd in isolates - a mini-app could quit the
whole host process with one call.

* script: stop validation from blessing scripts that failed to parse

The parser RECOVERS from errors (dangling else, missing expression), logs
them, sets had_error - which nothing ever read - and hands back a runnable
module. Nothing enters the trap queue, so a host validating with a
captured_errors sink + take_errors() got an empty list and reported
success; three freshly-written mini-apps shipped real parse errors straight
through host_launcher's validate this way, visible only as stray [E] log
lines.

report_error now also records the formatted message on the parser
(ScriptParser::errors), and both eval paths (eval_with_source and the
streaming eval_with_append_source) drain that into bx.captured_errors when
a sink is installed. No sink = logs only, exactly as before. Regression
tests in tests/parse_error_capture.rs, including the exact fn-final
if/else shape that slipped through.

* splash_host: review fixes — is_ok result field, silent surfaces, JSON hardening

Three classes of fixes from an adversarial review of the bridge:

- The result object's success field is now is_ok. 'ok' is the script
  dialect's ok-test KEYWORD, so r.ok never parsed as a field access — every
  callback that read it silently died. A pure-VM regression test
  (fn_ref_callback.rs) now exercises the exact store-callback-then-answer
  flow the bridge uses.

- SplashHostRequest carries may_prompt, set per isolate via
  Splash::set_host_prompts: background surfaces (home-screen widget tiles)
  are marked silent so a host can fail their permission-needing requests
  instead of popping consent dialogs nobody asked for. splash_host_respond
  also reports an outcome now (Delivered / NoCallback / IsolateGone) so
  hosts can log undeliverable answers, and Splash::isolate_heap_key lets a
  host relate a request to a specific widget (IPC fan-out skips the
  sender's own isolate with it).

- heap.to_json hardening: a cyclic object graph (script-buildable, host-
  serialized on every bridge request) recursed to a stack overflow — now a
  depth cap emits null leaves; backslashes were mis-escaped as a single
  backslash (invalid JSON downstream), tab and other control chars weren't
  escaped at all, and a handle serialized as unquoted junk.

* script: a closure's captured varargs must not shadow its own parameters

A call binds positional args by INDEXING the fn object's vec, which holds
declared parameters — but also, past that, any varargs the call received
(unnamed_fn_arg pushes them with a NIL key). A closure captures the scope
it was minted in, so those leftovers ride along ahead of the closure's own
parameters.

Concretely: script timers invoke their callback with one number (the time).
Hand start_timeout a zero-arg closure and that number lands in the scope as
a NIL-keyed vararg; any closure created in that body then binds its FIRST
parameter against the leftover — first failing the typecheck ("arg 0 (nil)
type mismatch: expected number, got object"), and once that was relaxed,
binding the value under the NIL key so the real parameter stayed nil. It
cost a full debug cycle in host_launcher, where every host-service callback
created inside a boot timer silently never ran.

Both binding paths now walk the DECLARED (named) entries in order, so
captured varargs can never be mistaken for a parameter. Regression test in
tests/extra_arg_typecheck.rs reproduces the timer shape exactly.

(Pre-existing and unrelated: widget_tree's test_observe_and_find_single_node
and test_property_patch_no_structural_rebuild fail on upstream dev too.)

* script: stop parse_json silently dropping negative numbers

The tokenizer emits a leading `-` as its own Operator token, and none of
the three JSON value positions (object value, array element, root) had a
case for it. The sign was swallowed — and inside an object the KEY went
with it, because the minus consumed the value slot and the parser resynced
on the next token.

So `{"lat":37.7,"lon":-122.4}` parsed to `{"lat":37.7}`. No error, no
warning, just a missing field. That is how it was found: a mini-app asked
the host where it was, got coordinates with no longitude, and quietly fell
back to a default city. Sub-zero temperatures and negative UTC offsets
(New York is -14400) were being dropped the same way.

A pending-sign flag is applied to the next number in all three positions.
Bare scalar roots stay unsupported ("42" never parsed either) — separate
pre-existing gap, not touched here. Tests in
platform/script/tests/json_negative_numbers.rs.

* splash_storage: let the host raise a single isolate's jail quota

The jail's 16MB whole-app cap is a constant, so "this app may keep more
than the standard amount" had nowhere to live. A per-heap quota map beside
SANDBOX_ROOTS gives the host one, set through Splash::set_storage_quota
and cleared with the isolate like every other per-isolate binding. Script
still can't see or raise its own cap.

Lowering a quota never deletes anything — it just stops further growth —
so revoking the grant is safe on an app that already wrote past the
default.

host_launcher uses this for a `storage-large` permission (64MB), which is
the point: a capability the user can revoke and have it actually mean
something.
2026-08-19 20:06:01 +02:00

102 lines
3.3 KiB
Rust

//! Regression tests: parse errors must reach a captured-diagnostics sink.
//!
//! Empirically (host_launcher, 2026-08-14): the parser RECOVERS from errors
//! like a dangling `else` in expression position — it logs, sets `had_error`,
//! and still produces a runnable module. Nothing entered the trap queue, so a
//! validating host (`captured_errors` sink + `take_errors`) reported SUCCESS
//! for scripts that failed to parse, and broken mini-apps sailed through
//! validation with their errors only in the log.
use makepad_script::*;
fn test_vm() -> ScriptVm<'static> {
let host = Box::leak(Box::new(0i32));
let std = Box::leak(Box::new(0i32));
ScriptVm {
host,
std,
bx: Box::new(ScriptVmBase::new()),
}
}
fn eval_captured(vm: &mut ScriptVm, name: &str, code: &str) -> Vec<String> {
vm.bx.captured_errors = Some(Vec::new());
vm.with_instruction_limit(500_000, |vm| {
vm.eval(ScriptMod {
cargo_manifest_path: String::new(),
module_path: String::new(),
file: format!("parse_error_capture_{name}"),
line: 0,
column: 0,
code: code.to_string(),
values: vec![],
})
});
vm.take_errors()
}
/// The exact shape that sailed through host_launcher's validation while
/// failing to parse (isolation_probe's svc_result, 2026-08-14): a fn whose
/// FINAL statement is an if with call-statement branches and the `else` on
/// its own line. The parser reports "Unexpected else" and recovers; the sink
/// must see it.
const DANGLING_ELSE: &str = "\
fn svc_result(r){
let ok = r.ok
if ok {
ui.a.set_text(\"A\")
ui.b.set_text(\"A\")
}
else {
ui.a.set_text(\"B\")
ui.b.set_text(\"B\")
}
}
1 + 2";
#[test]
fn dangling_else_reaches_the_sink() {
let mut vm = test_vm();
let errors = eval_captured(&mut vm, "dangling_else", DANGLING_ELSE);
assert!(
errors.iter().any(|e| e.contains("Unexpected else")),
"parse error missing from captured sink: {errors:?}"
);
}
/// A clean script contributes nothing. (Ends on a parenthesized expression:
/// a final `let` and a final bare ident each trip unrelated quirks.)
#[test]
fn clean_parse_captures_nothing() {
let mut vm = test_vm();
let errors = eval_captured(&mut vm, "clean", "let x = 1 + 2\n(x + 1)");
assert!(errors.is_empty(), "unexpected errors: {errors:?}");
}
/// The append/streaming eval path surfaces parse errors the same way.
#[test]
fn streaming_eval_surfaces_parse_errors() {
let mut vm = test_vm();
vm.bx.captured_errors = Some(Vec::new());
let code = DANGLING_ELSE;
vm.with_instruction_limit(500_000, |vm| {
vm.eval_with_append_source(
ScriptMod {
cargo_manifest_path: String::new(),
module_path: "stream#1".to_string(),
file: "parse_error_capture_stream".to_string(),
line: 0,
column: 0,
code: String::new(),
values: vec![],
},
code,
NIL.into(),
)
});
let errors = vm.take_errors();
assert!(
errors.iter().any(|e| e.contains("Unexpected else")),
"streaming parse error missing from captured sink: {errors:?}"
);
}