From 3e729bbf0862ec3604543edfdb6dc3ac20b5bde3 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:14:09 +0200 Subject: [PATCH 1/2] Limit shape.setBounds, shape.resize and shape.move size and position Found while testing crates/engine/src/cmd/shape.rs:215 (set_bounds; shape.resize at :~183 and shape.move at :~171 have the same problem) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/shape.rs | 24 ++++++++++++++++++------ crates/engine/src/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index e3bf3920..10c737ee 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -169,13 +169,23 @@ pub fn specs() -> Vec { ] } +/// Keeps a side length in `0..=MAX_PT` so a huge value cannot exhaust memory when rendering. +fn clamp_side(v: f64) -> f64 { + if v.is_finite() { v.clamp(0.0, MAX_PT) } else { 0.0 } +} + +/// Keeps a coordinate within `±POS_LIMIT`. +fn clamp_pos(v: f64) -> f64 { + if v.is_finite() { v.clamp(-POS_LIMIT, POS_LIMIT) } else { 0.0 } +} + fn move_by(s: &mut Session, p: &Value) -> Result { let (dx, dy) = (f64_param(p, "dx"), f64_param(p, "dy")); let (ax, ay) = (f64_param(p, "x"), f64_param(p, "y")); edit_geometry(s, p, "shape.move", |sh| { if let Some(x) = sh.xfrm.as_mut() { - x.x = ax.unwrap_or(x.x) + dx.unwrap_or(0.0); - x.y = ay.unwrap_or(x.y) + dy.unwrap_or(0.0); + x.x = clamp_pos(ax.unwrap_or(x.x) + dx.unwrap_or(0.0)); + x.y = clamp_pos(ay.unwrap_or(x.y) + dy.unwrap_or(0.0)); } Ok(()) }) @@ -202,6 +212,8 @@ fn resize(s: &mut Session, p: &Value) -> Result { x.h = h.unwrap_or(x.h); } } + x.w = clamp_side(x.w); + x.h = clamp_side(x.h); // Rotated shapes resize about their centre; others keep the top-left. if x.rot != 0.0 { x.x = cx - x.w / 2.0; @@ -215,10 +227,10 @@ fn resize(s: &mut Session, p: &Value) -> Result { fn set_bounds(s: &mut Session, p: &Value) -> Result { edit_geometry(s, p, "shape.setBounds", |sh| { if let Some(x) = sh.xfrm.as_mut() { - x.x = f64_param(p, "x").unwrap_or(x.x); - x.y = f64_param(p, "y").unwrap_or(x.y); - x.w = f64_param(p, "w").unwrap_or(x.w).max(0.0); - x.h = f64_param(p, "h").unwrap_or(x.h).max(0.0); + x.x = clamp_pos(f64_param(p, "x").unwrap_or(x.x)); + x.y = clamp_pos(f64_param(p, "y").unwrap_or(x.y)); + x.w = clamp_side(f64_param(p, "w").unwrap_or(x.w)); + x.h = clamp_side(f64_param(p, "h").unwrap_or(x.h)); } Ok(()) }) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index a464fd8b..b8cf0ace 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3623,3 +3623,28 @@ fn table_column_width_of_a_huge_column_is_ignored() { assert_eq!(table_of(&s, id).cols, vec![100.0, 100.0], "no column changes"); } } + +#[test] +fn shape_geometry_commands_limit_size_and_position() { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + s.execute("shape.insert", &json!({"preset": "moon", "rect": [100, 100, 200, 100]})).unwrap(); + let xfrm = |s: &Session| s.doc().unwrap().selected_shapes()[0].xfrm.unwrap(); + let within = |s: &Session| { + let x = xfrm(s); + [x.w, x.h].iter().all(|v| v.is_finite() && (0.0..=15_840.0).contains(v)) && [x.x, x.y].iter().all(|v| v.is_finite() && v.abs() <= 100_000.0) + }; + s.execute("shape.setBounds", &json!({"x": 1e100, "y": -1e100, "w": 100000, "h": 1e100})).unwrap(); + let x = xfrm(&s); + assert!(within(&s), "setBounds stored {x:?}"); + s.execute("shape.resize", &json!({"w": 10, "h": 10})).unwrap(); + s.execute("shape.resize", &json!({"h": 1e100})).unwrap(); + let x = xfrm(&s); + assert!(within(&s), "resize stored {x:?}"); + s.execute("shape.resize", &json!({"h": 1e100, "lockAspect": true})).unwrap(); + let x = xfrm(&s); + assert!(within(&s), "locked resize stored {x:?}"); + s.execute("shape.move", &json!({"x": 1e100, "y": -1e100})).unwrap(); + let x = xfrm(&s); + assert!(within(&s), "move stored {x:?}"); +} From efc279d6f20af1de9b6856a603fa2e343bb2083a Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 18 Oct 2026 11:18:16 -0400 Subject: [PATCH 2/2] Clamp overflowing geometry to the limit and keep rotated resizes in range An overflow to infinity (move x=1e308 dx=1e308) now lands on the limit instead of resetting to 0, and a rotated shape re-centred after a capped resize stays within POS_LIMIT (CodeRabbit review). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/shape.rs | 9 +++++---- crates/engine/src/tests.rs | 9 +++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index 10c737ee..12c44b8b 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -170,13 +170,14 @@ pub fn specs() -> Vec { } /// Keeps a side length in `0..=MAX_PT` so a huge value cannot exhaust memory when rendering. +/// An overflow to infinity lands on the limit; NaN becomes 0. fn clamp_side(v: f64) -> f64 { - if v.is_finite() { v.clamp(0.0, MAX_PT) } else { 0.0 } + if v.is_nan() { 0.0 } else { v.clamp(0.0, MAX_PT) } } /// Keeps a coordinate within `±POS_LIMIT`. fn clamp_pos(v: f64) -> f64 { - if v.is_finite() { v.clamp(-POS_LIMIT, POS_LIMIT) } else { 0.0 } + if v.is_nan() { 0.0 } else { v.clamp(-POS_LIMIT, POS_LIMIT) } } fn move_by(s: &mut Session, p: &Value) -> Result { @@ -216,8 +217,8 @@ fn resize(s: &mut Session, p: &Value) -> Result { x.h = clamp_side(x.h); // Rotated shapes resize about their centre; others keep the top-left. if x.rot != 0.0 { - x.x = cx - x.w / 2.0; - x.y = cy - x.h / 2.0; + x.x = clamp_pos(cx - x.w / 2.0); + x.y = clamp_pos(cy - x.h / 2.0); } } Ok(()) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b8cf0ace..7afcb102 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3647,4 +3647,13 @@ fn shape_geometry_commands_limit_size_and_position() { s.execute("shape.move", &json!({"x": 1e100, "y": -1e100})).unwrap(); let x = xfrm(&s); assert!(within(&s), "move stored {x:?}"); + // A move that overflows to infinity lands on the limit, not back at 0. + s.execute("shape.move", &json!({"x": 1e308, "dx": 1e308})).unwrap(); + assert_eq!(xfrm(&s).x, 100_000.0); + // A rotated shape resizes about its centre and still stays inside the limit. + s.execute("shape.setBounds", &json!({"x": 99_000, "y": 0, "w": 10, "h": 10})).unwrap(); + s.execute("shape.rotate", &json!({"deg": 45})).unwrap(); + s.execute("shape.resize", &json!({"w": 1e100})).unwrap(); + let x = xfrm(&s); + assert!(within(&s), "rotated resize stored {x:?}"); }