diff --git a/crates/engine/src/actions_cmds.rs b/crates/engine/src/actions_cmds.rs index 9f3d74e3e0..7f9b52ff85 100644 --- a/crates/engine/src/actions_cmds.rs +++ b/crates/engine/src/actions_cmds.rs @@ -100,6 +100,12 @@ pub struct ActionState { /// Number of steps executed by the current top-level playback tree. #[serde(skip)] pub playback_steps: u64, + /// How many scripts (`file.scripts.browse`) are running inside one another. + #[serde(skip)] + pub script_depth: u8, + /// Steps run by the current top-level script and everything it calls. + #[serde(skip)] + pub script_steps: u64, /// Bumped whenever `list` changes, so the preset store can skip unchanged writes. #[serde(skip)] pub rev: u64, @@ -150,6 +156,14 @@ pub fn save_target_closed(s: &Session, id: &str, target: Option bool { + // Inside a script the step also counts against the script's total: each top-level playback + // resets its own counter, so a script playing actions in a row would never reach the limit. + if s.actions.script_depth > 0 { + if s.actions.script_steps >= crate::automate_cmds::MAX_SCRIPT_STEPS { + return false; + } + s.actions.script_steps += 1; + } if s.actions.playback_steps >= MAX_PLAYBACK_STEPS { false } else { diff --git a/crates/engine/src/adjust_cmds.rs b/crates/engine/src/adjust_cmds.rs index f4e0694fa3..830d601b5f 100644 --- a/crates/engine/src/adjust_cmds.rs +++ b/crates/engine/src/adjust_cmds.rs @@ -355,7 +355,7 @@ pub(crate) fn adjust_as_filter(kind: &str, p: &Value, surf: &Surface) -> Option< } else { let mode = surf.format().mode; let adj = crate::adjust_params::from_params(kind, p, None, mode).ok()?; - crate::pixels::adjust_surface(&mut out, &adj, None, mode); + crate::pixels::adjust_surface(&mut out, &adj, None, mode).ok()?; } out.prune(); Some(out) diff --git a/crates/engine/src/atn_cmds.rs b/crates/engine/src/atn_cmds.rs index 12b4994feb..6f43e5ef70 100644 --- a/crates/engine/src/atn_cmds.rs +++ b/crates/engine/src/atn_cmds.rs @@ -1949,7 +1949,8 @@ fn conditional_from(d: &Descriptor) -> Mapped { return Mapped::Unsupported { harmless: false }; } let branch = |key: &str| match get_ref(d, key).map(target) { - Some(Target::Name(n)) => Some(n), + // Clipped like the stored action names, so the load-time rename map finds it. + Some(Target::Name(n)) => Some(clip_name(&n)), _ => None, }; step("actions.conditional", json!({"if": condition, "then": branch("then"), "else": branch("else")})) @@ -2468,7 +2469,7 @@ fn apply_comp_to(_: &str, p: &Value) -> Option { fn play_from(d: &Descriptor) -> Mapped { match get_ref(d, "null").map(target) { - Some(Target::Name(n)) => step("actions.play", json!({"action": n})), + Some(Target::Name(n)) => step("actions.play", json!({"action": clip_name(&n)})), _ => Mapped::Unsupported { harmless: false }, } } @@ -3877,10 +3878,18 @@ fn normalise(v: &mut Value) { } } +/// Longest action or set name kept (Photoshop's own limit is 255). A file's names are lengths it +/// chooses, and each is copied into every action and every unsupported-step diagnostic. +const MAX_NAME_CHARS: usize = 1024; + +fn clip_name(s: &str) -> String { + s.chars().take(MAX_NAME_CHARS).collect() +} + fn convert_action(a: &AtnAction, set: &str, unsupported: &mut Vec) -> Action { - let name = a.name.to_string_lossy(); + let name = clip_name(&a.name.to_string_lossy()); let mut action = Action::new(name.clone(), Vec::new()); - action.set = Some(set.to_string()); + action.set = Some(clip_name(set)); for (i, item) in a.items.iter().enumerate() { let mapped = match (event_for(&item.event), &item.descriptor) { (Some(e), Some(d)) => (e.from)(d), @@ -3914,7 +3923,7 @@ fn convert_action(a: &AtnAction, set: &str, unsupported: &mut Vec) pub fn import(bytes: &[u8]) -> std::result::Result { let file = atn::parse(bytes).map_err(|e| e.to_string())?; let set = file.name.to_string_lossy(); - let set = if set.trim().is_empty() { DEFAULT_SET.to_string() } else { set }; + let set = if set.trim().is_empty() { DEFAULT_SET.to_string() } else { clip_name(&set) }; let mut out = Imported { set: set.clone(), warnings: file.warnings.clone(), ..Default::default() }; for a in &file.actions { let action = convert_action(a, &set, &mut out.unsupported); diff --git a/crates/engine/src/atn_cmds/tests.rs b/crates/engine/src/atn_cmds/tests.rs index 738892a828..2b967f8c9d 100644 --- a/crates/engine/src/atn_cmds/tests.rs +++ b/crates/engine/src/atn_cmds/tests.rs @@ -315,6 +315,25 @@ fn load_renames_each_play_target_once() { assert_eq!(s.actions.list[1].steps[0].1, json!({"action": "A"}), "B still plays the A it had"); } +/// A name over the 1,024-character limit is clipped for the action and for the steps that call it, +/// so a rename still finds its Play target. +#[test] +fn load_renames_play_targets_longer_than_the_name_limit() { + let mut s = Session::new(); + let long = "x".repeat(2000); + let clipped = "x".repeat(1024); + let mut mine = Action::new(clipped.clone(), vec![]); + mine.set = Some("Mine".into()); + s.actions.list.push(mine); + let a = Action::new(long.clone(), vec![]); + let caller = Action::new("Caller", vec![("actions.play".into(), json!({"action": long}))]); + let (bytes, _) = export("File", &[&a, &caller], &[], false).unwrap(); + s.execute("actions.load", json!({"data": b64(&bytes)})).unwrap(); + let renamed = format!("{clipped} 2"); + let caller = s.actions.list.iter().find(|a| a.name == "Caller").unwrap(); + assert_eq!(caller.steps[0].1["action"], renamed.as_str()); +} + #[test] fn load_many_duplicate_names_get_sequential_suffixes() { // Several actions share one base name. Deduplication must stay O(n) overall and produce @@ -3082,3 +3101,21 @@ fn saves_round_trip_through_photoshop_events() { let odd = Action::new("B", vec![("file.saveAs".to_string(), json!({"path": "/out/result.xcf"}))]); assert!(export("Saves", &[&odd], &[], false).is_err()); } + +#[test] +fn long_names_are_clipped_instead_of_copied_into_every_diagnostic() { + let long = "N".repeat(5000); + let unknown = |n: usize| (0..n).map(|_| item("zzzz", "Mystery", None)).collect::>(); + let file = AtnFile { + name: UnicodeString::new_nul(&long), + expanded: false, + actions: vec![AtnAction { name: UnicodeString::new_nul(&long), items: unknown(50), ..Default::default() }], + warnings: vec![], + }; + let imp = import(&atn::write(&file).unwrap()).unwrap(); + assert_eq!(imp.set.chars().count(), MAX_NAME_CHARS); + assert_eq!(imp.actions[0].name.chars().count(), MAX_NAME_CHARS); + assert_eq!(imp.actions[0].set.as_deref().map(|s| s.chars().count()), Some(MAX_NAME_CHARS)); + assert_eq!(imp.unsupported.len(), 50); + assert!(imp.unsupported.iter().all(|u| u.action.chars().count() == MAX_NAME_CHARS)); +} diff --git a/crates/engine/src/automate_cmds.rs b/crates/engine/src/automate_cmds.rs index 800ff954e0..42fde9e9bf 100644 --- a/crates/engine/src/automate_cmds.rs +++ b/crates/engine/src/automate_cmds.rs @@ -119,12 +119,27 @@ fn check_known(steps: &[(String, Value)], cmd: &str) -> Result<()> { } } +/// Scripts nested in scripts, and steps one top-level script (with everything it calls) may run: +/// a script can name others, so a graph that calls each of the previous ones twice is exponential. +const MAX_SCRIPT_DEPTH: u8 = 16; +pub(crate) const MAX_SCRIPT_STEPS: u64 = crate::actions_cmds::MAX_PLAYBACK_STEPS; + +/// A script run from a script reports its own failure inside an `Ok`; it stops the caller too. +fn nested_script_failure(id: &str, r: &Value) -> Option { + (id == "file.scripts.browse" && r.get("ok") == Some(&Value::Bool(false))).then(|| "a script it runs failed".to_string()) +} + fn run_steps(s: &mut Session, steps: &[(String, Value)]) -> Vec { let mut results = Vec::new(); for (id, params) in steps { + s.actions.script_steps = s.actions.script_steps.saturating_add(1); + if s.actions.script_steps > MAX_SCRIPT_STEPS { + results.push(json!({"command": id, "error": format!("the script ran more than {MAX_SCRIPT_STEPS} steps in all (scripts calling scripts)")})); + break; + } match s.execute(id, params.clone()) { // A called action reports a failed step inside an `Ok`; it stops the script too. - Ok(r) => match crate::actions_cmds::nested_failure(id, &r) { + Ok(r) => match crate::actions_cmds::nested_failure(id, &r).or_else(|| nested_script_failure(id, &r)) { Some(error) => { results.push(json!({"command": id, "result": r, "error": error})); break; @@ -156,7 +171,15 @@ fn browse(s: &mut Session, p: &Value) -> Result { _ => return Err(bad(cmd, "give \"path\" (a script file), \"script\" (its text) or \"steps\"")), }; check_known(&steps, cmd)?; + if s.actions.script_depth >= MAX_SCRIPT_DEPTH { + return Err(bad(cmd, format!("scripts are nested more than {MAX_SCRIPT_DEPTH} deep"))); + } + if s.actions.script_depth == 0 { + s.actions.script_steps = 0; + } + s.actions.script_depth += 1; let results = run_steps(s, &steps); + s.actions.script_depth -= 1; let ok = results.iter().all(|r| r.get("error").is_none()); Ok(json!({"steps": steps.len(), "ok": ok, "results": results})) } diff --git a/crates/engine/src/automate_cmds/tests.rs b/crates/engine/src/automate_cmds/tests.rs index 8bea15c69a..3402496331 100644 --- a/crates/engine/src/automate_cmds/tests.rs +++ b/crates/engine/src/automate_cmds/tests.rs @@ -484,3 +484,46 @@ fn load_files_into_stack_aligns_before_the_smart_object() { s.execute("file.scripts.loadFilesIntoStack", json!({"paths": [files[0].clone()], "align": true})).unwrap(); assert_eq!(s.active().unwrap().doc.layers.len(), 1); } + +/// A script file of `lines` (one `file.scripts.browse` call per entry) in `dir`. +fn script_calling(dir: &str, name: &str, callees: &[String]) -> String { + let path = format!("{dir}/{name}.txt"); + let body: String = callees.iter().map(|c| format!("file.scripts.browse {}\n", json!({"path": c}))).collect(); + std::fs::write(&path, body).unwrap(); + path +} + +#[test] +fn scripts_calling_scripts_share_one_step_budget() { + let dir = tmp("fanout"); + // Eight levels, each calling the one below six times: 6^8 (1.7 million) calls of the leaf. + let mut below = script_calling(&dir, "leaf", &[]); + std::fs::write(&below, "select.deselect {}\n").unwrap(); + for level in 0..8 { + below = script_calling(&dir, &format!("level{level}"), &vec![below; 6]); + } + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + let started = std::time::Instant::now(); + let r = s.execute("file.scripts.browse", json!({"path": below})).unwrap(); + assert_eq!(r["ok"], false, "the budget stops the run: {r}"); + assert!(started.elapsed().as_secs() < 60, "{:?}", started.elapsed()); + // The budget restarts with the next top-level script. + s.execute("file.scripts.browse", json!({"script": "select.deselect {}"})).unwrap(); +} + +#[test] +fn scripts_nested_deeper_than_the_limit_fail() { + let dir = tmp("deep"); + let mut below = script_calling(&dir, "leaf", &[]); + std::fs::write(&below, "select.deselect {}\n").unwrap(); + for level in 0..20 { + below = script_calling(&dir, &format!("level{level}"), &[below]); + } + let mut s = Session::new(); + s.execute("file.new", json!({"width": 8, "height": 8})).unwrap(); + let r = s.execute("file.scripts.browse", json!({"path": below})).unwrap(); + assert_eq!(r["ok"], false, "{r}"); + assert!(r.to_string().contains("nested more than"), "{r}"); + assert_eq!(s.actions.script_depth, 0, "the depth is released"); +} diff --git a/crates/engine/src/color_cmds.rs b/crates/engine/src/color_cmds.rs index 4c8e4df434..9e9c0aafd3 100644 --- a/crates/engine/src/color_cmds.rs +++ b/crates/engine/src/color_cmds.rs @@ -590,7 +590,8 @@ pub fn resolve_profile(spec: &str, doc: Option<&Document>, mode: Option = Builtin::ALL.iter().map(|b| b.id()).collect(); diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 695ee042d5..767a9791b0 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -469,7 +469,7 @@ fn destructive_adjust(s: &mut Session, label: &str, adj: Adjustment, p: &Value) let sel = doc.selection.clone(); let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?; let mask = l.mask.as_mut().ok_or_else(|| EngineError::Other("layer has no mask".into()))?; - pixels::adjust_mask(&mut mask.surface, &adj, sel.as_ref()); + pixels::adjust_mask(&mut mask.surface, &adj, sel.as_ref()).map_err(EngineError::Other)?; Ok(Value::Null) }); } @@ -479,7 +479,7 @@ fn destructive_adjust(s: &mut Session, label: &str, adj: Adjustment, p: &Value) return s.edit(label, |doc, _| { let sel = doc.selection.clone(); if let Some(surf) = crate::channel_cmds::channel_surface_for_filter(doc, None, p)? { - pixels::adjust_mask(surf, &adj, sel.as_ref()); + pixels::adjust_mask(surf, &adj, sel.as_ref()).map_err(EngineError::Other)?; } Ok(Value::Null) }); @@ -489,7 +489,7 @@ fn destructive_adjust(s: &mut Session, label: &str, adj: Adjustment, p: &Value) let sel = doc.selection.clone(); let mode = doc.mode; let surf = paint_surface(doc, id, &Value::Null)?; - pixels::adjust_surface(surf, &adj, sel.as_ref(), mode); + pixels::adjust_surface(surf, &adj, sel.as_ref(), mode).map_err(EngineError::Other)?; Ok(()) })?; Ok(Value::Null) diff --git a/crates/engine/src/edit_cmds.rs b/crates/engine/src/edit_cmds.rs index 1a6c3a4072..5ba3a26110 100644 --- a/crates/engine/src/edit_cmds.rs +++ b/crates/engine/src/edit_cmds.rs @@ -715,7 +715,7 @@ fn auto_adjust(s: &mut Session, kind: &str) -> Result { let sel = doc.selection.clone(); let mode = doc.mode; let surf = crate::commands::paint_surface(doc, id, &Value::Null)?; - crate::pixels::adjust_surface(surf, &adj, sel.as_ref(), mode); + crate::pixels::adjust_surface(surf, &adj, sel.as_ref(), mode).map_err(EngineError::Other)?; Ok(()) })?; Ok(Value::Null) diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index c058792914..6e81656bdb 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -576,6 +576,17 @@ pub(crate) fn flattened(doc: &Document, fmt: PixelFormat) -> Surface { photocraft_compose::flatten_to_surface(doc, fmt, None) } +/// [`flattened`] for documents that came from a file (Save a Copy, Place, Load Files into Stack): +/// a sparse bundle declares its canvas, and one the size of the format allows would be rendered +/// in full, however little it holds. Refused past the compositor's own per-render limit. +pub(crate) fn flattened_checked(doc: &Document, fmt: PixelFormat, cmd: &str) -> Result { + let b = doc.bounds(); + if u64::from(b.width()) * u64::from(b.height()) > photocraft_compose::MAX_RENDER_PIXELS { + return Err(EngineError::BadParams { cmd: cmd.into(), msg: format!("the document's {}×{} canvas is too large to flatten", b.width(), b.height()) }); + } + Ok(flattened(doc, fmt)) +} + // ---------- close / revert / save a copy / open as ---------- fn close_all(s: &mut Session) -> Result { @@ -637,7 +648,7 @@ fn save_a_copy(s: &mut Session, p: &Value) -> Result { if p.get("layers").and_then(Value::as_bool) == Some(false) { // "Layers" unchecked in Save a Copy: write the flattened image. let fmt = doc.pixel_format(); - let px = flattened(&doc, fmt); + let px = flattened_checked(&doc, fmt, "file.saveACopy")?; doc.layers = vec![Layer::new("Background", LayerContent::Raster(px))]; } let warnings = save_doc(&doc, &path, SaveOpts::from_params(s, p, "file.saveACopy")?)?; @@ -744,7 +755,7 @@ pub fn place_bytes(s: &mut Session, name: &str, bytes: Vec, linked: Option (cw / 2.0, ch / 2.0), }; let (dx, dy) = ((center.0 - w * scale / 2.0).round(), (center.1 - h * scale / 2.0).round()); - let mut px = flattened(&src, fmt); + let mut px = flattened_checked(&src, fmt, "file.place")?; if (scale - 1.0).abs() > 1e-9 { px = resize_surface(&px, scale, scale, Resample::Bicubic); } @@ -1455,42 +1466,72 @@ fn batch(s: &mut Session, p: &Value) -> Result { list } }; - let actions = crate::actions_cmds::ActionState { list, playing: s.actions.playing, playback_stack: s.actions.playback_stack.clone(), ..Default::default() }; + // A batch inside a played action charges its inputs × steps (at least one each: an input is read, + // decoded and saved even with no steps) to the playback tree it is in and + // hands the count on, so a batch of batches can't multiply past the limit. A top-level batch + // is not limited by it. + let mut playback_steps = 0; + if s.actions.playing > 0 { + playback_steps = s.actions.playback_steps.saturating_add((steps.len().max(1) as u64).saturating_mul(inputs.len() as u64)); + if playback_steps > crate::actions_cmds::MAX_PLAYBACK_STEPS { + return Err(EngineError::BadParams { cmd: cmd.into(), msg: "this batch would exceed the playback step limit of the action that runs it".into() }); + } + s.actions.playback_steps = playback_steps; + } + let actions = crate::actions_cmds::ActionState { + list, + playing: s.actions.playing, + playback_stack: s.actions.playback_stack.clone(), + playback_steps, + ..Default::default() + }; let names = batch_naming::plan(&inputs, &output, &format, p)?; let save = SaveOpts::from_params(s, p, cmd)?; let authorize = s.authorize; + // Each input runs in its own scratch session, so what the played steps spend is carried from + // one input to the next (failed or not) and back to this session, not restarted per input. + let carried = std::cell::Cell::new(playback_steps); + let carried = &carried; let run = move |scratch: &mut Session, out: &str| { scratch.authorize = authorize; scratch.actions = actions.clone(); - // The document as the last recorded save found it, written once the action has run so a - // failing step still writes nothing. - let mut saved: Option> = None; - for (id, params) in &steps { - if batch_save_point(id) { - saved = Some(scratch.active().ok_or(EngineError::NoDocument)?.doc.clone()); - continue; + scratch.actions.playback_steps = carried.get(); + let result = (|| { + // The document as the last recorded save found it, written once the action has run so a + // failing step still writes nothing. + let mut saved: Option> = None; + for (id, params) in &steps { + if batch_save_point(id) { + saved = Some(scratch.active().ok_or(EngineError::NoDocument)?.doc.clone()); + continue; + } + let r = scratch.execute(id, params.clone())?; + // A failed step inside a called action makes this file an error, not a saved result. + if let Some(error) = crate::actions_cmds::nested_failure(id, &r) { + return Err(EngineError::Other(error)); + } } - let r = scratch.execute(id, params.clone())?; - // A failed step inside a called action makes this file an error, not a saved result. - if let Some(error) = crate::actions_cmds::nested_failure(id, &r) { - return Err(EngineError::Other(error)); + // While the saved document is still open the steps after the save reach the result too + // (#3058), so it is saved after the action as usual. Closed after its save, or with another + // document left active, it is written as the save found it. + match saved { + Some(doc) if scratch.active().is_none_or(|d| d.doc.id != doc.id) => { + save_doc(&doc, out, save)?; + Ok(true) + } + _ => Ok(false), } - } - // While the saved document is still open the steps after the save reach the result too - // (#3058), so it is saved after the action as usual. Closed after its save, or with another - // document left active, it is written as the save found it. - match saved { - Some(doc) if scratch.active().is_none_or(|d| d.doc.id != doc.id) => { - save_doc(&doc, out, save)?; - Ok(true) - } - _ => Ok(false), - } + })(); + carried.set(scratch.actions.playback_steps); + result }; let r = match names { Some(names) => process_outputs(s, inputs.iter().map(|i| i.path.as_str()).zip(names), save, &run), None => process_tree(s, &inputs, &output, &format, save, "", &run), }; + if s.actions.playing > 0 { + s.actions.playback_steps = carried.get(); + } Ok(r) } @@ -1540,7 +1581,7 @@ fn load_files_into_stack(s: &mut Session, p: &Value) -> Result { let fmt = stack.pixel_format(); // First file at the bottom, like Photoshop's script. for (name, d) in &docs { - stack.layers.push(Layer::new(name.clone(), LayerContent::Raster(flattened(d, fmt)))); + stack.layers.push(Layer::new(name.clone(), LayerContent::Raster(flattened_checked(d, fmt, cmd)?))); } let n = stack.layers.len(); let smart = p.get("createSmartObject").and_then(Value::as_bool).unwrap_or(false); diff --git a/crates/engine/src/layer_copy_cmds.rs b/crates/engine/src/layer_copy_cmds.rs index 7995ff6c56..78e72f0a7b 100644 --- a/crates/engine/src/layer_copy_cmds.rs +++ b/crates/engine/src/layer_copy_cmds.rs @@ -222,6 +222,8 @@ fn copy_to_document(s: &mut Session, p: &Value) -> Result { new.push(id); } *active = above; + // Copied groups keep their own depth: repeated copies would otherwise nest without end. + crate::layer_multi_cmds::check_group_depth(doc, "Copy Layers")?; Ok(new) }); // Nothing changed: the source stays the active document. diff --git a/crates/engine/src/layer_copy_cmds/tests.rs b/crates/engine/src/layer_copy_cmds/tests.rs index 93bd7b94ef..b3200b16ef 100644 --- a/crates/engine/src/layer_copy_cmds/tests.rs +++ b/crates/engine/src/layer_copy_cmds/tests.rs @@ -342,3 +342,30 @@ fn a_named_duplicate_takes_the_name_in_one_step() { let r = s.execute("layer.copyToDocument", json!({"document": 1, "name": "Ink"})).unwrap(); assert_eq!(doc(&s, 1).layer(copied(&r)).unwrap().name, "Ink"); } + +#[test] +fn a_copied_group_cannot_nest_the_destination_past_the_group_depth_limit() { + use photocraft_doc::{Layer, MAX_GROUP_DEPTH}; + let (mut s, paint) = two_docs(40, 30, 8); + // A group in the source, and a destination whose active layer sits at the deepest legal level. + s.execute("layer.groupLayers", json!({"layers": [paint.0]})).unwrap(); + let group = s.active().unwrap().active_layer.unwrap(); + s.set_active(1); + let mut inner = None; + s.edit("nest", |doc, active| { + let mut l = Layer::new("deep", LayerContent::Raster(Surface::new(PixelFormat::RGBA8))); + inner = Some(l.id); + for _ in 0..MAX_GROUP_DEPTH { + l = Layer::group("g", vec![l]); + } + doc.layers.push(l); + *active = inner; + Ok(()) + }) + .unwrap(); + s.set_active(0); + let before = doc(&s, 1).clone(); + let e = s.execute("layer.copyToDocument", json!({"document": 1, "layers": [group.0]})).unwrap_err(); + assert!(e.to_string().contains("deeper than"), "{e}"); + assert_eq!(*doc(&s, 1), before, "nothing was copied"); +} diff --git a/crates/engine/src/pixels.rs b/crates/engine/src/pixels.rs index dc4e531f53..01339e859d 100644 --- a/crates/engine/src/pixels.rs +++ b/crates/engine/src/pixels.rs @@ -58,20 +58,55 @@ pub fn composite_layers_uncropped(solo: &Document, format: photocraft_color::Pix photocraft_compose::composite_to_surface(solo, area, format, None) } +/// Most bytes of tiles an adjustment may materialize beyond the ones a surface stores: two distant +/// tiles make the content bounds a grid of millions of tiles that hold nothing. Written tiles are +/// copies, so the peak is about twice this. +const MAX_UNSTORED_BYTES: u64 = 1 << 30; + +/// Tiles in the grid that covers `r` (one extra per axis for a tile straddling its edge). +fn grid_tiles(r: Rect) -> u64 { + (u64::from(r.width()).div_ceil(TILE_SIZE as u64) + 1) * (u64::from(r.height()).div_ceil(TILE_SIZE as u64) + 1) +} + +/// Bytes the tiles of the grid over `r` take beyond the `stored` ones, in format `fmt`. +fn unstored_bytes(r: Rect, stored: u64, fmt: &photocraft_color::PixelFormat) -> u64 { + grid_tiles(r).saturating_sub(stored).saturating_mul((TILE_SIZE as u64).pow(2)).saturating_mul(fmt.bytes_per_pixel() as u64) +} + +/// The tile-sized pieces of `r`, each cut to `r`. +fn tile_rects(r: Rect) -> impl Iterator { + let t = i64::from(TILE_SIZE); + let cols = i64::from(r.x0).div_euclid(t)..=(i64::from(r.x1) - 1).div_euclid(t); + let rows = i64::from(r.y0).div_euclid(t)..=(i64::from(r.y1) - 1).div_euclid(t); + rows.flat_map(move |cy| cols.clone().map(move |cx| Rect::new((cx * t) as i32, (cy * t) as i32, ((cx + 1) * t) as i32, ((cy + 1) * t) as i32).intersect(&r))) +} + +const TOO_SPARSE: &str = "the adjustment would have to fill too large an area around the pixels the layer holds"; + /// Apply an adjustment destructively to a surface, weighted by an optional selection. /// Applies `adj` to a surface (any colour model and depth, via straight RGBA) through the /// selection; `mode` is the document's, for the tone transfer (e.g. Exposure in Grayscale). /// Works tile by tile in parallel, so a large layer needs one tile's buffers per thread rather /// than four full-layer copies, and the result is the same as one pass over the whole region. -pub fn adjust_surface(s: &mut Surface, adj: &Adjustment, selection: Option<&Surface>, mode: photocraft_color::ColorMode) { +pub fn adjust_surface(s: &mut Surface, adj: &Adjustment, selection: Option<&Surface>, mode: photocraft_color::ColorMode) -> Result<(), String> { use rayon::prelude::*; let r = s.content_bounds(); if r.is_empty() { - return; + return Ok(()); } let fmt = s.format(); - let done: Vec<_> = s - .take_tiles(r) + // Two distant tiles make the content bounds a grid of millions of tiles that hold nothing. + // Past a budget a transparent default needs only the stored tiles adjusted (the gaps stay + // transparent); an opaque default would change under the adjustment everywhere in that grid, + // so the adjustment is refused rather than left half applied. + let stored: Vec<_> = s.tiles().map(|(c, _)| *c).filter(|c| !c.rect().intersect(&r).is_empty()).collect(); + let transparent = fmt.alpha && s.default_pixel().last().is_some_and(|a| *a <= 0.0); + let over = unstored_bytes(r, stored.len() as u64, &fmt) > MAX_UNSTORED_BYTES; + if over && !transparent { + return Err(TOO_SPARSE.into()); + } + let taken = if over { stored.into_iter().flat_map(|c| s.take_tiles(c.rect())).collect() } else { s.take_tiles(r) }; + let done: Vec<_> = taken .into_par_iter() .flat_map_iter(|(tc, tile)| { // A one-tile surface, so `adjusted` reads and writes this tile only. @@ -84,6 +119,7 @@ pub fn adjust_surface(s: &mut Surface, adj: &Adjustment, selection: Option<&Surf }) .collect(); s.put_tiles(done); + Ok(()) } #[cfg(test)] @@ -96,26 +132,46 @@ mod tests; /// Applies `adj` to a layer mask through the selection. Untouched mask pixels count (they read as /// the mask's default value), and without a selection the default changes too, since a mask /// reaches past the canvas: Invert turns a reveal-all mask into a hide-all one (#780). -pub fn adjust_mask(s: &mut Surface, adj: &Adjustment, selection: Option<&Surface>) { +pub fn adjust_mask(s: &mut Surface, adj: &Adjustment, selection: Option<&Surface>) -> Result<(), String> { let mode = photocraft_color::ColorMode::Grayscale; if let Some(sel) = selection { let r = sel.content_bounds(); - if !r.is_empty() { - let out = adjusted(s, r, adj, Some(sel), mode); - s.write_region(r, &out); + // Tile by tile, and only where the selection stores something (it is unselected, so + // unchanged, elsewhere): its content bounds can span a grid of mostly nothing. + let sparse = sel.default_pixel().iter().all(|v| *v == 0.0); + let tiles: Vec = if sparse { + sel.tiles().map(|(c, _)| c.rect().intersect(&r)).filter(|t| !t.is_empty()).collect() + } else if r.is_empty() { + Vec::new() + } else { + // Unselected pixels count too, so every tile of the span is adjusted, one at a time + // (not one read of the whole span), and the span is bounded by what that writes. + if unstored_bytes(r, 0, &s.format()) > MAX_UNSTORED_BYTES { + return Err(TOO_SPARSE.into()); + } + tile_rects(r).collect() + }; + for t in &tiles { + let out = adjusted(s, *t, adj, Some(sel), mode); + s.write_region(*t, &out); + } + if !tiles.is_empty() { s.prune(); } - return; + return Ok(()); } let fmt = s.format(); let unit = Rect::from_xywh(0, 0, 1, 1); let mut out = Surface::with_default(fmt, &adjusted(&Surface::with_default(fmt, &s.default_pixel()), unit, adj, None, mode)); - let r = s.tile_bounds(); - if !r.is_empty() { + // Everything the mask doesn't store reads as its default, which `out` already holds adjusted. + // So only the stored tiles are adjusted, not the (possibly huge) rectangle around them. + let stored: Vec = s.tiles().map(|(c, _)| c.rect()).collect(); + for r in stored { out.write_region(r, &adjusted(s, r, adj, None, mode)); } out.prune(); *s = out; + Ok(()) } /// `s`'s pixels over `r` (untouched ones included) with `adj` applied through the selection, encoded. diff --git a/crates/engine/src/pixels_tests.rs b/crates/engine/src/pixels_tests.rs index 3b1c25c21e..10596fbca9 100644 --- a/crates/engine/src/pixels_tests.rs +++ b/crates/engine/src/pixels_tests.rs @@ -110,7 +110,7 @@ fn tiled_adjustment_matches_one_pass_over_the_content() { for sel in [None, Some(&soft)] { let want = one_pass(&s, &adj, sel, fmt.mode); let mut got = s.clone(); - adjust_surface(&mut got, &adj, sel, fmt.mode); + adjust_surface(&mut got, &adj, sel, fmt.mode).unwrap(); assert!(want != s, "{kind} {fmt:?}: the adjustment changes the layer"); assert!(got == want, "{kind} {fmt:?} selection {}: tiled result differs", sel.is_some()); } @@ -121,7 +121,7 @@ fn tiled_adjustment_matches_one_pass_over_the_content() { #[test] fn an_empty_layer_is_left_alone() { let mut s = Surface::new(PixelFormat::RGBA8); - adjust_surface(&mut s, &Adjustment::Invert, None, ColorMode::Rgb); + adjust_surface(&mut s, &Adjustment::Invert, None, ColorMode::Rgb).unwrap(); assert_eq!(s.tile_count(), 0); } @@ -263,7 +263,7 @@ fn invert_in_a_lab_document_mirrors_l_a_and_b() { let mut s = Surface::new(fmt); let r = Rect::new(0, 0, 2, 2); s.write_region(r, &(0..4).flat_map(|_| from_rgba(&fmt, [c[0], c[1], c[2], 1.0])).collect::>()); - adjust_surface(&mut s, &Adjustment::Invert, None, ColorMode::Lab); + adjust_surface(&mut s, &Adjustment::Invert, None, ColorMode::Lab).unwrap(); let px = s.read_region(Rect::new(1, 1, 2, 2)); let rgba = photocraft_raster::to_rgba(&fmt, &px); let got = srgb_to_lab([rgba[0], rgba[1], rgba[2]]); @@ -318,7 +318,7 @@ fn adjusted_pixel(fmt: PixelFormat, ch: &[f32], adj: &Adjustment, k: Option if let Some(k) = k { sel.write_region(unit, &[k]); } - adjust_surface(&mut s, adj, k.map(|_| &sel), fmt.mode); + adjust_surface(&mut s, adj, k.map(|_| &sel), fmt.mode).unwrap(); s.read_region(unit) } @@ -404,3 +404,52 @@ fn hue_saturation_and_color_balance_keep_a_cmyk_layers_black() { assert!((0..4).all(|c| (half[c] - (ink[c] + (want[0][c] - ink[c]) * k)).abs() < 1e-3), "{}: half a selection", adj.label()); } } + +/// Two small blocks two billion pixels apart: the content bounds are a grid of 6·10¹³ tiles +/// that holds nothing, which no adjustment may materialize. +fn distant_blocks(fmt: PixelFormat, default: f32) -> Surface { + let mut s = Surface::with_default(fmt, &vec![default; fmt.channels()]); + s.fill_rect(Rect::new(0, 0, 10, 10), &vec![0.25; fmt.channels()]); + s.fill_rect(Rect::new(2_000_000_000, 2_000_000_000, 2_000_000_010, 2_000_000_010), &vec![0.25; fmt.channels()]); + s +} + +#[test] +fn adjusting_a_layer_of_distant_blocks_visits_only_its_tiles() { + let mut s = distant_blocks(PixelFormat::RGBA8, 0.0); + adjust_surface(&mut s, &Adjustment::Invert, None, ColorMode::Rgb).unwrap(); + let p = s.read_region(Rect::new(2_000_000_005, 2_000_000_005, 2_000_000_006, 2_000_000_006)); + // Straight colour inverted, alpha untouched. + assert!((p[0] - 0.75).abs() < 0.01 && (p[3] - 0.25).abs() < 0.01, "{p:?}"); + assert_eq!(s.tile_count(), 2); +} + +#[test] +fn adjusting_a_mask_of_distant_blocks_visits_only_its_tiles() { + let mut m = distant_blocks(PixelFormat::GRAY8, 1.0); + adjust_mask(&mut m, &Adjustment::Invert, None).unwrap(); + assert!(m.default_pixel()[0] < 0.01, "the default is inverted: {:?}", m.default_pixel()); + let p = m.read_region(Rect::new(2_000_000_005, 2_000_000_005, 2_000_000_006, 2_000_000_006)); + assert!((p[0] - 0.75).abs() < 0.01, "{p:?}"); + // Through a selection of its own distant tiles, too. + let mut sel = Surface::new(PixelFormat::GRAY8); + sel.fill_rect(Rect::new(2_000_000_000, 2_000_000_000, 2_000_000_010, 2_000_000_010), &[1.0]); + adjust_mask(&mut m, &Adjustment::Invert, Some(&sel)).unwrap(); + let p = m.read_region(Rect::new(2_000_000_005, 2_000_000_005, 2_000_000_006, 2_000_000_006)); + assert!((p[0] - 0.25).abs() < 0.01, "{p:?}"); +} + +#[test] +fn an_opaque_layer_of_distant_blocks_is_refused_not_half_adjusted() { + let mut s = distant_blocks(PixelFormat::RGBA8, 1.0); + let before = s.clone(); + assert!(adjust_surface(&mut s, &Adjustment::Invert, None, ColorMode::Rgb).is_err()); + assert!(s == before, "a refused adjustment leaves the layer as it was"); +} + +#[test] +fn a_selection_with_a_nonzero_default_over_distant_blocks_is_refused() { + let mut m = Surface::with_default(PixelFormat::GRAY8, &[0.5]); + let sel = distant_blocks(PixelFormat::GRAY8, 1.0); + assert!(adjust_mask(&mut m, &Adjustment::Invert, Some(&sel)).is_err()); +} diff --git a/crates/engine/src/prefs.rs b/crates/engine/src/prefs.rs index d0d76d0530..6b7639b3ae 100644 --- a/crates/engine/src/prefs.rs +++ b/crates/engine/src/prefs.rs @@ -1929,7 +1929,11 @@ impl Session { /// Batches defer colour validation until their complete candidate is staged. Returns /// whether colour settings were touched, including whole-section/root-object updates. fn set_pref(&mut self, path: &str, value: Value, validate_color: bool) -> std::result::Result { - if let Some(rest) = path.strip_prefix("colorSettings") { + // The section itself or a path inside it: `colorSettingsworkingRgb`, `colorSettings.` and + // `colorSettings..workingRgb` are none, and the authorization policy doesn't treat them as one. + if let Some(rest) = + path.strip_prefix("colorSettings").filter(|r| r.is_empty() || r.strip_prefix('.').is_some_and(|c| c.split('.').all(|part| !part.is_empty()))) + { let mut cur = serde_json::to_value(&self.color.settings).map_err(|e| e.to_string())?; let rest = rest.trim_start_matches('.'); if rest.is_empty() { @@ -2735,3 +2739,19 @@ pub fn specs() -> Vec { #[cfg(test)] #[path = "prefs/tests.rs"] mod tests; + +#[cfg(test)] +mod color_settings_path_tests { + use crate::Session; + use serde_json::json; + + #[test] + fn a_path_that_only_starts_with_color_settings_is_not_in_that_section() { + let mut s = Session::new(); + let before = s.color.settings.clone(); + for path in ["colorSettingsworkingRgb", "colorSettings.", "colorSettings..workingRgb"] { + assert!(s.execute("prefs.set", json!({"path": path, "value": "/dev/zero"})).is_err(), "{path}"); + } + assert_eq!(s.color.settings, before); + } +} diff --git a/crates/engine/src/preset_import_cmds.rs b/crates/engine/src/preset_import_cmds.rs index 14273c2bd9..417f5623a1 100644 --- a/crates/engine/src/preset_import_cmds.rs +++ b/crates/engine/src/preset_import_cmds.rs @@ -146,11 +146,20 @@ fn add_abr_presets(s: &mut Session, p: &Value, group: String, imp: photocraft_io Ok(json!({ "group": group, "imported": names, "count": names.len(), "folders": folders, "version": imp.version, "warnings": imp.warnings })) } +/// Largest canvas (268 MP) a brush may be made from. +const MAX_BRUSH_SOURCE_PIXELS: u64 = 1 << 28; + /// The image's paint amount (darkness × alpha of its composite, as Define Brush Preset reads a /// selection), cropped to the painted extent. The composite is rendered in bands twice: once to /// find the painted bounds, then only those, so a large mostly-white image never needs a /// full-size buffer. fn image_paint(doc: &photocraft_doc::Document, cmd: &str) -> Result<(Vec, usize, usize)> { + // The whole canvas is rendered (twice) to find the painted part: a sparse native file declares + // its canvas, so refuse one far past any brush source before rendering it. + let canvas = doc.bounds(); + if u64::from(canvas.width()) * u64::from(canvas.height()) > MAX_BRUSH_SOURCE_PIXELS { + return Err(bad(cmd, format!("the image's {}×{} canvas is too large to make a brush from", canvas.width(), canvas.height()))); + } let paint = |c: [f32; 4]| ((1.0 - photocraft_color::blend::lum([c[0], c[1], c[2]])) * c[3]).clamp(0.0, 1.0); let mut bounds: Option = None; let _ = photocraft_compose::render_bands(doc, doc.bounds(), 0, |band| -> std::result::Result<(), ()> { diff --git a/crates/engine/src/video_cmds.rs b/crates/engine/src/video_cmds.rs index 7713e0d36f..9917522d06 100644 --- a/crates/engine/src/video_cmds.rs +++ b/crates/engine/src/video_cmds.rs @@ -119,6 +119,10 @@ fn new_blank(s: &mut Session, _p: &Value) -> Result { } let fps = doc.timeline.as_ref().map_or(30.0, |t| t.fps); let n = doc.timeline.as_ref().map_or(1, |t| t.duration).max(1); + // A loaded timeline's duration bypasses the validation of timeline.create / setProps. + if n > crate::timeline_cmds::MAX_DURATION { + return Err(EngineError::Other(format!("the timeline is {n} frames; a video layer is limited to {} frames", crate::timeline_cmds::MAX_DURATION))); + } let frames: Vec = (0..n).map(|_| Surface::new(fmt)).collect(); let name = doc.next_layer_name("Video Layer"); let mut l = Layer::raster(name, fmt); @@ -256,6 +260,9 @@ type ImportFrames = fn( &mut dyn FnMut(photocraft_doc::Document, u32) -> std::result::Result<(), photocraft_io::IoError>, ) -> std::result::Result; +/// Frame storage one footage import may keep (whole tiles, all frames together). +const MAX_FOOTAGE_BYTES: u64 = 2 << 30; + /// Load frames from `path`: a single image (1 frame, or every frame of an animated GIF or WebP), or a /// directory of images (sorted = an image sequence). Each becomes a full raster frame in the /// document's format. @@ -281,8 +288,23 @@ fn load_frames(path: &str, fmt: photocraft_color::PixelFormat) -> Result = Vec::with_capacity(files.len()); let mut delays = Vec::new(); + // Each frame stores whole tiles however little it draws, so count the tiles kept so far. + let mut kept_bytes = 0u64; + let mut keep = |frames: &mut Vec, surface: Surface| -> std::result::Result<(), String> { + // A frame of fully transparent pixels stores no tiles, so the count needs its own limit. + if frames.len() >= crate::timeline_cmds::MAX_DURATION { + return Err(format!("the footage has more than {} frames", crate::timeline_cmds::MAX_DURATION)); + } + let tile = u64::from(photocraft_geom::TILE_SIZE.unsigned_abs()).pow(2) * surface.format().bytes_per_pixel() as u64; + kept_bytes = kept_bytes.saturating_add(tile.saturating_mul(surface.tile_count() as u64)); + if kept_bytes > MAX_FOOTAGE_BYTES { + return Err(format!("the footage needs more than {} MiB of frames", MAX_FOOTAGE_BYTES >> 20)); + } + frames.push(surface); + Ok(()) + }; for f in &files { let bytes = std::fs::read(f).map_err(|e| EngineError::Other(format!("read `{}`: {e}", f.display())))?; let name = f.to_string_lossy(); @@ -293,12 +315,19 @@ fn load_frames(path: &str, fmt: photocraft_color::PixelFormat) -> Result None, }; if let Some(import) = animation { - import(&name, &bytes, &mut |doc, delay_ms| { - frames.push(crate::file_cmds::flattened(&doc, fmt)); + let mut over = None; + let done = import(&name, &bytes, &mut |doc, delay_ms| { + if let Err(msg) = keep(&mut frames, crate::file_cmds::flattened(&doc, fmt)) { + over = Some(msg); + return Err(photocraft_io::IoError::Cancelled); + } delays.push(delay_ms); Ok(()) - }) - .map_err(|e| EngineError::Other(format!("`{}`: {e}", f.display())))?; + }); + if let Some(msg) = over { + return Err(EngineError::Other(format!("`{}`: {msg}", f.display()))); + } + done.map_err(|e| EngineError::Other(format!("`{}`: {e}", f.display())))?; continue; } let mut doc = crate::file_cmds::import(&name, &bytes)?; @@ -306,7 +335,8 @@ fn load_frames(path: &str, fmt: photocraft_color::PixelFormat) -> Result Result { Ok(json!({"layers": n})) } +/// The document's name without its extension, as a file name: a native document keeps whatever +/// name its file declares, parent-directory components included, and the frames are written +/// beside the chosen folder, not wherever the name leads. +fn safe_stem(name: &str) -> String { + let leaf = name.rsplit(['/', '\\']).next().unwrap_or(name); + let stem = leaf.rsplit_once('.').map_or(leaf, |(a, _)| a).trim(); + if stem.is_empty() || stem.chars().all(|c| c == '.') { "video".to_string() } else { stem.to_string() } +} + /// Export › Render Video: composite the whole document at each timeline frame and write an image /// sequence (`{dir}/{stem}_NNNN.{format}`). Clean-room; no proprietary codec. fn render_video(s: &mut Session, p: &Value) -> Result { @@ -450,7 +489,7 @@ pub fn render_video_with( // Only the GIF delay reads the rate (frames come from the duration); rates under 1 fps are // valid timelines, so only an unusable loaded rate falls back to 30. let fps = base.timeline.as_ref().map(|t| t.fps).filter(|f| f.is_finite() && *f > 0.0).unwrap_or(30.0); - let stem = base.name.rsplit_once('.').map_or(base.name.as_str(), |(a, _)| a).to_string(); + let stem = safe_stem(&base.name); let fmt = base.pixel_format(); let bounds = base.bounds(); let (w, h) = (bounds.width() as usize, bounds.height() as usize); @@ -495,7 +534,8 @@ pub fn render_video_with( } // Image sequence. - let opts = photocraft_io::ExportOptions::default(); + // PSD / PSB frames would embed the files linked smart objects name: not for a restricted session. + let opts = photocraft_io::ExportOptions { read_linked_files: s.authorize.is_none(), ..Default::default() }; let mut files = Vec::new(); for f in 0..frames { let mut doc = base.clone(); diff --git a/crates/engine/src/video_cmds/tests.rs b/crates/engine/src/video_cmds/tests.rs index a3f304ce8e..3560c5f2c3 100644 --- a/crates/engine/src/video_cmds/tests.rs +++ b/crates/engine/src/video_cmds/tests.rs @@ -669,3 +669,14 @@ fn fps_from_delays_picks_most_common_and_breaks_ties_by_first_seen() { many.push(20); assert_eq!(fps_from_delays(&many), Some(50.0)); } + +#[test] +fn render_video_names_frames_after_the_leaf_of_the_document_name() { + assert_eq!(safe_stem("clip.mov"), "clip"); + assert_eq!(safe_stem("../../outside/clip.png"), "clip"); + assert_eq!(safe_stem("C:\\Users\\x\\clip.v2.gif"), "clip.v2"); + for hostile in ["..", "../..", "", "/", "a/..", ".hidden."] { + let stem = safe_stem(hostile); + assert!(!stem.contains(['/', '\\']) && !stem.is_empty() && stem.chars().any(|c| c != '.'), "{hostile:?} -> {stem:?}"); + } +}