From 213cc75ff22e79aa434b57033157851a642e2805 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:13:59 +0100 Subject: [PATCH 1/7] Fix looks_like_svg rejecting DOCTYPE with an internal subset Found while testing crates/svg/src/import/mod.rs:723-740 (looks_like_svg, the `')) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/svg/src/import.rs | 41 +++++++++++++++++++++++++++- crates/svg/src/import/shape_tests.rs | 11 ++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index dc936260..ea9c321f 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -718,6 +718,45 @@ fn options(data: &[u8], limits: budget::Limits) -> usvg::Options<'static> { o } +/// Length of the `` declaration at the start of `s`, skipping `>` inside quotes and an +/// internal subset (`[ ... ]`). +fn doctype_end(s: &str) -> Option { + let mut quote = None; + let mut depth = 0u32; + let mut skip_to = 0; + for (i, c) in s.char_indices() { + if i < skip_to { + continue; + } + if quote.is_none() && depth > 0 { + // Comments and processing instructions in the subset may hold quotes and brackets. + let rest = &s[i..]; + let close = if rest.starts_with("") + } else if rest.starts_with("") + } else { + None + }; + if let Some(close) = close { + let len = rest.find(close)? + close.len(); + skip_to = i + len; + continue; + } + } + match (quote, c) { + (Some(q), _) if c == q => quote = None, + (Some(_), _) => {} + (None, '"' | '\'') => quote = Some(c), + (None, '[') => depth += 1, + (None, ']') => depth = depth.saturating_sub(1), + (None, '>') if depth == 0 => return Some(i + 1), + _ => {} + } + } + None +} + /// Whether `text` (from the clipboard, say) is an SVG document: ` bool { @@ -731,7 +770,7 @@ pub fn looks_like_svg(text: &str) -> bool { } else if s.starts_with("").map(|i| i + 3) } else if s.starts_with("').map(|i| i + 1) + doctype_end(s) } else { None }; diff --git a/crates/svg/src/import/shape_tests.rs b/crates/svg/src/import/shape_tests.rs index 5bad142f..71d0f508 100644 --- a/crates/svg/src/import/shape_tests.rs +++ b/crates/svg/src/import/shape_tests.rs @@ -107,3 +107,14 @@ fn layer_names_decode_id_escapes() { assert_eq!(layer_name("", "Group"), "Group"); assert_eq!(layer_name("é_x41_", "Group"), "éA"); } + +#[test] +fn doctype_with_internal_subset_is_still_svg() { + let s = "\n\n]>\n"; + assert!(super::looks_like_svg(s)); + let quoted = "y]\"> ]>"; + assert!(super::looks_like_svg(quoted)); + let commented = " --> ]>"; + assert!(super::looks_like_svg(commented)); + assert!(!super::looks_like_svg(" ]>")); +} From 8ae76d9b2a8a822934b707c418afb32a7e2601c5 Mon Sep 17 00:00:00 2001 From: Mert Dede Date: Tue, 27 Oct 2026 07:57:53 +0300 Subject: [PATCH 2/7] M8: bound attached MCP replies to 512 MiB (#1113) The attached backend read each reply from the app's control port with read_line, which has no limit. It now reads up to 512 MiB before the newline (an app window capture is at most 256 MiB as base64) and stops at the first byte past that. An oversized or non-UTF-8 reply drops the connection and is reported like a lost reply; the next call reconnects. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: GPT-6.1 Sol Co-Authored-By: GPT-6 Astra --- crates/mcp/src/backend.rs | 28 +++++++++--- crates/mcp/src/backend_tests.rs | 77 +++++++++++++++++++++++++++++++++ docs/control-protocol.md | 2 + docs/mcp.md | 4 ++ docs/roadmap.md | 3 +- 5 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 crates/mcp/src/backend_tests.rs diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index f2021cc6..1c2ab141 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -1,7 +1,7 @@ //! Where MCP tool calls end up: a control-channel method call, answered either by an in-process //! engine session ([`crate::Headless`]) or by a running FigCraft app ([`Attached`]). -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::net::{TcpStream, ToSocketAddrs}; use std::time::Duration; @@ -76,17 +76,23 @@ pub fn decode_data(v: &Value) -> Result, String> { base64::engine::general_purpose::STANDARD.decode(d.trim()).map_err(|e| format!("bad base64 from the app: {e}")) } +/// The longest reply line we read from the app (512 MiB). The app's window captures alone reach +/// 256 MiB as base64 (192 MiB PNGs); anything longer is refused without reading the rest. +const MAX_REPLY: usize = 512 * 1024 * 1024; + /// A running FigCraft app, reached through its loopback control port (`figcraft --control PORT`). pub struct Attached { addr: String, conn: Option<(BufReader, TcpStream)>, next_id: u64, + /// [`MAX_REPLY`]; smaller in tests. + max_reply: usize, } impl Attached { /// Connect to `addr` (e.g. `127.0.0.1:7979`), failing fast when nothing is listening. pub fn connect(addr: &str) -> std::io::Result { - let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 }; + let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, max_reply: MAX_REPLY }; r.reconnect()?; Ok(r) } @@ -133,17 +139,25 @@ impl Attached { } fn roundtrip(&mut self, line: &str) -> std::io::Result { + let max_reply = self.max_reply; let Some((reader, writer)) = self.conn.as_mut() else { return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "not connected to the app")); }; writer.write_all(line.as_bytes())?; writer.write_all(b"\n")?; writer.flush()?; - let mut reply = String::new(); - if reader.read_line(&mut reply)? == 0 { + let mut reply = Vec::new(); + if reader.take((max_reply + 1) as u64).read_until(b'\n', &mut reply)? == 0 { return Err(std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "control channel closed")); } - Ok(reply) + if reply.last() == Some(&b'\n') { + reply.pop(); + } + if reply.len() > max_reply { + let mib = max_reply / (1024 * 1024); + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, format!("reply larger than {mib} MiB"))); + } + String::from_utf8(reply).map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) } } @@ -181,3 +195,7 @@ impl Backend for Attached { format!("attached to the FigCraft app at {}", self.addr) } } + +#[cfg(test)] +#[path = "backend_tests.rs"] +mod tests; diff --git a/crates/mcp/src/backend_tests.rs b/crates/mcp/src/backend_tests.rs new file mode 100644 index 00000000..ec4fc473 --- /dev/null +++ b/crates/mcp/src/backend_tests.rs @@ -0,0 +1,77 @@ +//! The attached backend reads the app's replies with a bound (#1113). + +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; +use std::sync::mpsc; +use std::time::Duration; + +use serde_json::json; + +use super::{Attached, Backend}; + +/// A fake app that takes one connection per reply, reads one request on it and answers with the +/// reply. It keeps the connections open until [`finish`] (so an unterminated reply never reaches +/// EOF), then returns how many requests it read. +fn fake_app(replies: Vec>) -> (u16, mpsc::Sender<()>, std::thread::JoinHandle) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let (done, wait) = mpsc::channel::<()>(); + let app = std::thread::spawn(move || { + let mut open = Vec::new(); + for reply in &replies { + let (stream, _) = listener.accept().unwrap(); + let mut out = stream.try_clone().unwrap(); + let mut line = String::new(); + BufReader::new(stream).read_line(&mut line).unwrap(); + out.write_all(reply).unwrap(); + out.flush().unwrap(); + open.push(out); + } + wait.recv_timeout(Duration::from_secs(30)).ok(); + open.len() + }); + (port, done, app) +} + +/// Let the fake app close its connections, and check it read `requests` requests. +fn finish(done: mpsc::Sender<()>, app: std::thread::JoinHandle, requests: usize) { + drop(done); + assert_eq!(app.join().unwrap(), requests); +} + +fn ok_reply(text: &str) -> Vec { + format!("{}\n", json!({"id": 1, "ok": true, "result": text})).into_bytes() +} + +#[test] +fn an_oversized_reply_is_refused_and_the_next_call_reconnects() { + let (port, done, fake) = fake_app(vec![vec![b'x'; 200], ok_reply("fine")]); + let mut app = Attached::port(port).unwrap(); + app.max_reply = 64; + let err = app.call("engine.commands", json!({})).unwrap_err(); + assert!(err.contains("reply larger than"), "{err}"); + assert!(err.contains("may have completed"), "{err}"); + assert!(app.conn.is_none()); + assert_eq!(app.call("engine.commands", json!({})).unwrap(), json!("fine")); + finish(done, fake, 2); +} + +#[test] +fn a_reply_at_the_limit_is_read() { + let reply = ok_reply("at the limit"); + let (port, done, fake) = fake_app(vec![reply.clone()]); + let mut app = Attached::port(port).unwrap(); + app.max_reply = reply.len() - 1; // the newline doesn't count + assert_eq!(app.call("engine.commands", json!({})).unwrap(), json!("at the limit")); + finish(done, fake, 1); +} + +#[test] +fn a_reply_that_is_not_utf8_is_an_error() { + let (port, done, fake) = fake_app(vec![b"{\"ok\": true, \"result\": \"\xff\"}\n".to_vec()]); + let mut app = Attached::port(port).unwrap(); + let err = app.call("engine.commands", json!({})).unwrap_err(); + assert!(err.contains("failed"), "{err}"); + assert!(app.conn.is_none()); + finish(done, fake, 1); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 448a9979..2f211950 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -29,6 +29,8 @@ active connections**; additional connections are closed immediately. Socket read have a **60 s inactivity timeout**, so idle clients and clients that stop reading release their worker. This bounds blocked socket operations, not the total duration of a frame arriving slowly. Clients can reconnect after an idle timeout (the attached MCP backend already does). +Replies have no size limit on the app's side. The attached MCP backend reads them up to **512 MiB +before the newline** and drops the connection on a longer one (see [MCP server](mcp.md)). Both the socket queue and the UI queue hold at most **8 requests** each, including requests waiting for another window's frame. A full socket queue replies with `"control request queue is full"` diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..cbf989cf 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -23,6 +23,10 @@ Attached calls are never automatically replayed after a transport failure: the a applied the request before its reply was lost. The error reports that the operation may have completed; inspect the document before retrying an edit. The next explicit call reconnects. If that connection cannot be established, the error says the request was not sent. +A reply from the app is read up to **512 MiB before the newline** (the largest window capture is +256 MiB as base64). The backend stops reading a longer reply at the first byte past the limit and +treats it like a lost reply: the connection is closed, the operation may have completed, and the +next call reconnects. For larger exports, give `export` a `path` or lower the scale. A connection the app closed while it sat idle is noticed before the next request is written and replaced then, so a pause between calls is not an error. diff --git a/docs/roadmap.md b/docs/roadmap.md index 14d734ba..d9df3e02 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # FigCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (the attached MCP server reads the app's replies with a 512 MiB bound (#1113); Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) Forward-looking plan. Where FigCraft stands today: [`ROADMAP.md`](../ROADMAP.md) and [`target-app-parity.md`](target-app-parity.md); the full ranked list: [`gaps.md`](gaps.md). Hours @@ -75,6 +75,7 @@ blockers. | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | Bounded attached MCP replies (M8, #1113): `figcraft-cli mcp --attach` read each reply from the app's control port with no size limit, so a process listening on that port could make it buffer until memory ran out. Replies are now read up to 512 MiB (an app window capture is at most 256 MiB as base64); reading stops at the first byte past that, the connection is closed and the call is reported like any other lost reply, and the next call reconnects. Regression tests in the MCP backend tests. Numbers unchanged | | 2026-10-26 | minor | Dragging a selected unfilled shape (M1, #1370): a press inside a selected stroke-only shape over a filled layer stacked below it moved that layer; the drag-from-inside fallback of #910 now treats any layer painted below the selection like the frame or canvas behind it, so the selection drags, while a layer painted above it still takes the press. Regression test in the selection behaviour suite. Numbers unchanged | | 2026-10-26 | minor | Text truncation (M4, #1375, #1376, #1377): the last line shown takes as much of the next word as fits before the ellipsis ("Lorem consec…", not "Lorem…"); an explicit height, `transform.resize`, a resize handle or fixed sizing on a truncated text keeps it truncating instead of turning it fixed size; Max lines on auto-height text hugs up to that many lines and ends in an ellipsis (`text.maxLines`, `node.set {maxLines}`, Type settings, `.fig` export and import), and `node.get` / `get_node` report `maxLines`. Regression tests in the text and engine behaviour suites. Numbers unchanged | | 2026-10-24 | minor | Installed weights of an embedded font family (M7, #1246): release builds embed some families with only a few styles (Poppins Regular, Italic and Bold), and the first font scan dropped every installed face of such a family, so an installed Poppins SemiBold or Medium was never offered (a rescan did offer them). The first scan now keeps the embedded faces, which still win for the styles they cover, and offers the installed styles they lack beside them. Regression test in the font database tests. Numbers unchanged | From 3b78ca9558278261cee2370cc399e99237c2b217 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:57:38 +0100 Subject: [PATCH 3/7] Treat a % suffix on node.set opacity as a percentage Found while testing crates/engine/src/cmd/props.rs:476-479 (the "opacity" arm), with crates/engine/src/automation/params.rs:14-20 (f64_of) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/props.rs | 3 ++- crates/engine/src/panels_behaviour_tests.rs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 30a10393..723dd717 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -475,7 +475,8 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map doc.get_mut(id)?.locked = v.as_bool().ok_or_else(|| bad(cmd, "`locked` must be a boolean"))?, "opacity" => { let o = f64_of(v).ok_or_else(|| bad(cmd, "`opacity` must be a number"))?; - let o = if o > 1.0 { o / 100.0 } else { o }; + let percent = v.as_str().is_some_and(|s| s.trim().ends_with('%')); + let o = if percent || o > 1.0 { o / 100.0 } else { o }; doc.get_mut(id)?.opacity = o.clamp(0.0, 1.0) as f32; } "blend" | "blendMode" => { diff --git a/crates/engine/src/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index 9870aa31..202fc4f2 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -475,3 +475,14 @@ fn page_navigation_skips_dividers() { ex(&mut s, "view.nextPage", json!({})); assert_eq!(s.page().unwrap(), later, "Next page stops on the last page"); } + +#[test] +fn opacity_percent_strings_always_divide_by_100() { + let mut s = Session::new(); + let r = rect(&mut s, "R", 0.0, 0.0, None); + for (v, want) in [(json!("1%"), 0.01), (json!("0.5%"), 0.005), (json!("50%"), 0.5), (json!(1), 1.0), (json!(50), 0.5), (json!(0.25), 0.25)] { + ex(&mut s, "node.set", json!({"id": r, "props": {"opacity": v.clone()}})); + let o = s.doc().unwrap().node(r).unwrap().opacity; + assert!((o - want).abs() < 1e-6, "opacity {v} gave {o}, want {want}"); + } +} From 9d4a3c6d8674e3916c401c26da9b178c68ba81b6 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:15:17 +0000 Subject: [PATCH 4/7] Reject colour arrays containing non-numeric entries in color_of Found while testing crates/engine/src/automation/params.rs:170 (color_of, Value::Array arm) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/automation/params.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/automation/params.rs b/crates/engine/src/automation/params.rs index 35a74e0e..222348a4 100644 --- a/crates/engine/src/automation/params.rs +++ b/crates/engine/src/automation/params.rs @@ -167,7 +167,7 @@ pub fn color_of(v: &Value) -> Option { Value::String(s) => Color::parse_css(s), Value::Object(_) => serde_json::from_value::(v.clone()).ok().map(Color::sanitized), Value::Array(a) => { - let c: Vec = a.iter().filter_map(|x| x.as_f64()).map(|f| f as f32).collect(); + let c = a.iter().map(|x| x.as_f64().map(|f| f as f32)).collect::>>()?; match c.as_slice() { [r, g, b] => Some(Color::new(*r, *g, *b, 1.0)), [r, g, b, a] => Some(Color::new(*r, *g, *b, *a)), @@ -278,3 +278,16 @@ pub fn parse(cmd: &str, p: &Value, key: &str) -> Some(v) => serde_json::from_value(v.clone()).map(Some).map_err(|e| bad(cmd, format!("`{key}`: {e}"))), } } + +#[cfg(test)] +mod color_of_tests { + use super::*; + use serde_json::json; + + #[test] + fn color_of_rejects_non_numeric_array_entries() { + assert_eq!(color_of(&json!([1, "x", 0, 0])), None); + assert_eq!(color_of(&json!([1, 0, 0, "x"])), None); + assert_eq!(color_of(&json!([1, 0, 0, 0.5])), Some(Color::new(1.0, 0.0, 0.0, 0.5))); + } +} From 2d02f289ec849d26c1ad4a461c3a3a956e70fcb6 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 02:16:00 +0100 Subject: [PATCH 5/7] Keep session joins working when a document has out-of-range ids Fixes #1067 Co-Authored-By: Claude Opus 5.5 --- crates/collab/src/entry_ids_tests.rs | 31 ++++++++++++++++++++++++++++ crates/collab/src/server.rs | 16 +++++++++----- crates/doc/src/ids.rs | 5 +++-- crates/doc/src/ids_tests.rs | 4 ++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/crates/collab/src/entry_ids_tests.rs b/crates/collab/src/entry_ids_tests.rs index db58bb2f..976d2cb6 100644 --- a/crates/collab/src/entry_ids_tests.rs +++ b/crates/collab/src/entry_ids_tests.rs @@ -92,6 +92,37 @@ fn client_ids_skip_spaces_used_by_styles_and_comments() { assert_eq!(join(&mut server).id(), 10); } +#[test] +fn an_id_in_the_top_space_does_not_block_joins() { + let mut d = Document::new("a"); + d.styles.push(style(format!("{STYLE_ID_PREFIX}{}", u64::MAX), "Out of range")); + let c = comment(u64::from(u32::MAX) << 32, &d); + d.comments.push(c); + let mut server = Server::new(d, "", Limits::default()); + let a = join(&mut server); + assert!(a.id() > 0 && a.id() < u32::MAX, "{}", a.id()); + // After a reload the session still takes newcomers and keeps the document as it was. + let state = server.state().unwrap(); + let mut restored = Server::restore(state.clone(), "", Limits::default()).unwrap(); + let b = join(&mut restored); + assert!(b.id() > a.id() && b.id() < u32::MAX, "{}", b.id()); + assert_eq!(restored.document().styles.len(), 1); + assert_eq!(restored.document().comments.len(), 1); + // A state saved while the counter was stuck at the top recovers too. + let stuck = crate::SessionState { next_client: u32::MAX, ..state }; + let mut restored = Server::restore(stuck, "", Limits::default()).unwrap(); + let c = join(&mut restored); + assert!(c.id() > a.id() && c.id() < u32::MAX, "{}", c.id()); + // A new entry may not take an id from a space nobody was given yet. + for (k, space) in [c.id() + 1, c.id() + 1000, u32::MAX].into_iter().enumerate() { + let id = format!("{STYLE_ID_PREFIX}{}", (u64::from(space) << 32) | 1); + let ServerMsg::Reject { reason, .. } = send(&mut restored, c.id(), 1 + k as u64, vec![put_style(id.clone())]) else { + panic!("accepted {id}") + }; + assert!(reason.contains("id space"), "{reason}"); + } +} + /// Each client makes `rounds` styles, collections (with an extra mode), variables and comments /// before seeing anyone else's. fn create_everything(doc: &mut Document, who: usize, round: usize) { diff --git a/crates/collab/src/server.rs b/crates/collab/src/server.rs index 52f112ad..54089af6 100644 --- a/crates/collab/src/server.rs +++ b/crates/collab/src/server.rs @@ -153,7 +153,13 @@ impl Server { server.state = replica; server.session = state.session; server.seq = state.seq; - server.next_client = server.next_client.max(state.next_client); + // A counter stuck at the top (saved while an out-of-range id in the document blocked + // every join) carries on above the ids actually handed out. + let saved = match state.next_client { + u32::MAX => state.issued.keys().chain(state.log.iter().map(|a| &a.author)).max().map_or(0, |c| c.saturating_add(1)), + n => n, + }; + server.next_client = server.next_client.max(saved); server.issued = state.issued.into_iter().filter(|(id, _)| *id < server.next_client).collect(); server.deleted = state.deleted; server.removed = state.removed; @@ -481,9 +487,9 @@ impl Server { /// The document-level entries (styles, variables, collections, comments) a batch adds or /// removes, each with whether it is there afterwards. A new one is refused when its id lies in - /// the space of another client this session gave an id to (unless it was removed before: undo - /// puts entries back); ids from before the session (space 0, earlier sessions, other files) - /// are fine. + /// the space of another client this session gave an id to, or of one it hasn't given out yet + /// (unless it was removed before: undo puts entries back); ids from before the session (space + /// 0, earlier sessions, other files) are fine. fn check_entries(&self, from: u32, changes: &[Change], after: &Document) -> Result, Error> { let before = self.state.document(); let mut touched: BTreeSet = BTreeSet::new(); @@ -510,7 +516,7 @@ impl Server { } if now && !self.removed.contains(&entry) - && let Some(space) = entry.id_space().filter(|s| *s != from && self.issued.contains_key(s)) + && let Some(space) = entry.id_space().filter(|s| *s != from && (self.issued.contains_key(s) || *s >= self.next_client)) { return Err(Error::Invalid(format!("new {} isn't in client {from}'s id space (it is client {space}'s)", entry.describe()))); } diff --git a/crates/doc/src/ids.rs b/crates/doc/src/ids.rs index 024946bb..7e244fbb 100644 --- a/crates/doc/src/ids.rs +++ b/crates/doc/src/ids.rs @@ -128,7 +128,8 @@ impl Document { /// The highest client id space any id in the document lies in (layers, comments, styles, /// variables, collections, modes): a session hands out client ids above it, so ids made in - /// earlier sessions are never made again. + /// earlier sessions are never made again. Space `u32::MAX` doesn't count: numbers too big for + /// any client's space land there ([`space_of`]) and no session hands that client id out. pub fn highest_id_space(&self) -> u32 { let nodes = self.ids().map(|id| id.0); let comments = self.comments.iter().map(|c| c.id); @@ -136,6 +137,6 @@ impl Document { let vars = self.variables.variables.iter().filter_map(|v| id_number(&v.id, VARIABLE_ID_PREFIX)); let colls = self.variables.collections.iter().filter_map(|c| id_number(&c.id, COLLECTION_ID_PREFIX)); let modes = self.variables.collections.iter().flat_map(|c| c.modes.iter().filter_map(|m| mode_number(&m.id))); - nodes.chain(comments).chain(styles).chain(vars).chain(colls).chain(modes).map(space_of).max().unwrap_or(0) + nodes.chain(comments).chain(styles).chain(vars).chain(colls).chain(modes).map(space_of).filter(|s| *s != u32::MAX).max().unwrap_or(0) } } diff --git a/crates/doc/src/ids_tests.rs b/crates/doc/src/ids_tests.rs index 4f489886..7c6a50e0 100644 --- a/crates/doc/src/ids_tests.rs +++ b/crates/doc/src/ids_tests.rs @@ -103,6 +103,10 @@ fn the_highest_space_counts_every_kind_of_id() { assert_eq!(d.highest_id_space(), 9); d.comments.push(comment(11u64 << 32, d.first_page().unwrap())); assert_eq!(d.highest_id_space(), 11); + // Numbers past every client's space (they saturate to `u32::MAX`) are no session's. + d.styles.push(style(&format!("S:{}", u64::MAX))); + d.comments.push(comment(u64::from(u32::MAX) << 32, d.first_page().unwrap())); + assert_eq!(d.highest_id_space(), 11); // Other forms are space 0. assert_eq!(id_space("lib:S:99999999999", "S:"), 0); assert_eq!(id_space("S:+5", "S:"), 0); From 08a36418a65bfa3ebad5a9d571ef6e5bf2bd2fa7 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:31:37 +0700 Subject: [PATCH 6/7] #1444: accept imported entries from unissued spaces; reserve only the top half (NO_SESSION_SPACE) --- crates/collab/src/entry_ids_tests.rs | 21 +++++++++++++++++++-- crates/collab/src/server.rs | 14 ++++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/crates/collab/src/entry_ids_tests.rs b/crates/collab/src/entry_ids_tests.rs index 976d2cb6..72a147ae 100644 --- a/crates/collab/src/entry_ids_tests.rs +++ b/crates/collab/src/entry_ids_tests.rs @@ -6,6 +6,7 @@ use figcraft_doc::{Collection, Comment, Document, Message, Mode, Style, StyleKin use crate::change::{Change, Entry, Json}; use crate::message::{ClientMsg, ServerMsg}; +use crate::server::NO_SESSION_SPACE; use crate::sim::Sim; use crate::{Client, Limits, NoRecompute, Outgoing, Server}; @@ -113,8 +114,8 @@ fn an_id_in_the_top_space_does_not_block_joins() { let mut restored = Server::restore(stuck, "", Limits::default()).unwrap(); let c = join(&mut restored); assert!(c.id() > a.id() && c.id() < u32::MAX, "{}", c.id()); - // A new entry may not take an id from a space nobody was given yet. - for (k, space) in [c.id() + 1, c.id() + 1000, u32::MAX].into_iter().enumerate() { + // A new entry may not take an id from the reserved top half of the spaces. + for (k, space) in [NO_SESSION_SPACE, u32::MAX].into_iter().enumerate() { let id = format!("{STYLE_ID_PREFIX}{}", (u64::from(space) << 32) | 1); let ServerMsg::Reject { reason, .. } = send(&mut restored, c.id(), 1 + k as u64, vec![put_style(id.clone())]) else { panic!("accepted {id}") @@ -123,6 +124,22 @@ fn an_id_in_the_top_space_does_not_block_joins() { } } +#[test] +fn styles_imported_from_another_session_are_accepted_and_joins_go_on() { + let mut server = Server::new(Document::new("a"), "", Limits::default()); + let a = join(&mut server); + // `file.import` copies styles with their ids, made by clients this session hasn't given out + // (up to the highest space a member may use). + for (k, space) in [a.id() + 1, a.id() + 1000, NO_SESSION_SPACE - 1].into_iter().enumerate() { + let id = format!("{STYLE_ID_PREFIX}{}", (u64::from(space) << 32) | 1); + assert!(matches!(send(&mut server, a.id(), 1 + k as u64, vec![put_style(id.clone())]), ServerMsg::Changes(_)), "refused {id}"); + } + // After a restart client ids start above them, and there are plenty left. + let mut restored = Server::restore(server.state().unwrap(), "", Limits::default()).unwrap(); + assert_eq!(join(&mut restored).id(), NO_SESSION_SPACE); + assert_eq!(join(&mut Server::new(restored.document().clone(), "", Limits::default())).id(), NO_SESSION_SPACE); +} + /// Each client makes `rounds` styles, collections (with an extra mode), variables and comments /// before seeing anyone else's. fn create_everything(doc: &mut Document, who: usize, round: usize) { diff --git a/crates/collab/src/server.rs b/crates/collab/src/server.rs index c45b0ba6..d8aab594 100644 --- a/crates/collab/src/server.rs +++ b/crates/collab/src/server.rs @@ -25,6 +25,12 @@ pub enum Outgoing { /// Why a viewer's batch is refused. pub const VIEW_ONLY: &str = "you can only view this file (your link is view-only)"; +/// A new style, variable, collection or comment may not take an id in a space at or above this +/// that isn't its creator's. Client ids start above every space in the document after a restart, +/// so no member can use them up: at least this many always remain. Lower spaces stay open, for +/// entries imported from other files' sessions. +pub const NO_SESSION_SPACE: u32 = 1 << 31; + /// What changed a server's lasting state, in order: what a store appends to its log between /// snapshots ([`Server::journaling`]) and replays over the last snapshot after a restart /// ([`Server::replay`]). Presence and who is connected aren't in it. @@ -506,9 +512,9 @@ impl Server { /// The document-level entries (styles, variables, collections, comments) a batch adds or /// removes, each with whether it is there afterwards. A new one is refused when its id lies in - /// the space of another client this session gave an id to, or of one it hasn't given out yet - /// (unless it was removed before: undo puts entries back); ids from before the session (space - /// 0, earlier sessions, other files) are fine. + /// the space of another client this session gave an id to, or in a space at or above + /// [`NO_SESSION_SPACE`] (unless it was removed before: undo puts entries back); ids from + /// before the session (space 0, earlier sessions, other files) are fine. fn check_entries(&self, from: u32, changes: &[Change], after: &Document) -> Result, Error> { let before = self.state.document(); let mut touched: BTreeSet = BTreeSet::new(); @@ -535,7 +541,7 @@ impl Server { } if now && !self.removed.contains(&entry) - && let Some(space) = entry.id_space().filter(|s| *s != from && (self.issued.contains_key(s) || *s >= self.next_client)) + && let Some(space) = entry.id_space().filter(|s| *s != from && (self.issued.contains_key(s) || *s >= NO_SESSION_SPACE)) { return Err(Error::Invalid(format!("new {} isn't in client {from}'s id space (it is client {space}'s)", entry.describe()))); } From 069449e33c3164026b8637ddd68a0f70c79f4090 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:18:57 +0700 Subject: [PATCH 7/7] #1425: share fraction_of for opacity, fill opacity, innerRadius and arcInner --- crates/engine/src/automation/params.rs | 7 +++++++ crates/engine/src/cmd/edit.rs | 4 ++-- crates/engine/src/cmd/edit/props.rs | 8 ++++---- crates/engine/src/panels_behaviour_tests.rs | 15 +++++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/automation/params.rs b/crates/engine/src/automation/params.rs index 35a74e0e..f07e4d86 100644 --- a/crates/engine/src/automation/params.rs +++ b/crates/engine/src/automation/params.rs @@ -20,6 +20,13 @@ pub fn f64_of(v: &Value) -> Option { } } +/// A 0-1 fraction: a `%` suffix or a number above 1 is a percentage ("1%" is 0.01, 50 is 0.5). +pub fn fraction_of(v: &Value) -> Option { + let f = f64_of(v)?; + let percent = v.as_str().is_some_and(|s| s.trim().ends_with('%')); + Some(if percent || f > 1.0 { f / 100.0 } else { f }) +} + pub fn num(p: &Value, key: &str) -> Option { get(p, key).and_then(f64_of) } diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 2d51b958..d24600c9 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -8,8 +8,8 @@ use serde_json::{Map, Value, json}; use super::{CommandSpec, ok}; use crate::params::{ - blend, boolean, color, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, string, - targets, + blend, boolean, color, fraction_of, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, + string, targets, }; use crate::{BatchStep, EngineError, Result, Session, bad, spec}; diff --git a/crates/engine/src/cmd/edit/props.rs b/crates/engine/src/cmd/edit/props.rs index 91855c2e..6d0deb29 100644 --- a/crates/engine/src/cmd/edit/props.rs +++ b/crates/engine/src/cmd/edit/props.rs @@ -1,6 +1,6 @@ use super::{ - BTreeMap, Effect, EngineError, Map, Node, NodeId, NodeKind, Result, Session, Value, bad, blend, boolean, color, get, json, num, ok, opt_index, - paint_of, parse, req_id, req_index, req_string, req_targets, roots, string, targets, z_sorted, + BTreeMap, Effect, EngineError, Map, Node, NodeId, NodeKind, Result, Session, Value, bad, blend, boolean, color, fraction_of, get, json, num, ok, + opt_index, paint_of, parse, req_id, req_index, req_string, req_targets, roots, string, targets, z_sorted, }; pub(super) fn batch_rename(s: &mut Session, p: &Value) -> Result { @@ -312,8 +312,8 @@ pub(super) fn paint_set(s: &mut Session, p: &Value, fills: bool) -> Result 1.0 { o / 100.0 } else { o }).clamp(0.0, 1.0) as f32; + if let Some(o) = get(p, "opacity").and_then(fraction_of) { + slot.opacity = o.clamp(0.0, 1.0) as f32; slot.color_alpha = 1.0; } if let Some(v) = boolean(p, "visible") { diff --git a/crates/engine/src/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index 202fc4f2..d97d2507 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -486,3 +486,18 @@ fn opacity_percent_strings_always_divide_by_100() { assert!((o - want).abs() < 1e-6, "opacity {v} gave {o}, want {want}"); } } + +#[test] +fn fill_opacity_star_ratio_and_arc_ratio_read_a_percent_suffix() { + let mut s = Session::new(); + let r = rect(&mut s, "R", 0.0, 0.0, None); + ex(&mut s, "fill.set", json!({"ids": [r], "color": "#ff0000", "opacity": "1%"})); + let fill = s.doc().unwrap().node(r).unwrap().fills[0].opacity; + assert!((fill - 0.01).abs() < 1e-6, "fill opacity \"1%\" gave {fill}"); + let star = id(&ex(&mut s, "shape.star", json!({"x": 0, "y": 0, "width": 100, "height": 100, "props": {"innerRadius": "1%"}}))); + let figcraft_doc::NodeKind::Star { inner_ratio, .. } = s.doc().unwrap().node(star).unwrap().kind else { panic!("star") }; + assert!((inner_ratio - 0.01).abs() < 1e-9, "innerRadius \"1%\" gave {inner_ratio}"); + let arc = id(&ex(&mut s, "shape.ellipse", json!({"x": 0, "y": 0, "width": 100, "height": 100, "props": {"arcInner": "1%"}}))); + let figcraft_doc::NodeKind::Ellipse { inner, .. } = s.doc().unwrap().node(arc).unwrap().kind else { panic!("ellipse") }; + assert!((inner - 0.01).abs() < 1e-9, "arcInner \"1%\" gave {inner}"); +}