From fb71daf2c6fb508bf11313fed47aa1fa59baae40 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Fri, 9 Oct 2026 06:12:34 +0200 Subject: [PATCH] Control channel: ui.click rejects a count above 256 instead of queueing unbounded input (#982) (#1446) --- crates/ui-egui/src/control.rs | 43 ++++++++++++++++++++++++++++++----- docs/control-protocol.md | 1 + 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 667a1e3d33..e419f4cd46 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -16,6 +16,7 @@ //! - `ui.window.open {document?}` / `ui.window.close {window}`: extra document windows //! - `ui.pointer {events: [{kind: down|move|up, x, y, pressure?, tiltX?, tiltY?, rotation?}], modifiers?, button?}`: drive the active tool in document coordinates (`button: "secondary"` opens the tool's canvas context menu or Brush Preset picker, or erases with Preferences › Tools › Right-click with painting tools = erase) //! - `ui.click {x, y, button?, count?}` / `ui.move {x, y}`: synthetic pointer input in screen points +//! (`count` at most [`MAX_CLICKS`]) //! - `ui.key {key, command?, shift?, alt?, ctrl?}` / `ui.type {text}`: synthetic keyboard input //! - `ui.resize {width, height}`: resize the main window //! - `ui.gpu.simulateLoss {error?}`: act as if the wgpu device was lost (or, with `error: true`, @@ -98,6 +99,11 @@ pub const UI_SET_FIELDS: [&str; 21] = [ "gradientClassic", ]; +/// Most clicks one `ui.click` may queue (#982). Each click is a press and a release that the app +/// feeds in its own frame before replying, so `count` sizes both the input queue and the wait; +/// the same ceiling as the steps of one batch request. +pub const MAX_CLICKS: u64 = 256; + fn ok(v: Value) -> Outcome { Outcome::Done(json!({"ok": true, "result": v})) } @@ -628,13 +634,14 @@ fn dispatch(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest) "middle" => egui::PointerButton::Middle, _ => egui::PointerButton::Primary, }; + let clicks = if req.method == "ui.click" { u("count").unwrap_or(1) } else { 0 }; + if clicks > MAX_CLICKS { + return err(format!("`count` must be at most {MAX_CLICKS} (got {clicks})")); + } app.synthetic.push(egui::Event::PointerMoved(pos)); - if req.method == "ui.click" { - let clicks = p.get("count").and_then(Value::as_u64).unwrap_or(1); - for _ in 0..clicks { - app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: true, modifiers: Default::default() }); - app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: false, modifiers: Default::default() }); - } + for _ in 0..clicks { + app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: true, modifiers: Default::default() }); + app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: false, modifiers: Default::default() }); } ctx.request_repaint(); Outcome::AfterInput @@ -1243,4 +1250,28 @@ mod tests { let r = app.run("actions.play", json!({"action": "Open"})).unwrap(); assert_eq!(r["ran"], 1, "{r}"); } + + #[test] + fn huge_click_count_is_rejected() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + let ctx = egui::Context::default(); + // #982: a huge count used to queue a press and a release per click before replying. + for count in [json!(MAX_CLICKS + 1), json!(1_000_000_000u64), json!(u64::MAX)] { + let r = call(&mut app, &ctx, "ui.click", json!({"x": 10, "y": 10, "count": count})); + assert_eq!(r["ok"], false, "{r}"); + assert!(r["error"].as_str().unwrap().contains("`count` must be at most 256"), "{r}"); + assert!(app.synthetic.is_empty(), "a rejected click queues nothing"); + } + // Single, double and the largest allowed clicks still queue a move plus a press and a release each. + for (count, events) in [(None, 3), (Some(2), 5), (Some(MAX_CLICKS), 2 * MAX_CLICKS as usize + 1)] { + let params = match count { + Some(n) => json!({"x": 10, "y": 10, "count": n}), + None => json!({"x": 10, "y": 10}), + }; + let (req, _rx) = ControlRequest::new("ui.click", params); + assert!(matches!(handle(&mut app, &ctx, &req), Outcome::AfterInput)); + assert_eq!(app.synthetic.len(), events); + app.synthetic.clear(); + } + } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 3a28b4eab9..3db45beef9 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -51,6 +51,7 @@ The transport is `apps/photocraft/src/control_server.rs`, and the handlers are i - `ui.window.open {document?}` / `ui.window.close {window}`: extra document windows - `ui.pointer {events: [{kind: down|move|up, x, y, pressure?, tiltX?, tiltY?, rotation?}], modifiers?, button?}`: drive the active tool in document coordinates (pressure 0..1, tilt in degrees -90..90, barrel rotation 0..360: a simulated pen). Modifier flags (`shift`, `alt`, `command`, `ctrl`, `space`) go either at the top level or grouped under `modifiers`; when `modifiers` is present, top-level flags are ignored. `space: true` holds Space (the Crop frame, marquee, lasso or shape being drawn then moves instead of growing). `button: "right"` (or `"secondary"`) is the right button: with the Move tool (or `command: true` with any tool) it opens the canvas layer menu (`layerMenu` in `ui.inspect` lists the layers there, topmost first; select one with `layer.select`); with Marquee, Lasso, Magic Wand, Object Selection, or Pen it opens a tool context menu (`canvasToolMenu` in `ui.inspect`, with ordered command ids, separators, and enabled states); with a painting tool it opens the Brush Preset picker, or erases with Preferences › Tools › Right-click with painting tools = erase. The menus open at the screen position of the pressed document point. Use `ui.context.choose {id}` for enabled tool-context actions. The Pen menu is documented in `docs/context-menu-parity.md`. With a Color Picker as the top dialog, `down` and `move` sample the image into its new colour instead (its eyedropper, like a click on the canvas) and the tool is not driven. - Magnetic Lasso (`tool: "magneticLasso"`): a `down`/`up` pair is a click. The first sets the first fastening point (its modifiers set the selection mode); later clicks fasten a point, or close the border on the first point. `move` events trace the border with or without a button held, fastening points by themselves; with `alt: true` a click draws a straight segment and a drag a freehand one. `ui.inspect` shows the border in progress under `magnetic` (`path`, `anchors`, `live`, `mode`). `ui.key` Enter closes it along the edges, Backspace removes the last fastening point, Escape cancels. To select along edges in one call instead, use the `select.magneticLasso` command with rough points around the shape. +- `ui.click {x, y, button?, count?}` / `ui.move {x, y}`: synthetic pointer input in screen points (`button`: `left` (default), `right` or `middle`). `count` (default 1, `2` for a double-click) is at most 256; a larger one is an error and queues nothing - `ui.key {key, command?, shift?, alt?, ctrl?}` (flags may also be grouped under `modifiers`): press and release a key, e.g. `{"key": "ArrowLeft", "shift": true}` - `ui.type {text}`: type text (goes to the focused widget, or to the canvas while the Type tool is editing) - `ui.resize {width, height}`: resize the main window