diff --git a/crates/engine/src/artboard_cmds.rs b/crates/engine/src/artboard_cmds.rs index eb1fe2c3d6..16368d81cd 100644 --- a/crates/engine/src/artboard_cmds.rs +++ b/crates/engine/src/artboard_cmds.rs @@ -13,7 +13,7 @@ use photocraft_geom::Rect; use photocraft_raster::Surface; use serde_json::{Value, json}; -use crate::commands::{CommandSpec, color_param, int}; +use crate::commands::{CommandSpec, color_param, int, int_i32}; use crate::file_cmds::{SaveOpts, encode, f64_param, join, native_doc, sanitize, save_doc, stem, str_param, write_file}; use crate::{EngineError, Result, Session}; @@ -109,19 +109,22 @@ fn background_param(p: &Value, cmd: &str) -> Result> /// `rect` as `[x, y, w, h]` or `x`/`y`/`width`/`height` keys over `base`. fn rect_param(p: &Value, base: Rect, cmd: &str) -> Result { if let Some(Value::Array(a)) = p.get("rect") { - let v: Vec = a.iter().filter_map(|x| x.as_f64().map(|f| f.round() as i32)).collect(); + let v: Vec = a.iter().filter_map(|x| x.as_f64().filter(|f| f.is_finite()).map(|f| f.round() as i32)).collect(); if v.len() != 4 || v[2] <= 0 || v[3] <= 0 { return Err(bad(cmd, "\"rect\" must be [x, y, width, height] with a positive size")); } return Ok(Rect::from_xywh(v[0], v[1], v[2] as u32, v[3] as u32)); } - let x = int(p, "x").map_or(base.x0, |v| v as i32); - let y = int(p, "y").map_or(base.y0, |v| v as i32); + let x = int_i32(cmd, p, "x")?.unwrap_or(base.x0); + let y = int_i32(cmd, p, "y")?.unwrap_or(base.y0); let w = int(p, "width").unwrap_or(i64::from(base.width())); let h = int(p, "height").unwrap_or(i64::from(base.height())); if w <= 0 || h <= 0 { return Err(bad(cmd, "the artboard size must be positive")); } + if w > i32::MAX as i64 || h > i32::MAX as i64 { + return Err(bad(cmd, "the artboard size is too large for a 32-bit canvas")); + } Ok(Rect::from_xywh(x, y, w as u32, h as u32)) } diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index ce789f11cc..2b83775108 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -121,6 +121,17 @@ pub(crate) fn int(p: &Value, key: &str) -> Option { p.get(key).and_then(|v| v.as_i64().or_else(|| v.as_f64().filter(|f| f.is_finite()).map(|f| f.round() as i64))) } +/// [`int`] narrowed to `i32` for geometry: `None` when absent, and a bad-params error when the +/// value would wrap through `as i32` — out-of-range integers truncate by multiples of 2^32, +/// which silently relocates geometry (`dx = 2^32 + 50` moves 50 px, `3e9` goes negative) +/// instead of erroring. +pub(crate) fn int_i32(cmd: &str, p: &Value, key: &str) -> Result> { + match int(p, key) { + None => Ok(None), + Some(v) => i32::try_from(v).map(Some).map_err(|_| bad(cmd, format!("`{key}` = {v} is outside the 32-bit coordinate range"))), + } +} + fn f32_or(p: &Value, key: &str, default: f32) -> f32 { p.get(key).and_then(Value::as_f64).map(|v| v as f32).unwrap_or(default) } @@ -324,8 +335,8 @@ fn build() -> Vec { r##"{"x":i32,"y":i32,"width":u32,"height":u32,"mode":"replace|add|subtract|intersect"="replace","ellipse":bool=false,"antiAlias":bool=true,"feather":px=0}"##, has_doc, |s, p| { - let get = |k: &str| int(p, k).ok_or_else(|| bad("select.rect", format!("missing `{k}`"))); - let r = Rect::from_xywh(get("x")? as i32, get("y")? as i32, get("width")?.max(0) as u32, get("height")?.max(0) as u32); + let get = |k: &str| int_i32("select.rect", p, k).and_then(|v| v.ok_or_else(|| bad("select.rect", format!("missing `{k}`")))); + let r = Rect::from_xywh(get("x")?, get("y")?, get("width")?.max(0) as u32, get("height")?.max(0) as u32); let mode = p.get("mode").and_then(Value::as_str).unwrap_or("replace").to_string(); let ellipse = p.get("ellipse").and_then(Value::as_bool).unwrap_or(false); // Options bar: anti-aliased ellipse edges (4x4 supersampled) and Feather (applied to the new shape only). diff --git a/crates/engine/src/edit_menu_cmds.rs b/crates/engine/src/edit_menu_cmds.rs index 5c0590a1a7..8feaed9415 100644 --- a/crates/engine/src/edit_menu_cmds.rs +++ b/crates/engine/src/edit_menu_cmds.rs @@ -303,10 +303,20 @@ fn channel_mask<'a>(doc: &'a Document, v: &Value) -> Option<&'a Surface> { } } -fn rect_param(p: &Value, key: &str) -> Option { - let a = p.get(key)?.as_array()?; - let n = |i: usize| a.get(i).and_then(Value::as_f64).map(|v| v.round() as i32); - Some(Rect::new(n(0)?, n(1)?, n(0)? + n(2)?.max(0), n(1)? + n(3)?.max(0))) +/// `key` as an `[x, y, w, h]` rectangle. The saturating float→int casts bound each component; +/// the additions saturate too — `area: [1e30, 0, 1e30, 10]` is a whole-canvas window, not an +/// overflow (it panicked in debug builds before the `saturating_add`). +fn rect_param(p: &Value, key: &str, cmd: &str) -> Result { + let a = p.get(key).and_then(Value::as_array).ok_or_else(|| bad(cmd, format!("`{key}` must be [x, y, w, h]")))?; + let n = |i: usize| { + a.get(i) + .and_then(Value::as_f64) + .filter(|f| f.is_finite()) + .map(|v| v.round() as i32) + .ok_or_else(|| bad(cmd, format!("`{key}` must be four finite numbers"))) + }; + let (x, y, w, h) = (n(0)?, n(1)?, n(2)?, n(3)?); + Ok(Rect::new(x, y, x.saturating_add(w.max(0)), y.saturating_add(h.max(0)))) } fn content_aware_fill(s: &mut Session, p: &Value) -> Result { @@ -323,7 +333,7 @@ fn content_aware_fill(s: &mut Session, p: &Value) -> Result { let ext = hb.width().max(hb.height()) as i32; let sampling = str_or(p, "sampling", "auto").to_string(); let custom_mask: Option = p.get("channel").and_then(|v| channel_mask(&doc, v)).cloned(); - let custom_rect = rect_param(p, "area"); + let custom_rect = if p.get("area").is_some() { Some(rect_param(p, "area", cmd)?) } else { None }; let window = match sampling.as_str() { "auto" => hb.inflate((ext * 3 / 4).max(32)), "rectangular" => hb.inflate(int(p, "margin").map_or(ext.max(16), |m| m.clamp(0, 100_000) as i32)), diff --git a/crates/engine/src/edit_menu_cmds/tests.rs b/crates/engine/src/edit_menu_cmds/tests.rs index 20738a5981..a3444e834b 100644 --- a/crates/engine/src/edit_menu_cmds/tests.rs +++ b/crates/engine/src/edit_menu_cmds/tests.rs @@ -101,6 +101,21 @@ fn blob_session(depth: u32) -> Session { s } +#[test] +fn content_aware_fill_huge_area_neither_panics_nor_wraps() { + let mut s = blob_session(8); + // `[1e30, 0, 1e30, 10]` overflowed the i32 additions in the area parser (a debug-build + // panic, a garbage sampling window in release); the additions saturate now, so this is + // simply a whole-canvas window. + let r = s.execute("edit.contentAwareFill", json!({"sampling": "custom", "area": [1e30, 0.0, 1e30, 10.0], "colorAdaptation": "none"})).unwrap(); + assert!(r["filled"].as_u64().unwrap() > 100); + // A malformed area names the problem instead of silently falling back. + for area in [json!([0.0, 0.0, null, 10.0]), json!([0.0, 0.0, 10.0]), json!("nope")] { + let err = s.execute("edit.contentAwareFill", json!({"sampling": "custom", "area": area})).unwrap_err(); + assert!(err.to_string().contains("`area`"), "{err}"); + } +} + #[test] fn content_aware_fill_removes_object_at_all_depths() { for depth in [8, 16, 32] { diff --git a/crates/engine/src/filters.rs b/crates/engine/src/filters.rs index 0f89ef85dd..1181dd7823 100644 --- a/crates/engine/src/filters.rs +++ b/crates/engine/src/filters.rs @@ -15,7 +15,9 @@ fn f(p: &Value, k: &str, d: f32) -> f32 { p.get(k).and_then(Value::as_f64).map_or(d, |v| v as f32) } fn i(p: &Value, k: &str, d: i32) -> i32 { - p.get(k).and_then(|v| v.as_i64().or_else(|| v.as_f64().map(|f| f.round() as i64))).map_or(d, |v| v as i32) + // Clamped like the neighbouring filter params: an out-of-range value saturates instead of + // wrapping through `as i32` (`3e9` used to come back as a negative offset). + p.get(k).and_then(|v| v.as_i64().or_else(|| v.as_f64().map(|f| f.round() as i64))).map_or(d, |v| v.clamp(i64::from(i32::MIN), i64::from(i32::MAX)) as i32) } fn b(p: &Value, k: &str, d: bool) -> bool { p.get(k).and_then(Value::as_bool).unwrap_or(d) @@ -498,6 +500,17 @@ mod tests { } } + #[test] + fn huge_offsets_clamp_instead_of_wrapping() { + // 3e9 wrapped to a negative offset through `as i32`; it saturates now, so it + // equals the maximal in-range offset instead of shifting the other way. + let mut a = session(); + a.execute("filter.other.offset", json!({"horizontal": 3_000_000_000_i64, "vertical": 0})).unwrap(); + let mut b = session(); + b.execute("filter.other.offset", json!({"horizontal": 2_147_483_647_i64, "vertical": 0})).unwrap(); + assert_eq!(active_pixels(&a), active_pixels(&b)); + } + #[test] fn last_filter_repeats_with_same_params() { let mut s = session(); diff --git a/crates/engine/src/image_cmds.rs b/crates/engine/src/image_cmds.rs index 841de8a632..e22afc95e3 100644 --- a/crates/engine/src/image_cmds.rs +++ b/crates/engine/src/image_cmds.rs @@ -250,8 +250,15 @@ fn canvas_size(s: &mut Session, p: &Value) -> Result { /// Image → Crop (to the selection bounds). fn crop(s: &mut Session, p: &Value) -> Result { let delete = p.get("deleteCroppedPixels").and_then(Value::as_bool).unwrap_or(true); - let explicit = match (crate::commands::int(p, "x"), crate::commands::int(p, "y"), crate::commands::int(p, "width"), crate::commands::int(p, "height")) { - (Some(x), Some(y), Some(w), Some(h)) if w > 0 && h > 0 => Some(Rect::new(x as i32, y as i32, (x + w) as i32, (y + h) as i32)), + // Values beyond i32 would wrap through the narrowing casts into a rectangle unrelated to + // the numbers passed; reject them (see `commands::int_i32`). + let explicit = match ( + crate::commands::int_i32("image.crop", p, "x")?, + crate::commands::int_i32("image.crop", p, "y")?, + crate::commands::int_i32("image.crop", p, "width")?, + crate::commands::int_i32("image.crop", p, "height")?, + ) { + (Some(x), Some(y), Some(w), Some(h)) if w > 0 && h > 0 => Some(Rect::new(x, y, x.saturating_add(w), y.saturating_add(h))), _ => None, }; // An explicit rectangle (the Crop tool) may extend past the canvas; a selection crop is clamped. @@ -434,6 +441,17 @@ pub fn specs() -> Vec { mod tests { use super::*; + #[test] + fn crop_rejects_rectangles_that_would_wrap() { + let mut s = session(); + // 2^32 + 100 wrapped to `x = 100` through the narrowing casts: a crop rectangle + // somewhere unrelated to the numbers passed. + let err = s.execute("image.crop", json!({"x": 4_294_967_396_i64, "y": 0, "width": 10, "height": 10})).unwrap_err(); + assert!(err.to_string().contains("32-bit"), "{err}"); + // In-range rectangles past the canvas still work (clamped by the crop itself). + s.execute("image.crop", json!({"x": -10, "y": -10, "width": 1000, "height": 1000})).unwrap(); + } + fn session() -> Session { let mut s = Session::new(); s.execute("file.new", json!({"width": 40, "height": 20})).unwrap(); diff --git a/crates/engine/src/layer_multi_cmds.rs b/crates/engine/src/layer_multi_cmds.rs index b24637f655..0d8a0c7480 100644 --- a/crates/engine/src/layer_multi_cmds.rs +++ b/crates/engine/src/layer_multi_cmds.rs @@ -10,7 +10,7 @@ use photocraft_doc::{Document, Layer, LayerContent, LayerId}; use photocraft_geom::Rect; use serde_json::{Value, json}; -use crate::commands::{CommandSpec, int}; +use crate::commands::{CommandSpec, int_i32}; use crate::{DocState, EngineError, Result, Session}; // ---------- selection state ---------- @@ -237,8 +237,8 @@ pub(crate) fn move_layers(doc: &mut Document, moves: &[(LayerId, i32, i32)]) -> /// `layer.translate`: the explicit layer, or every selected layer, plus their linked layers. pub fn translate(s: &mut Session, p: &Value) -> Result { - let dx = int(p, "dx").unwrap_or(0) as i32; - let dy = int(p, "dy").unwrap_or(0) as i32; + let dx = int_i32("layer.translate", p, "dx")?.unwrap_or(0); + let dy = int_i32("layer.translate", p, "dy")?.unwrap_or(0); if dx == 0 && dy == 0 { return Ok(Value::Null); } @@ -1204,6 +1204,21 @@ mod tests { assert_eq!(sel(&s), before); } + #[test] + fn translate_rejects_offsets_that_would_wrap() { + let mut s = session(8); + let a = rect_layer(&mut s, Rect::new(0, 0, 5, 5)); + select_all(&mut s, &[a]); + // 2^32 + 50 wrapped to `dx = 50` and 3e9 to a negative offset through `as i32`. + for dx in [4_294_967_346_i64, 3_000_000_000_i64] { + let err = s.execute("layer.translate", json!({"dx": dx, "dy": 0})).unwrap_err(); + assert!(err.to_string().contains("32-bit"), "{err}"); + } + assert_eq!(bounds(&s, a), Rect::new(0, 0, 5, 5), "the layer never moved"); + // Large in-range offsets still work. + s.execute("layer.translate", json!({"dx": -200_000, "dy": 200_000})).unwrap(); + } + #[test] fn linked_layers_move_together_and_undo() { for depth in [8, 16, 32] { diff --git a/crates/engine/src/selection_cmds.rs b/crates/engine/src/selection_cmds.rs index e3d2d5b7d7..e21ea7da16 100644 --- a/crates/engine/src/selection_cmds.rs +++ b/crates/engine/src/selection_cmds.rs @@ -531,6 +531,22 @@ mod tests { assert!(s.active().unwrap().doc.selection.is_some()); } + #[test] + fn geometry_params_that_would_wrap_are_rejected() { + let mut s = session(); + // 2^32 + 50 wrapped to `x = 50` and 3e9 to a negative coordinate through `as i32`; + // both are bad-params errors now, whatever the front door (UI, CLI, control, MCP). + for x in [4_294_967_346_i64, 3_000_000_000_i64] { + let err = s.execute("select.rect", json!({"x": x, "y": 0, "width": 10, "height": 10})).unwrap_err(); + assert!(err.to_string().contains("32-bit"), "{err}"); + } + let err = s.execute("document.pixel", json!({"x": 4_294_967_346_i64, "y": 0})).unwrap_err(); + assert!(err.to_string().contains("32 bits"), "{err}"); + // In-range coordinates, including negative and past-canvas ones, are unaffected. + s.execute("select.rect", json!({"x": -100, "y": -100, "width": 500, "height": 500})).unwrap(); + s.execute("document.pixel", json!({"x": 0, "y": 0})).unwrap(); + } + #[test] fn grow_and_similar() { let mut s = session();