From 891e90bfee560fc73bdcf591e16de060a1200e41 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 11:43:12 +0000 Subject: [PATCH] fix(engine): preset imports number duplicate names in linear time --- crates/engine/src/cmd.rs | 20 ++++++++++++++++++++ crates/engine/src/cmd/pdfcmds.rs | 8 ++++++-- crates/engine/src/cmd/printpresets.rs | 8 ++++++-- crates/engine/src/tests/cmdsplit.rs | 22 ++++++++++++++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 285604af5..1af3693ca 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -611,6 +611,26 @@ pub(crate) fn unique_name(base: &str, taken: impl Fn(&str) -> bool) -> String { (2..).map(|i| format!("{base} {i}")).find(|n| !taken(n)).unwrap_or_else(|| base.to_string()) } +/// [`unique_name`] for names given one after another (an import): `next` keeps, per base (in +/// any case), the number after the last one tried, so a run of one name doesn't count up from 2 +/// again for each copy. Names only become taken meanwhile, so the result is the same. +pub(crate) fn unique_name_from(base: &str, next: &mut std::collections::HashMap, taken: impl Fn(&str) -> bool) -> String { + if !taken(base) { + return base.to_string(); + } + let i = next.entry(base.to_lowercase()).or_insert(2); + loop { + let n = format!("{base} {i}"); + *i += 1; + if !taken(&n) { + return n; + } + } +} + +/// The most presets one presets file imports: each is looked up among the saved ones. +pub(crate) const MAX_IMPORTED_PRESETS: usize = 1000; + pub(crate) use edit::{settle_interaction, typing_in_progress}; pub(crate) use object::bounds_rewrite; pub(crate) use vectorcraft_doc::range::parse_range; diff --git a/crates/engine/src/cmd/pdfcmds.rs b/crates/engine/src/cmd/pdfcmds.rs index c9c94d808..b5a8778fe 100644 --- a/crates/engine/src/cmd/pdfcmds.rs +++ b/crates/engine/src/cmd/pdfcmds.rs @@ -8,7 +8,7 @@ use serde_json::{Value, json}; use vectorcraft_pdf::{PdfPreset, builtin_preset, builtin_presets}; use super::fileio::pdf::{find_preset, resolve}; -use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, str_param, unique_name}; +use super::{CommandSpec, Result, Session, always, bad, bool_or, cmd, str_param}; /// The `format` of a PDF presets file, also its extension. pub const PRESET_FORMAT: &str = "vcpdfpresets"; @@ -196,8 +196,12 @@ fn import(s: &mut Session, p: &Value) -> Result { for q in &file.presets { q.settings.check_values().map_err(|e| bad(C, format!("`{}`: {e}", q.name)))?; } + if file.presets.len() > super::MAX_IMPORTED_PRESETS { + return Err(bad(C, format!("a PDF presets file with over {} presets", super::MAX_IMPORTED_PRESETS))); + } let replace = bool_or(p, "replace", false); let mut imported = vec![]; + let mut next = std::collections::HashMap::new(); for mut preset in file.presets { preset.settings.clear_passwords(); let base = Some(preset.name.trim()).filter(|n| !n.is_empty()).unwrap_or("PDF Preset").to_string(); @@ -211,7 +215,7 @@ fn import(s: &mut Session, p: &Value) -> Result { } } None => { - preset.name = unique_name(&base, |n| s.pdf_preset_taken(n, None)); + preset.name = super::unique_name_from(&base, &mut next, |n| s.pdf_preset_taken(n, None)); imported.push(preset.name.clone()); s.store_pdf_preset(None, &preset); s.prefs.pdf_presets.push(preset); diff --git a/crates/engine/src/cmd/printpresets.rs b/crates/engine/src/cmd/printpresets.rs index 8f0dd298d..2cc44586e 100644 --- a/crates/engine/src/cmd/printpresets.rs +++ b/crates/engine/src/cmd/printpresets.rs @@ -8,7 +8,7 @@ use vectorcraft_pdf::PrintSettings; use super::fileio::pdf::changed; use super::print::settings_over; -use super::{CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, str_param, unique_name}; +use super::{CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, str_param}; /// The built-in preset's name. pub const DEFAULT_PRESET: &str = "[Default]"; @@ -221,8 +221,12 @@ fn import(s: &mut Session, p: &Value) -> Result { for q in &file.presets { q.settings.check().map_err(|e| bad(C, format!("`{}`: {e}", q.name)))?; } + if file.presets.len() > super::MAX_IMPORTED_PRESETS { + return Err(bad(C, format!("a print presets file with over {} presets", super::MAX_IMPORTED_PRESETS))); + } let replace = bool_or(p, "replace", false); let mut imported = vec![]; + let mut next = std::collections::HashMap::new(); for mut preset in file.presets { // [Default] comes in as a saved copy ("Default 2"…): the built-in one stays. let name = preset.name.trim().trim_start_matches('[').trim_end_matches(']').trim(); @@ -236,7 +240,7 @@ fn import(s: &mut Session, p: &Value) -> Result { } } None => { - preset.name = unique_name(&base, |n| s.print_preset_taken(n, None)); + preset.name = super::unique_name_from(&base, &mut next, |n| s.print_preset_taken(n, None)); imported.push(preset.name.clone()); s.prefs.print_presets.push(preset); } diff --git a/crates/engine/src/tests/cmdsplit.rs b/crates/engine/src/tests/cmdsplit.rs index af94ea3f3..2511e6f3d 100644 --- a/crates/engine/src/tests/cmdsplit.rs +++ b/crates/engine/src/tests/cmdsplit.rs @@ -66,6 +66,28 @@ fn unique_name_appends_the_first_free_number() { assert_eq!(unique_name("A 2", |n| taken.contains(&n)), "A 2 2"); } +#[test] +fn unique_name_from_counts_on_for_a_run_of_one_name() { + let mut taken = vec!["A".to_string(), "a 3".to_string()]; + let mut next = std::collections::HashMap::new(); + for want in ["A 2", "A 4", "A 5"] { + let is_taken = |n: &str| taken.iter().any(|t| t.eq_ignore_ascii_case(n)); + let n = crate::cmd::unique_name_from("A", &mut next, is_taken); + assert_eq!(n, want); + taken.push(n); + } + assert_eq!(crate::cmd::unique_name_from("B", &mut next, |_| false), "B"); +} + +#[test] +fn a_presets_file_with_too_many_presets_is_refused() { + let mut s = Session::new(); + let presets: Vec<_> = (0..=crate::cmd::MAX_IMPORTED_PRESETS).map(|_| serde_json::json!({"name": "P"})).collect(); + let file = serde_json::json!({"format": crate::cmd::printpresets::PRESET_FORMAT, "presets": presets}).to_string(); + let e = s.execute("print.presets.import", &serde_json::json!({"data": file})).unwrap_err().to_string(); + assert!(e.contains("over"), "{e}"); +} + #[test] fn leaf_targets_expand_groups_and_stop_at_compound_paths() { let mut s = session();