From 7c28138ce3ca14edf62990880157eb609e8802d6 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 19:57:39 -0800 Subject: [PATCH] Commands: pointer packets are read whole before any event runs A headless or control channel pointer packet now checks every event's kind, coordinates and mods before it sends any of them, so a malformed later event no longer leaves the earlier ones applied. A mods value that is not an object, such as an array, is refused. Rotate View takes null mods as omitted. The MCP docs name the params that refuse null. --- crates/mcp/src/headless.rs | 37 +++++++++++++------------- crates/mcp/src/tests.rs | 5 ++++ crates/ui-egui/src/control.rs | 29 +++++++++++--------- crates/ui-egui/src/control/tests.rs | 24 +++++++++++++++++ crates/ui-egui/src/tests/rotateview.rs | 3 ++- docs/mcp.md | 4 ++- 6 files changed, 70 insertions(+), 32 deletions(-) diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index 0377cd5ae..cf150623e 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -71,7 +71,7 @@ fn opt_bool(p: &Value, k: &str) -> Result, String> { /// A pointer packet's `mods`: none when omitted or null, else an object of booleans. fn packet_mods(p: &Value) -> Result, String> { - opt(p, "mods", |m| serde_json::from_value(m.clone()).ok(), "an object of true or false values") + opt(p, "mods", |m| m.as_object().and_then(|_| serde_json::from_value(m.clone()).ok()), "an object of true or false values") } pub(crate) fn pointer_kind(k: &str) -> Option { @@ -231,25 +231,26 @@ impl Headless { let base = packet_mods(p)?.unwrap_or_default(); let default = PointerEvent::json_backquote(p, false)?; let held = events.iter().map(|e| PointerEvent::json_backquote(e, default)).collect::, _>>()?; - let mods = events.iter().map(|e| Ok(packet_mods(e)?.unwrap_or(base))).collect::, String>>()?; + // Every event is read before any is sent, so a malformed packet sends none of them. + let read = events + .iter() + .zip(held) + .map(|(e, backquote)| { + let mods = packet_mods(e)?.unwrap_or(base); + let kind = s(e, "kind").unwrap_or(""); + let kind = pointer_kind(kind).ok_or_else(|| format!("unknown pointer kind `{kind}` (down|drag|up|move|doubleclick)"))?; + if s(e, "space") == Some("screen") { + return Err("screen-space pointer events need the desktop app; use document coordinates".to_string()); + } + let x = e.get("x").and_then(Value::as_f64).ok_or("pointer event needs numeric `x`")?; + let y = e.get("y").and_then(Value::as_f64).ok_or("pointer event needs numeric `y`")?; + Ok((e, kind, Point::new(x, y), mods, backquote)) + }) + .collect::, String>>()?; self.ensure_document(); let mut requests = vec![]; - for ((e, backquote), mods) in events.iter().zip(held).zip(mods) { - let kind = s(e, "kind").unwrap_or(""); - let kind = pointer_kind(kind).ok_or_else(|| format!("unknown pointer kind `{kind}` (down|drag|up|move|doubleclick)"))?; - if s(e, "space") == Some("screen") { - return Err("screen-space pointer events need the desktop app; use document coordinates".into()); - } - let x = e.get("x").and_then(Value::as_f64).ok_or("pointer event needs numeric `x`")?; - let y = e.get("y").and_then(Value::as_f64).ok_or("pointer event needs numeric `y`")?; - let ev = PointerEvent { - kind, - pos: Point::new(x, y), - mods, - backquote, - pressure: PointerEvent::json_pressure(e), - pen: PointerEvent::json_pen(e), - }; + for (e, kind, pos, mods, backquote) in read { + let ev = PointerEvent { kind, pos, mods, backquote, pressure: PointerEvent::json_pressure(e), pen: PointerEvent::json_pen(e) }; let reqs = self.session.pointer(&ev, self.view).map_err(|e| e.to_string())?; self.apply_ui_requests(reqs, &mut requests)?; // With no frames to show the highlight or apply an edit in later, each event waits for diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index dbe429a28..86cc6228a 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -1461,6 +1461,7 @@ fn headless_methods_refuse_params_of_the_wrong_type() { h.call("ui.tool.select", json!({"tool": "rectangle"})).unwrap(); let before = h.call("document.json", json!({})).unwrap(); let drag = |mods: Value| json!({"events": [{"kind": "down", "x": 10, "y": 10}, {"kind": "drag", "x": 60, "y": 60, "mods": mods}, {"kind": "up", "x": 60, "y": 60}]}); + let after_rect = |last: Value| json!({"events": [{"kind": "down", "x": 10, "y": 10}, {"kind": "drag", "x": 60, "y": 60}, {"kind": "up", "x": 60, "y": 60}, last]}); for (method, params, key) in [ ("ui.render", json!({"artboard": "0"}), "artboard"), ("ui.render", json!({"scale": "2"}), "scale"), @@ -1469,6 +1470,10 @@ fn headless_methods_refuse_params_of_the_wrong_type() { ("ui.key", json!({"key": "Delete", "shift": "true"}), "shift"), ("ui.pointer", drag(json!({"shift": "yes"})), "mods"), ("ui.pointer", json!({"mods": 1, "events": []}), "mods"), + ("ui.pointer", drag(json!([true])), "mods"), + // A whole rectangle comes before the malformed event, and none of it is drawn. + ("ui.pointer", after_rect(json!({"kind": "move", "x": "bad", "y": 0})), "x"), + ("ui.pointer", after_rect(json!({"kind": "wiggle", "x": 0, "y": 0})), "wiggle"), ] { let e = h.call(method, params.clone()).unwrap_err(); assert!(e.contains(&format!("`{key}`")), "{method} {params}: {e}"); diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 77e931bdc..34eaa7944 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -509,6 +509,7 @@ fn coord(p: &Value, key: &str, default: f32) -> Result { fn packet_mods(value: Option<&Value>) -> Result, String> { match value { None | Some(Value::Null) => Ok(None), + Some(m) if !m.is_object() => Err("`mods` must be an object of true or false values".to_string()), Some(m) => serde_json::from_value(m.clone()).map(Some).map_err(|e| format!("`mods` must be an object of true or false values: {e}")), } } @@ -920,7 +921,7 @@ fn rotate_view_pointer( /// A Rotate View packet accepts only the documented boolean modifier fields. fn validate_rotate_view_mods(value: Option<&Value>) -> Result<(), String> { - let Some(value) = value else { return Ok(()) }; + let Some(value) = value.filter(|value| !value.is_null()) else { return Ok(()) }; let Some(fields) = value.as_object() else { return Err("Rotate View `mods` must be an object of booleans".into()) }; if fields.iter().any(|(key, value)| !matches!(key.as_str(), "shift" | "alt" | "cmd" | "ctrl" | "space") || !value.is_boolean()) { return Err("Rotate View `mods` accepts only boolean shift, alt, cmd, ctrl and space".into()); @@ -975,8 +976,16 @@ fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &Con events .iter() .map(|e| { + let kind = match e.get("kind").and_then(Value::as_str).unwrap_or("") { + "down" => PointerKind::Down, + "drag" => PointerKind::Drag, + "up" => PointerKind::Up, + "move" => PointerKind::Move, + "doubleclick" | "dblclick" => PointerKind::DoubleClick, + other => return Err(format!("unknown pointer kind `{other}`")), + }; let at = |k: &str| crate::menus::opt_number(e, k).map(|n| n.unwrap_or(0.0)); - Ok((at("x")?, at("y")?, packet_mods(e.get("mods"))?.unwrap_or(base))) + Ok((kind, at("x")?, at("y")?, packet_mods(e.get("mods"))?.unwrap_or(base))) }) .collect::, String>>() }); @@ -986,6 +995,10 @@ fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &Con }; let view = app.view_info(); let xf = app.canvas_rect.zip(app.view().copied()).map(|(rect, v)| Xf::new(rect, &v)); + let screen = |e: &Value| e.get("space").and_then(Value::as_str) == Some("screen"); + if !rotating && xf.is_none() && events.iter().any(screen) { + return err("no canvas yet"); + } let from = app.session.input_count(); let held = PointerEvent::json_backquote(p, false).and_then(|d| events.iter().map(|e| PointerEvent::json_backquote(e, d)).collect::, _>>()); @@ -993,16 +1006,8 @@ fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &Con Ok(held) => held, Err(e) => return err(e), }; - for ((e, backquote), (x, y, mods)) in events.iter().zip(held).zip(read) { - let kind = match e.get("kind").and_then(Value::as_str).unwrap_or("") { - "down" => PointerKind::Down, - "drag" => PointerKind::Drag, - "up" => PointerKind::Up, - "move" => PointerKind::Move, - "doubleclick" | "dblclick" => PointerKind::DoubleClick, - other => return err(format!("unknown pointer kind `{other}`")), - }; - let screen = e.get("space").and_then(Value::as_str) == Some("screen"); + for ((e, backquote), (kind, x, y, mods)) in events.iter().zip(held).zip(read) { + let screen = screen(e); if rotating { if let Err(e) = rotate_view_pointer(app, kind, vectorcraft_geom::Point::new(x, y), screen, mods.shift) { return err(e); diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 2cdf13a4d..021365d81 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -349,6 +349,7 @@ fn control_input_refuses_params_of_the_wrong_type() { ("ui.set", json!({"grid": true, "controlBar": 0}), "controlBar"), ("ui.pointer", json!({"events": [{"kind": "move", "x": "5", "y": 0}]}), "x"), ("ui.pointer", move_to(json!({"shift": "yes"})), "mods"), + ("ui.pointer", move_to(json!([true])), "mods"), ("ui.pointer", json!({"mods": "shift", "events": [{"kind": "move", "x": 1, "y": 1}]}), "mods"), ] { let mut app = VectorcraftApp::new(Session::new(), Services::default()); @@ -456,6 +457,29 @@ fn synthetic_pointer_rejects_a_backquote_that_is_not_a_boolean() { } } +/// `ui.pointer` refuses an unknown event kind, or a screen-space event with no canvas, before any +/// of the packet's events runs. +#[test] +fn synthetic_pointer_rejects_an_unknown_kind_before_any_event_runs() { + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + app.run("file.new", json!({"width": 400, "height": 400})).unwrap(); + let id = app.run("shape.rectangle", json!({"x": 100, "y": 100, "width": 50, "height": 50})).unwrap()["id"].as_u64().unwrap(); + app.run("select.set", json!({"ids": [id]})).unwrap(); + assert_eq!(input_request(&mut app, "ui.tool.select", json!({"tool": "selection"}))["ok"], true); + let before = app.session.doc().unwrap().doc.node(vectorcraft_doc::NodeId(id)).unwrap().clone(); + let moves = [json!({"kind": "down", "x": 125, "y": 125}), json!({"kind": "drag", "x": 145, "y": 125}), json!({"kind": "up", "x": 145, "y": 125})]; + for (last, error) in [ + (json!({"kind": "wiggle", "x": 145, "y": 125}), "unknown pointer kind `wiggle`"), + (json!({"kind": "move", "x": 145, "y": 125, "space": "screen"}), "no canvas yet"), + ] { + let events = moves.iter().cloned().chain([last]).collect::>(); + let r = input_request(&mut app, "ui.pointer", json!({"events": events})); + assert_eq!(r["ok"], false, "{error}: {r}"); + assert!(r["error"].as_str().is_some_and(|e| e.contains(error)), "{r}"); + assert_eq!(app.session.doc().unwrap().doc.node(vectorcraft_doc::NodeId(id)).unwrap(), &before, "{error}: the drag does not run"); + } +} + /// `ui.pointer` replies with the key object: a click on one of two selected objects makes it /// the key, and a click on the key again lets it go. #[test] diff --git a/crates/ui-egui/src/tests/rotateview.rs b/crates/ui-egui/src/tests/rotateview.rs index dbdb0283f..715610f08 100644 --- a/crates/ui-egui/src/tests/rotateview.rs +++ b/crates/ui-egui/src/tests/rotateview.rs @@ -117,7 +117,8 @@ fn control_rotate_view_keeps_press_coordinates_and_shift_snapping_across_request send(&mut app, &ctx, "ui.pointer", json!({"mods": {"shift": true}, "events": [{"kind": "drag", "x": x, "y": y}]})); assert_eq!(app.view().unwrap().rotation, 45.0, "the same document point does not turn again as the view changes"); } - send(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "up", "x": x, "y": y}]})); + // Null `mods` keep the default, as an omitted value does. + send(&mut app, &ctx, "ui.pointer", json!({"mods": null, "events": [{"kind": "up", "x": x, "y": y, "mods": null}]})); assert_eq!(app.view().unwrap().rotation, 45.0); } diff --git a/docs/mcp.md b/docs/mcp.md index 7ced9eb15..078488e9d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1271,7 +1271,9 @@ type and change nothing: `view.setZoom` (`zoom`, `center`), `view.screenMode` (` `ui.symbolOptions` (`edit`), `ui.batchDialog` (`set`, `action`) and `file.exportSelection` (`multiple`). So do the control channel's `ui.key`, `ui.move`, `ui.wheel`, `ui.click`, `ui.drag`, `ui.pointer`, `ui.resize`, `ui.screenshot`, `ui.render` and `ui.set` for their numbers, switches and `mods`, and the headless session's key, pointer and screenshot -calls. Omitting a param or giving null keeps its default: +calls. Omitting a param or giving null keeps its default, with two exceptions: `ui.click`'s `count` and `ui.drag`'s +`count` and `steps` refuse null, and a headless pointer event needs its `x` and `y`. A pointer packet with one +malformed event sends none of its events: ```json {"id": "view.setZoom", "params": {"zoom": "200"}}