Palette: entries rebuild when keyboard shortcuts change

The command palette caches its entries per language and plug-in set
(#243), but each entry also shows its shortcut: after editing shortcuts
the palette kept showing the old ones until the language changed. The
cache key now includes the shortcut editor's generation. From #242.

Co-authored-by: WildfootW <wildfootw@wildfoo.tw>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

apps/vectorcraft/src/control_server.rs

@@ -1,14 +1,28 @@
 //! Loopback JSON-lines control server: one request per line, one reply per line.
 //! This is the transport the MCP server (`vectorcraft mcp --connect`) wraps.
+//!
+//! The server only ever reads requests: a line that is not a JSON object with a string `method`
+//! (an HTTP request line from a browser's cross-origin `fetch`, a stray `nc`, binary junk) gets
+//! one error reply and the connection is closed, so nothing after it is executed. Lines are
+//! bounded ([`MAX_LINE`]) and so is the number of open connections ([`MAX_CONNECTIONS`]); junk
+//! never reaches the UI thread. See `docs/control-protocol.md`.
 
-use std::io::{BufRead, BufReader, Write};
+use std::io::{BufRead, BufReader, Read, Write};
 use std::net::{TcpListener, TcpStream};
+use std::sync::Arc;
+use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::mpsc::{Receiver, Sender, channel};
 use std::time::Duration;
 
 use serde_json::{Value, json};
 use vectorcraft_ui_egui::ControlRequest;
 
+/// Longest accepted request line (bytes, without the newline). Requests are small JSON objects;
+/// anything longer is refused and the connection closed.
+pub const MAX_LINE: usize = 4 * 1024 * 1024;
+/// Connections served at once; further ones get an error line and are closed.
+pub const MAX_CONNECTIONS: usize = 16;
+
 pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> {
     let (tx, rx) = channel::<ControlRequest>();
     let listener = match TcpListener::bind(("127.0.0.1", port)) {
@@ -19,44 +33,249 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> {
         }
     };
     eprintln!("vectorcraft: control server listening on 127.0.0.1:{port}");
-    std::thread::spawn(move || {
-        for stream in listener.incoming().flatten() {
-            let tx = tx.clone();
-            let ctx = ctx.clone();
-            std::thread::spawn(move || serve(stream, tx, ctx));
-        }
-    });
+    std::thread::spawn(move || accept_loop(listener, tx, ctx));
     rx
 }
 
-fn serve(stream: TcpStream, tx: Sender<ControlRequest>, ctx: egui::Context) {
-    let Ok(read) = stream.try_clone() else { return };
-    let mut out = stream;
-    for line in BufReader::new(read).lines() {
-        let Ok(line) = line else { break };
-        if line.trim().is_empty() {
+fn accept_loop(listener: TcpListener, tx: Sender<ControlRequest>, ctx: egui::Context) {
+    let open = Arc::new(AtomicUsize::new(0));
+    for mut stream in listener.incoming().flatten() {
+        if open.fetch_add(1, Ordering::SeqCst) >= MAX_CONNECTIONS {
+            open.fetch_sub(1, Ordering::SeqCst);
+            // Best effort: the connection is dropped either way, and a peer that can't read the reason loses nothing.
+            let _ = writeln!(stream, "{}", json!({"ok": false, "error": "too many control connections"}));
             continue;
         }
-        let reply = match serde_json::from_str::<Value>(&line) {
-            Ok(msg) => {
-                let id = msg.get("id").cloned().unwrap_or(Value::Null);
-                let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string();
-                let params = msg.get("params").cloned().unwrap_or(json!({}));
-                let (req, rrx) = ControlRequest::new(method, params);
-                if tx.send(req).is_err() {
-                    break;
-                }
-                ctx.request_repaint();
-                let mut r = rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"}));
-                if let Some(o) = r.as_object_mut() {
-                    o.insert("id".into(), id);
+        let tx = tx.clone();
+        let ctx = ctx.clone();
+        let open = Arc::clone(&open);
+        std::thread::spawn(move || {
+            serve(stream, &tx, &ctx);
+            open.fetch_sub(1, Ordering::SeqCst);
+        });
+    }
+}
+
+fn serve(stream: TcpStream, tx: &Sender<ControlRequest>, ctx: &egui::Context) {
+    let Ok(read) = stream.try_clone() else { return };
+    serve_lines(read, stream, |method, params| {
+        let (req, rrx) = ControlRequest::new(method, params);
+        tx.send(req).ok()?;
+        ctx.request_repaint();
+        Some(rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})))
+    });
+}
+
+/// One parsed line.
+enum Line {
+    Request {
+        id: Value,
+        method: String,
+        params: Value,
+    },
+    Blank,
+    /// Not a request: reply with this error and close the connection.
+    Reject(String),
+}
+
+fn parse_line(bytes: &[u8]) -> Line {
+    let Ok(text) = std::str::from_utf8(bytes) else {
+        return Line::Reject("not a control request (invalid UTF-8); closing the connection".into());
+    };
+    let text = text.trim();
+    if text.is_empty() {
+        return Line::Blank;
+    }
+    let msg = match serde_json::from_str::<Value>(text) {
+        Ok(v) => v,
+        Err(e) => return Line::Reject(format!("bad JSON ({e}); expected one JSON request object per line; closing the connection")),
+    };
+    let Some(obj) = msg.as_object() else {
+        return Line::Reject("not a control request (expected a JSON object); closing the connection".into());
+    };
+    let Some(method) = obj.get("method").and_then(Value::as_str) else {
+        return Line::Reject("not a control request (missing string `method`); closing the connection".into());
+    };
+    Line::Request {
+        id: obj.get("id").cloned().unwrap_or(Value::Null),
+        method: method.to_string(),
+        params: obj.get("params").cloned().unwrap_or(json!({})),
+    }
+}
+
+/// Read one `\n`-terminated line of at most `max` bytes into `buf` (newline stripped).
+/// `Ok(false)` at end of stream; `Err` when the line is too long or the read fails.
+fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec<u8>, max: usize) -> Result<bool, String> {
+    buf.clear();
+    // `take` bounds the bytes buffered for one line: a peer can't make us allocate more than `max`.
+    let limit = u64::try_from(max).unwrap_or(u64::MAX).saturating_add(1);
+    let n = reader.take(limit).read_until(b'\n', buf).map_err(|e| e.to_string())?;
+    if n == 0 {
+        return Ok(false);
+    }
+    if buf.last() == Some(&b'\n') {
+        buf.pop();
+        if buf.last() == Some(&b'\r') {
+            buf.pop();
+        }
+        return Ok(true);
+    }
+    if buf.len() > max {
+        return Err(format!("request line longer than {max} bytes; closing the connection"));
+    }
+    // Last line without a newline, then end of stream.
+    Ok(true)
+}
+
+/// Serve requests from `read`, answering on `out`, until the peer closes, sends something that is
+/// not a request, or `handle` returns `None` (app gone). `handle` gets `(method, params)` and
+/// returns the reply object (the request's `id` is added here).
+fn serve_lines<R: Read, W: Write>(read: R, mut out: W, mut handle: impl FnMut(String, Value) -> Option<Value>) {
+    let mut reader = BufReader::new(read);
+    let mut buf = Vec::new();
+    loop {
+        let reject = match read_bounded_line(&mut reader, &mut buf, MAX_LINE) {
+            Ok(false) => return,
+            Err(e) => e,
+            Ok(true) => match parse_line(&buf) {
+                Line::Blank => continue,
+                Line::Reject(e) => e,
+                Line::Request { id, method, params } => {
+                    let Some(mut r) = handle(method, params) else { return };
+                    if let Some(o) = r.as_object_mut() {
+                        o.insert("id".into(), id);
+                    }
+                    if writeln!(out, "{r}").and_then(|()| out.flush()).is_err() {
+                        return;
+                    }
+                    continue;
                 }
-                r
-            }
-            Err(e) => json!({"ok": false, "error": format!("bad JSON: {e}")}),
+            },
         };
-        if writeln!(out, "{reply}").is_err() {
-            break;
+        // Best effort: the connection closes either way, and a peer that can't read the reason loses nothing.
+        let _ = writeln!(out, "{}", json!({"ok": false, "error": reject})).and_then(|()| out.flush());
+        return;
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn run(input: &[u8]) -> (Vec<Value>, Vec<String>) {
+        let mut out = Vec::new();
+        let mut calls = Vec::new();
+        serve_lines(input, &mut out, |m, _p| {
+            calls.push(m);
+            Some(json!({"ok": true, "result": null}))
+        });
+        let replies = String::from_utf8(out).unwrap().lines().map(|l| serde_json::from_str::<Value>(l).unwrap()).collect();
+        (replies, calls)
+    }
+
+    #[test]
+    fn requests_are_answered_in_order_with_ids() {
+        let (r, calls) = run(b"{\"id\":1,\"method\":\"a\"}\n\n{\"id\":2,\"method\":\"b\",\"params\":{}}\r\n{\"method\":\"c\"}");
+        assert_eq!(calls, ["a", "b", "c"]);
+        assert_eq!(r.len(), 3);
+        assert_eq!(r[0]["id"], 1);
+        assert_eq!(r[1]["id"], 2);
+        assert_eq!(r[2]["id"], Value::Null);
+    }
+
+    #[test]
+    fn http_request_closes_before_the_body_runs() {
+        // A browser's cross-origin `fetch` POST: the request line is rejected and the
+        // connection closed, so the JSON body is never executed.
+        let http = b"POST / HTTP/1.1\r\nHost: 127.0.0.1:7979\r\nOrigin: https://attacker.example\r\nContent-Type: text/plain\r\n\r\n\
+            {\"id\":7,\"method\":\"engine.execute\",\"params\":{\"command\":\"file.new\",\"params\":{}}}\n";
+        let (r, calls) = run(http);
+        assert!(calls.is_empty(), "nothing may run: {calls:?}");
+        assert_eq!(r.len(), 1);
+        assert_eq!(r[0]["ok"], false);
+        assert!(r[0]["error"].as_str().unwrap().contains("closing the connection"));
+    }
+
+    #[test]
+    fn non_request_json_closes_the_connection() {
+        for junk in [
+            &b"[1,2]\n{\"method\":\"a\"}\n"[..],
+            b"{\"id\":1}\n{\"method\":\"a\"}\n",
+            b"{\"method\":5}\n{\"method\":\"a\"}\n",
+            b"\xff\xfe\n{\"method\":\"a\"}\n",
+        ] {
+            let (r, calls) = run(junk);
+            assert!(calls.is_empty(), "{calls:?}");
+            assert_eq!(r.len(), 1);
+            assert_eq!(r[0]["ok"], false);
         }
+    }
+
+    #[test]
+    fn a_request_before_junk_still_runs() {
+        let (r, calls) = run(b"{\"method\":\"a\"}\nGET / HTTP/1.1\n{\"method\":\"b\"}\n");
+        assert_eq!(calls, ["a"]);
+        assert_eq!(r.len(), 2);
+        assert_eq!(r[1]["ok"], false);
+    }
+
+    #[test]
+    fn overlong_line_is_refused() {
+        // One byte over the limit, then a valid request: neither runs.
+        let mut input = vec![b'x'; MAX_LINE + 1];
+        input.extend_from_slice(b"\n{\"method\":\"a\"}\n");
+        let (r, calls) = run(&input);
+        assert!(calls.is_empty(), "{calls:?}");
+        assert_eq!(r.len(), 1);
+        assert!(r[0]["error"].as_str().unwrap().contains("longer than"));
+    }
+
+    #[test]
+    fn line_at_the_limit_is_accepted() {
+        let mut buf = Vec::new();
+        let line = vec![b'a'; 10];
+        let mut input = line.clone();
+        input.push(b'\n');
+        assert_eq!(read_bounded_line(&mut &input[..], &mut buf, 10), Ok(true));
+        assert_eq!(buf, line);
+        assert!(read_bounded_line(&mut &[b'a'; 11][..], &mut buf, 10).is_err());
+    }
+
+    #[test]
+    fn tcp_connection_is_closed_after_junk() {
+        // End to end over a real loopback socket (the transport the app uses).
+        let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap();
+        let addr = listener.local_addr().unwrap();
+        let server = std::thread::spawn(move || {
+            let (s, _) = listener.accept().unwrap();
+            let read = s.try_clone().unwrap();
+            serve_lines(read, s, |_, _| Some(json!({"ok": true})));
+        });
+        let mut c = TcpStream::connect(addr).unwrap();
+        c.set_read_timeout(Some(Duration::from_secs(5))).unwrap();
+        c.write_all(b"{\"id\":7,\"method\":\"x\"}\nhello\n{\"method\":\"y\"}\n").unwrap();
+        let mut text = String::new();
+        c.read_to_string(&mut text).unwrap(); // returns at EOF: the server closed the connection
+        server.join().unwrap();
+        let lines: Vec<&str> = text.lines().collect();
+        assert_eq!(lines.len(), 2, "{text}");
+        assert!(lines[0].contains("\"id\":7"));
+        assert!(lines[1].contains("closing the connection"));
+    }
+
+    #[test]
+    fn extra_connections_are_refused() {
+        let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap();
+        let addr = listener.local_addr().unwrap();
+        let (tx, _rx) = channel::<ControlRequest>();
+        std::thread::spawn(move || accept_loop(listener, tx, egui::Context::default()));
+        // Idle connections are accepted in order and held open.
+        let held: Vec<TcpStream> = (0..16).map(|_| TcpStream::connect(addr).unwrap()).collect();
+        let mut extra = TcpStream::connect(addr).unwrap();
+        extra.set_read_timeout(Some(Duration::from_secs(5))).unwrap();
+        let mut line = String::new();
+        BufReader::new(&mut extra).read_line(&mut line).unwrap();
+        assert!(line.contains("too many control connections"), "{line}");
+        drop(held);
     }
 }

crates/engine/src/tests_textedit.rs

@@ -328,6 +328,23 @@ fn select_all_and_cut_like_ui() {
     assert_eq!(obj(&s, id).plain_text(), "abc defabc def");
 }
 
+#[test]
+fn a_frame_dragged_with_the_type_tools_is_the_dragged_rectangle() {
+    for tool in ["type", "verticalType"] {
+        let mut s = session();
+        let v = ViewInfo::default();
+        s.select_tool(tool, v).unwrap();
+        for (k, x, y) in [(PointerKind::Down, 100.0, 100.0), (PointerKind::Drag, 300.0, 180.0), (PointerKind::Up, 300.0, 180.0)] {
+            s.pointer(&PointerEvent::new(k, x, y), v).unwrap();
+        }
+        let t = obj(&s, s.doc().unwrap().selection.objects[0]);
+        let TextKind::Area { frame } = &t.kind else { panic!("{tool}: not area type") };
+        let b = frame.bounds().unwrap();
+        let (a, z) = (t.xf * Point::new(b.x0, b.y0), t.xf * Point::new(b.x1, b.y1));
+        assert!((a - Point::new(100.0, 100.0)).hypot() < 1e-9 && (z - Point::new(300.0, 180.0)).hypot() < 1e-9, "{tool}: {a:?} {z:?}");
+    }
+}
+
 #[test]
 fn area_type_from_a_circle_keeps_glyphs_inside() {
     let mut s = session();

crates/tools/src/text.rs

@@ -235,19 +235,26 @@ impl TypeTool {
         }
     }
 
-    fn on_up(&mut self, cx: &ToolContext, start: Point) -> Vec<Action> {
+    /// Start editing the type under `p` with the caret there → the actions (ending the previous
+    /// edit, selecting the type); None when no type is under `p`.
+    fn edit_at(&mut self, cx: &ToolContext, p: Point) -> Option<Vec<Action>> {
+        let h = hit_test(cx.doc, p, cx.hit_options())?;
+        let Some(NodeKind::Text(t)) = cx.doc.node(h.leaf).map(|n| &n.kind) else { return None };
+        let lay = self.layout(t);
+        let byte = vectorcraft_text::hit_byte(&lay, t.xf.inverse() * p);
         let mut out = self.finish(cx);
+        self.start_editing(h.leaf, byte);
+        self.clicks = (Some(p), 1);
+        out.push(Action::Exec("select.set".into(), json!({"ids": [h.leaf.0]})));
+        Some(out)
+    }
+
+    fn on_up(&mut self, cx: &ToolContext, start: Point) -> Vec<Action> {
         // Click into existing text: place the caret.
-        if let Some(h) = hit_test(cx.doc, start, cx.hit_options())
-            && let Some(NodeKind::Text(t)) = cx.doc.node(h.leaf).map(|n| &n.kind)
-        {
-            let lay = self.layout(t);
-            let byte = vectorcraft_text::hit_byte(&lay, t.xf.inverse() * start);
-            self.start_editing(h.leaf, byte);
-            self.clicks = (Some(start), 1);
-            out.push(Action::Exec("select.set".into(), json!({"ids": [h.leaf.0]})));
+        if let Some(out) = self.edit_at(cx, start) {
             return out;
         }
+        let mut out = self.finish(cx);
         let drag = self.drag.take();
         // Area Type / Type on a Path: click a path.
         if self.mode != Mode::Type && drag.is_none() {
@@ -264,7 +271,7 @@ impl TypeTool {
         }
         let area = drag.map(|d| Rect::from_points(start, d)).filter(|r| r.width() > cx.tol(6.0) && r.height() > cx.tol(6.0));
         let mut params = match area {
-            Some(r) => json!({"x": r.x0, "y": r.y0 + 12.0, "text": "", "area": {"width": r.width(), "height": r.height()}}),
+            Some(r) => json!({"x": r.x0, "y": r.y0, "text": "", "area": {"width": r.width(), "height": r.height()}}),
             None => json!({"x": start.x, "y": start.y, "text": ""}),
         };
         params["vertical"] = json!(self.vertical);
@@ -357,6 +364,12 @@ impl Tool for TypeTool {
                     return out;
                 }
                 self.clicks = (None, 0);
+                // A press on other type edits it from there, so a drag selects its text at once,
+                // as it does in the type being edited.
+                if let Some(out) = self.edit_at(cx, ev.pos) {
+                    self.selecting = true;
+                    return out;
+                }
                 self.press = Some(ev.pos);
                 self.drag = None;
                 vec![]

crates/tools/src/text_tests.rs

@@ -176,6 +176,33 @@ fn drag_selects_and_overlay_highlights() {
     assert_eq!((opts["start"].as_u64(), opts["end"].as_u64()), (Some(7), Some(9)));
 }
 
+#[test]
+fn a_drag_across_type_not_being_edited_selects_its_text() {
+    let (d, id) = doc_with_text("Select me please");
+    let mut tool = TypeTool::new("type");
+    let (sel, p) = (Selection::default(), paint());
+    let c = cx(&d, &sel, &p);
+    let t = TypeTool::text(&c, id).unwrap().clone();
+    let lay = vectorcraft_text::layout(FontDb::global(), &t);
+    let pt = |b: usize| {
+        let (a, bb) = vectorcraft_text::caret_position(&lay, b);
+        t.xf * a.midpoint(bb)
+    };
+    let (a, b) = (pt(7), pt(9));
+    // The press starts editing the type and the drag selects, in one gesture.
+    let out = tool.pointer(&c, &PointerEvent::new(PointerKind::Down, a.x + 0.1, a.y));
+    assert!(out.contains(&Action::Exec("select.set".into(), json!({"ids": [id.0]}))));
+    tool.pointer(&c, &PointerEvent::new(PointerKind::Drag, b.x + 0.1, b.y));
+    tool.pointer(&c, &PointerEvent::new(PointerKind::Up, b.x + 0.1, b.y));
+    assert_eq!(tool.editing, Some(id));
+    assert_eq!(tool.sel(), (7, 9));
+    // A plain click still just places the caret.
+    let mut tool = TypeTool::new("type");
+    tool.pointer(&c, &PointerEvent::new(PointerKind::Down, a.x + 0.1, a.y));
+    tool.pointer(&c, &PointerEvent::new(PointerKind::Up, a.x + 0.1, a.y));
+    assert_eq!((tool.editing, tool.sel()), (Some(id), (7, 7)));
+}
+
 #[test]
 fn select_all_and_styled_paste() {
     let (d, _, mut tool) = editing("abc");

crates/ui-egui/src/palette.rs

@@ -42,11 +42,13 @@ struct Entry {
     haystack: String,
 }
 
-/// The palette's entries in `lang`. They are built when the language or the installed plug-ins
-/// change, not on every frame while a query is typed.
+/// The palette's entries in `lang`. They are built when the language, the installed plug-ins or
+/// the keyboard shortcuts (shown beside each entry) change, not on every frame while a query is
+/// typed.
 fn entries(ctx: &egui::Context, lang: crate::i18n::Lang) -> Arc<Vec<Entry>> {
-    type Cached = ((&'static str, u64), Arc<Vec<Entry>>);
-    let key = (lang.code(), menus::plugin_revision());
+    type Cached = ((&'static str, u64, u64), Arc<Vec<Entry>>);
+    let shortcuts = crate::shortcut_editor::GENERATION.load(std::sync::atomic::Ordering::Relaxed);
+    let key = (lang.code(), menus::plugin_revision(), shortcuts);
     let id = egui::Id::new("palette-entries");
     if let Some((k, v)) = ctx.data(|d| d.get_temp::<Cached>(id))
         && k == key
@@ -123,6 +125,9 @@ mod tests {
         assert!(Arc::ptr_eq(&en, &entries(&ctx, Lang::EN)), "kept while the language stays");
         let zh_entries = entries(&ctx, zh);
         assert!(!Arc::ptr_eq(&en, &zh_entries), "rebuilt for another language");
+        // Editing a shortcut rebuilds them, so the palette shows the new one.
+        crate::shortcut_editor::GENERATION.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        assert!(!Arc::ptr_eq(&zh_entries, &entries(&ctx, zh)), "rebuilt after a shortcut edit");
         let group = zh_entries.iter().find(|e| e.id == "object.group").unwrap();
         assert!(group.shown.ends_with(crate::i18n::tr(zh, "Group")), "{}", group.shown);
         // A query matches the English label, the shown label or the id.

docs/control-protocol.md

@@ -3,6 +3,16 @@
 `vectorcraft --control <port>` listens on `127.0.0.1:<port>` (loopback only). One JSON request per line:
 `{"id": 1, "method": "ui.inspect", "params": {}}` → `{"id": 1, "ok": true, "result": {...}}` or `{"id":1,"ok":false,"error":"..."}`.
 
+**Only requests are read.** Every line must be a JSON object with a string `method` (`id` and `params`
+are optional; blank lines are skipped). Anything else gets one error reply
+(`{"ok": false, "error": "… closing the connection"}`) and the server **closes the connection**, so
+nothing sent after it on that connection runs. That covers text that isn't JSON, a JSON array or number,
+an object without `method`, invalid UTF-8, and a line longer than 4 MiB. An HTTP request (for example a
+web page's cross-origin `fetch` to `127.0.0.1:<port>`) therefore can't smuggle a command in its body:
+its request line is rejected first. At most 16 connections are served at once; further ones get an error
+line and are closed. Clients that get an error reply should reconnect. The port has no authentication,
+so only enable it while you use it. Transport: `apps/vectorcraft/src/control_server.rs`.
+
 | Method | Params | |
 |---|---|---|
 | `engine.execute` | `{command, params}` | run any engine or UI command (see `engine.commands`) |