mirror of
https://github.com/storytold/designcraft.git
synced 2026-10-14 19:27:07 -04:00
Commands: huge row and column counts can't exhaust memory (#137)
Three commands took a count from their parameters and used it unchecked. Parameters come from MCP, the control channel and scripts, so the count is hostile input (never-crash: cap input-sized allocations, checked arithmetic). - layout.createGuides made one or two guides per row and column with no upper limit: 30 million rows took 1.2 GB, and 2^32 - 1 would exhaust memory, which aborts the process past the command guard. A count over 2^32 wrapped to a small one. Both counts are now capped at 100. - edit.stepAndRepeat multiplied rows by columns as usize. In debug builds the overflow panicked; in release builds 2^32 x 2^32 wraps to 0, passes the 1000-copy limit and starts a grid of 2^64 copies. The product is now checked, and a grid that large falls back to `count` copies like any other grid over 1000. - file.new stored any column count in the page (4 billion, or a wrapped value past 2^32). Page::column_rects builds one rect per column each time the columns are laid out. The count is now clamped to 1..=216, the range layout.marginsAndColumns already uses. Each case has a regression test that failed before the fix. Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
1 parent
fe16e00e37
commit
ce3deb651a
4 files changed
+43
-5
No files matched your search
@@ -124,7 +124,7 @@ fn file_new(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
nd.height = f64_or(p, "height", nd.height);
|
||||
nd.pages = p.get("pages").and_then(Value::as_u64).map(|v| v as usize).unwrap_or(nd.pages).clamp(1, 9999);
|
||||
nd.facing_pages = p.get("facingPages").and_then(Value::as_bool).unwrap_or(nd.facing_pages);
|
||||
nd.columns = p.get("columns").and_then(Value::as_u64).map(|v| v as u32).unwrap_or(nd.columns);
|
||||
nd.columns = p.get("columns").and_then(Value::as_u64).map(|v| v.clamp(1, 216) as u32).unwrap_or(nd.columns);
|
||||
nd.gutter = f64_or(p, "gutter", nd.gutter);
|
||||
nd.primary_text_frame = p.get("primaryTextFrame").and_then(Value::as_bool).unwrap_or(false);
|
||||
match p.get("margins") {
|
||||
|
||||
@@ -51,7 +51,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
"Create Guides…",
|
||||
["Layout"],
|
||||
None,
|
||||
"{rows?: 0, columns?: 0, rowGutter?: 12, columnGutter?: 12, fitTo?: margins|page, removeExisting?: false, spread?, page? (index in the spread; default all pages)}",
|
||||
"{rows?: 0, columns?: 0 (at most 100 each), rowGutter?: 12, columnGutter?: 12, fitTo?: margins|page, removeExisting?: false, spread?, page? (index in the spread; default all pages)}",
|
||||
has_doc,
|
||||
create_guides
|
||||
),
|
||||
@@ -157,9 +157,13 @@ pub(crate) fn grid_positions(a: f64, b: f64, n: u32, gutter: f64) -> Vec<f64> {
|
||||
v
|
||||
}
|
||||
|
||||
/// The most rows or columns Create Guides lays out. The count comes from the caller, and every
|
||||
/// guide is stored in the document.
|
||||
const MAX_GUIDE_GRID: u32 = 100;
|
||||
|
||||
fn create_guides(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let rows = p.get("rows").and_then(Value::as_u64).unwrap_or(0) as u32;
|
||||
let cols = p.get("columns").and_then(Value::as_u64).unwrap_or(0) as u32;
|
||||
let count = |key: &str| p.get(key).and_then(Value::as_u64).unwrap_or(0).min(u64::from(MAX_GUIDE_GRID)) as u32;
|
||||
let (rows, cols) = (count("rows"), count("columns"));
|
||||
let rg = p.get("rowGutter").and_then(Value::as_f64).unwrap_or(12.0);
|
||||
let cg = p.get("columnGutter").and_then(Value::as_f64).unwrap_or(12.0);
|
||||
let to_page = str_param(p, "fitTo") == Some("page");
|
||||
@@ -238,6 +242,18 @@ mod tests {
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert_eq!(s.doc().unwrap().doc.spreads[0].pages[0].guides.len(), 5);
|
||||
}
|
||||
|
||||
/// A huge row or column count once made that many guides (2³² − 1 of them exhaust memory, which
|
||||
/// no guard catches), and a count past 2³² wrapped to a small one.
|
||||
#[test]
|
||||
fn create_guides_caps_the_row_and_column_counts() {
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", &json!({})).unwrap();
|
||||
let args = json!({"spread": 0, "rows": 4_294_967_298_u64, "columns": 3_000_000, "rowGutter": 0.0, "columnGutter": 0.0});
|
||||
let r = s.execute("layout.createGuides", &args).unwrap();
|
||||
let per_page = 2 * (u64::from(MAX_GUIDE_GRID) - 1);
|
||||
assert_eq!(r["guides"], per_page * s.doc().unwrap().doc.spreads[0].pages.len() as u64);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -1919,7 +1919,7 @@ fn step_and_repeat(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let cols = p.get("columns").and_then(Value::as_u64).map(|v| v as usize);
|
||||
let count = p.get("count").and_then(Value::as_u64).unwrap_or(1).clamp(1, 1000) as usize;
|
||||
let offsets: Vec<Vec2> = match (rows, cols) {
|
||||
(Some(r), Some(c)) if r * c <= 1000 => (0..r)
|
||||
(Some(r), Some(c)) if r.checked_mul(c).is_some_and(|n| n <= 1000) => (0..r)
|
||||
.flat_map(|i| (0..c).map(move |j| (i, j)))
|
||||
.filter(|&(i, j)| i + j > 0)
|
||||
.map(|(i, j)| Vec2::new(j as f64 * dx, i as f64 * dy))
|
||||
|
||||
@@ -372,6 +372,28 @@ fn step_and_repeat_grid() {
|
||||
assert_eq!(last.y0, 36.0 + 2.0 * 40.0);
|
||||
}
|
||||
|
||||
/// rows × columns once overflowed: a panic in debug builds, and in release builds a product that
|
||||
/// wrapped to 0 passed the 1000-copy limit and started a grid of 2⁶⁴ copies.
|
||||
#[test]
|
||||
fn step_and_repeat_with_a_huge_grid_makes_no_grid() {
|
||||
let mut s = session();
|
||||
s.execute("frame.create", &json!({"rect": [36, 36, 66, 66], "content": "unassigned"})).unwrap();
|
||||
let r = s.execute("edit.stepAndRepeat", &json!({"rows": 4_294_967_296_u64, "columns": 4_294_967_296_u64})).unwrap();
|
||||
// As for any grid of more than 1000 copies: `count` (default 1) copies instead.
|
||||
assert_eq!(r["created"], 1);
|
||||
}
|
||||
|
||||
/// The column count is the caller's and sizes a list every time the page's columns are laid out.
|
||||
#[test]
|
||||
fn new_document_caps_the_column_count() {
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", &json!({"columns": 4_000_000_000_u64})).unwrap();
|
||||
assert_eq!(s.doc().unwrap().doc.spreads[0].pages[0].columns.count, 216);
|
||||
// Past u32 the count wrapped (2³² + 3 became 3).
|
||||
s.execute("file.new", &json!({"columns": 4_294_967_299_u64})).unwrap();
|
||||
assert_eq!(s.doc().unwrap().doc.spreads[0].pages[0].columns.count, 216);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn snippets_roundtrip_between_documents() {
|
||||
let mut s = Session::new();
|
||||
|
||||
Reference in new issue
Block a user