From 428a861356517a6e035cadcacafba9ba8264f55a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 14:40:15 -0500 Subject: [PATCH] Project: layer name suffixes in one pass; template names bounded and never a parent folder (#2973) * Project: layer name suffixes in one pass; template names bounded and never a parent folder unique_layer_name tried each numeric suffix against every layer, so importing many layers of one name was cubic. The suffixes in use are now gathered once. sanitize kept '.' and '..' (a comp name used as a path component in an output template could point at the parent folder) and copied a name of any length into every template token; dot-only names become underscores and names are cut to 255 characters. * address review: limit sanitized names to 255 bytes * address review: keep long sanitized names distinct with a hash suffix * address review: unique layer names use the numbered_name stem rule --- crates/project/src/comp.rs | 15 ++++++++++-- crates/project/src/item.rs | 12 ++++++--- crates/project/src/render_queue.rs | 39 +++++++++++++++++++++++++++++- crates/project/src/tests.rs | 12 +++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/crates/project/src/comp.rs b/crates/project/src/comp.rs index d8d7cda16..c0c401c82 100644 --- a/crates/project/src/comp.rs +++ b/crates/project/src/comp.rs @@ -268,8 +268,19 @@ impl Comp { /// `base`, or the next free numbered name when a layer already has it (`Shape Layer 2`, /// `v-b1.mp4 2`; see `numbered_name`). pub fn unique_layer_name(&self, base: &str) -> String { - let taken = |n: &str| self.layers.iter().any(|l| l.name == n); - if taken(base) { crate::item::numbered_name(base, taken) } else { base.to_string() } + if !self.layers.iter().any(|l| l.name == base) { + return base.to_string(); + } + let stem = crate::item::name_stem(base); + // The suffixes in use, gathered in one pass: trying each candidate against every layer + // made importing many layers of one name cubic. + let taken: std::collections::HashSet = self + .layers + .iter() + .filter_map(|l| l.name.strip_prefix(stem)?.strip_prefix(' ')) + .filter_map(|s| s.parse::().ok().filter(|n| n.to_string() == s)) + .collect(); + (2..).find(|i| !taken.contains(i)).map(|i| format!("{stem} {i}")).unwrap_or_default() } } diff --git a/crates/project/src/item.rs b/crates/project/src/item.rs index 4fcc06ab9..b055c60ba 100644 --- a/crates/project/src/item.rs +++ b/crates/project/src/item.rs @@ -8,12 +8,18 @@ use super::{Arc, BTreeMap, Deserialize, FrameRate, Label, Project, Serialize, Ti /// another layer of the same name. A number set off by a space is a counter and is replaced /// (`Red Solid 1` → `Red Solid 2`, `shot 2` → `shot 3`); digits that end a word belong to the /// name and stay (`v-b1.mp4` → `v-b1.mp4 2`, `clip2.mov` → `clip2.mov 2`). -pub(crate) fn numbered_name(base: &str, taken: impl Fn(&str) -> bool) -> String { +/// `base` without a trailing ` N`: digits are dropped only when a space separates them from a +/// nonempty stem, so `v-b1.mp4` and `Layer1` keep theirs. +pub(crate) fn name_stem(base: &str) -> &str { let head = base.trim_end_matches(|c: char| c.is_ascii_digit()); - let stem = match head.strip_suffix(' ') { + match head.strip_suffix(' ') { Some(stem) if head.len() < base.len() && !stem.trim_end().is_empty() => stem.trim_end(), _ => base.trim_end(), - }; + } +} + +pub(crate) fn numbered_name(base: &str, taken: impl Fn(&str) -> bool) -> String { + let stem = name_stem(base); (2u64..).map(|k| format!("{stem} {k}")).find(|n| !taken(n)).unwrap_or_default() } diff --git a/crates/project/src/render_queue.rs b/crates/project/src/render_queue.rs index 5418e5ac7..fa830cb1b 100644 --- a/crates/project/src/render_queue.rs +++ b/crates/project/src/render_queue.rs @@ -1713,7 +1713,22 @@ pub fn numbered_template(template: &str, n: u32) -> String { /// Make a name safe as a file name component. pub fn sanitize(name: &str) -> String { - name.chars().map(|c| if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect() + let mut s: String = name.chars().map(|c| if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect(); + // At most a file name's worth (255 UTF-8 bytes, whole characters only): a template repeating + // a long name token must not expand to gigabytes. A cut name ends in a hash of the whole + // name so long names that share a prefix stay distinct. + if s.len() > 255 { + // shortcut: 64-bit FNV-1a, not collision-resistant against crafted names; use a real hash if names become untrusted keys. + let hash = name.bytes().fold(0xcbf2_9ce4_8422_2325u64, |h, b| (h ^ u64::from(b)).wrapping_mul(0x0100_0000_01b3)); + let mut end = 255 - 17; + while !s.is_char_boundary(end) { + end -= 1; + } + s.truncate(end); + s.push_str(&format!("-{hash:016x}")); + } + // `.` and `..` would name the folder itself or its parent when the token is a path component. + if !s.is_empty() && s.chars().all(|c| c == '.') { "_".repeat(s.len()) } else { s } } #[cfg(test)] @@ -1737,10 +1752,32 @@ mod tests { use super::*; + #[test] + fn sanitize_limits_the_name_in_bytes() { + // 255 two-byte characters are cut to whole characters plus a 17-byte hash suffix. + let s = sanitize(&"é".repeat(255)); + assert!(s.len() <= 255 && s.len() > 235, "{}", s.len()); + assert!(s.starts_with("éé") && s.is_char_boundary(s.len() - 17)); + // Names that share their first 255 bytes stay distinct, and a name that fits is untouched. + let (a, b) = (sanitize(&format!("{}A", "界".repeat(85))), sanitize(&format!("{}B", "界".repeat(85)))); + assert_ne!(a, b); + assert!(a.len() <= 255 && b.len() <= 255); + assert_eq!(sanitize(&"n".repeat(255)), "n".repeat(255)); + assert_eq!(sanitize(&"é".repeat(127)), "é".repeat(127)); + } + #[test] fn templates_and_sequences() { let v = TemplateVars { comp_name: "Main/Title", project_name: "P", width: 1920, height: 1080, frame_rate: 29.97, start_frame: 0, end_frame: 9 }; assert_eq!(expand_template(DEFAULT_TEMPLATE, OutputFormat::H264, &v), "Main_Title.mp4"); + // A dot-only name is not a parent-folder component; a long one is cut to a file name. + for name in ["..", "."] { + let v = TemplateVars { comp_name: name, ..v }; + assert_eq!(expand_template("out/[compName]/x.[fileExtension]", OutputFormat::Gif, &v), format!("out/{}/x.gif", "_".repeat(name.len()))); + } + let long = "n".repeat(100_000); + let v2 = TemplateVars { comp_name: &long, ..v }; + assert_eq!(expand_template(&"[compName]".repeat(100), OutputFormat::Gif, &v2).len(), 255 * 100); assert_eq!(expand_template("[compName]_[width]x[height]@[frameRate].[fileExtension]", OutputFormat::Gif, &v), "Main_Title_1920x1080@29.97.gif"); assert_eq!(sequence_path("out/a_[#####].png", 42), "out/a_00042.png"); assert_eq!(sequence_path("a###.tif", 7), "a007.tif"); diff --git a/crates/project/src/tests.rs b/crates/project/src/tests.rs index d040c8f0b..80ba3c771 100644 --- a/crates/project/src/tests.rs +++ b/crates/project/src/tests.rs @@ -146,6 +146,18 @@ fn unique_names_and_cycles() { let c = p.comp(cid).unwrap(); assert_eq!(c.unique_layer_name("Red Solid 1"), "Red Solid 2"); assert_eq!(c.unique_layer_name("Shape Layer 1"), "Shape Layer 1"); + // A suffix written another way (`02`) is not the number taken. + let mut d = c.clone(); + for n in ["Red Solid 02", "Red Solid 3"] { + let mut l = d.layers[0].clone(); + l.name = n.into(); + d.layers.push(l); + } + assert_eq!(d.unique_layer_name("Red Solid 1"), "Red Solid 2"); + // Digits that end a word stay in the stem. + let mut e = c.clone(); + e.layers[0].name = "v-b1.mp4".into(); + assert_eq!(e.unique_layer_name("v-b1.mp4"), "v-b1.mp4 2"); assert!(p.comp_contains(cid, cid)); }