diff --git a/crates/automation/src/bates.rs b/crates/automation/src/bates.rs index 5267716e85..ffb2d42723 100644 --- a/crates/automation/src/bates.rs +++ b/crates/automation/src/bates.rs @@ -8,6 +8,8 @@ use super::json::Args; use super::paths::read_input; use super::{Arc, Automation, Path, PathBuf}; +const MAX_BATES_BYTES: u64 = 512 * 1024 * 1024; + struct FilePlan { input: String, source: PathBuf, @@ -26,19 +28,23 @@ impl Automation { } let start = match args.0.get("start_number") { None | Some(Value::Null) => 1, - Some(value) => value - .as_u64() - .filter(|n| *n >= 1) - .ok_or_else(|| ToolError::InvalidArgs("start_number must be a whole number of at least 1".into()))?, + Some(value) => { + value.as_u64().filter(|n| *n >= 1).ok_or_else(|| ToolError::InvalidArgs("start_number must be an integer of at least 1".into()))? + } }; let digits = args.opt_int("digits")?.unwrap_or(6); let digits = u8::try_from(digits).ok().filter(|d| (1..=12).contains(d)).ok_or_else(|| ToolError::InvalidArgs("digits must be from 1 to 12".into()))?; let prefix = args.opt_str("prefix")?.unwrap_or(""); let suffix = args.opt_str("suffix")?.unwrap_or(""); - // `#` separates the token's fields and `>>` ends it: either would cut the token short. - if prefix.len() > 128 || suffix.len() > 128 || [prefix, suffix].iter().any(|s| s.contains('#') || s.contains(">>")) { - return Err(ToolError::InvalidArgs("prefix and suffix must be at most 128 bytes and cannot contain # or >>".into())); + if prefix.chars().count() > 128 + || suffix.chars().count() > 128 + || prefix.contains('#') + || suffix.contains('#') + || prefix.contains(">>") + || suffix.contains(">>") + { + return Err(ToolError::InvalidArgs("prefix and suffix must be at most 128 characters and cannot contain # or >>".into())); } let dry_run = args.opt_bool("dry_run")?.unwrap_or(false); @@ -70,16 +76,7 @@ impl Automation { targets.push((output.to_string(), target)); passwords.push(entry.get("password").and_then(Value::as_str).map(str::to_owned)); } - let input_paths: std::collections::HashSet<_> = sources.iter().map(|(_, path)| path.clone()).collect(); - let mut output_paths = std::collections::HashSet::new(); - for (name, path) in &targets { - if input_paths.contains(path) { - return Err(failed(format!("output {name} names an input file; choose a separate output path"))); - } - if !output_paths.insert(path.clone()) { - return Err(failed(format!("two files use the same output path: {name}"))); - } - } + check_unique_outputs(&targets, dry_run)?; let mut plans = Vec::with_capacity(files.len()); let mut next = start; @@ -88,13 +85,13 @@ impl Automation { let size = std::fs::metadata(input_path).map_err(failed)?.len(); input_bytes = input_bytes .checked_add(size) - .filter(|total| *total <= 512 * 1024 * 1024) + .filter(|total| *total <= MAX_BATES_BYTES) .ok_or_else(|| failed("combined Bates input exceeds the 512 MiB safety limit"))?; let bytes = read_input(input_path)?; input_bytes = input_bytes .saturating_sub(size) .checked_add(u64::try_from(bytes.len()).unwrap_or(u64::MAX)) - .filter(|total| *total <= 512 * 1024 * 1024) + .filter(|total| *total <= MAX_BATES_BYTES) .ok_or_else(|| failed("combined Bates input exceeds the 512 MiB safety limit"))?; let display_name = input_path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let password = passwords.get(index).and_then(Option::as_deref); @@ -106,7 +103,8 @@ impl Automation { return Err(failed(format!("{input_name} has no pages to number"))); } let count = u64::try_from(pages).map_err(|_| failed(format!("page count is too large for {input_name}")))?; - let last = next.checked_add(count.saturating_sub(1)).ok_or_else(|| failed(format!("Bates number range overflows for {input_name}")))?; + let last_offset = count.checked_sub(1).ok_or_else(|| failed(format!("{input_name} has no pages to number")))?; + let last = next.checked_add(last_offset).ok_or_else(|| failed(format!("Bates number range overflows for {input_name}")))?; plans.push(FilePlan { input: input_name.clone(), source: input_path.clone(), @@ -116,7 +114,7 @@ impl Automation { first: next, last, }); - if index + 1 < sources.len() { + if index + 1 < files.len() { next = last.checked_add(1).ok_or_else(|| failed(format!("Bates sequence overflows after {input_name}")))?; } } @@ -163,12 +161,14 @@ impl Automation { let result = applied.and_then(|()| self.session.save_full_bytes(id)); match result { Ok(bytes) => { - let total = output_bytes.saturating_add(u64::try_from(bytes.len()).unwrap_or(u64::MAX)); - if total > 512 * 1024 * 1024 { - results.push(json!({"input":plan.input,"output":plan.output,"status":"error","pages":plan.pages,"first":plan.first,"last":plan.last,"error":"combined Bates output exceeds the 512 MiB safety limit"})); - } else { + let candidate = output_bytes + .checked_add(u64::try_from(bytes.len()).unwrap_or(u64::MAX)) + .filter(|total| *total <= MAX_BATES_BYTES); + if let Some(total) = candidate { output_bytes = total; staged.push((plan.output.clone(), bytes)); + } else { + results.push(json!({"input":plan.input,"output":plan.output,"status":"error","pages":plan.pages,"first":plan.first,"last":plan.last,"error":"combined Bates output exceeds the 512 MiB safety limit"})); } } Err(error) => results.push(json!({"input":plan.input,"output":plan.output,"status":"error","pages":plan.pages,"first":plan.first,"last":plan.last,"error":error.to_string()})), @@ -198,6 +198,123 @@ impl Automation { } } +fn check_unique_outputs(targets: &[(String, PathBuf)], dry_run: bool) -> Result<()> { + if targets.len() < 2 { + return Ok(()); + } + + // Probe exact basenames in a directory under each canonical parent, so the filesystem applies its own name rules. + let mut groups: std::collections::HashMap)> = std::collections::HashMap::new(); + for (name, target) in targets { + let parent = target.parent().ok_or_else(|| failed(format!("output has no parent directory: {name}")))?; + let identity = parent.canonicalize().map_err(failed)?; + let probe_name = output_probe_name(target).map_err(failed)?; + groups.entry(identity.clone()).or_insert_with(|| (identity, Vec::new())).1.push((name.clone(), probe_name)); + } + + if dry_run { + // Dry runs cannot probe the volume: Windows uses a conservative case-insensitive comparison, + // while Unix preserves case, so unusual case-sensitive Windows or case-insensitive Unix volumes can differ. + for (_, (_, names)) in groups { + for (index, (name, candidate)) in names.iter().enumerate() { + if names.iter().take(index).any(|(_, prior)| same_output_name(prior, candidate)) { + return Err(failed(format!("two files use the same output path: {name}"))); + } + } + } + return Ok(()); + } + + for (_, (parent, names)) in groups { + match tempfile::tempdir_in(&parent) { + Ok(probes) => { + for (name, probe_name) in names { + let probe = probes.path().join(probe_name); + match std::fs::OpenOptions::new().write(true).create_new(true).open(&probe) { + Ok(file) => drop(file), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + return Err(failed(format!("two files use the same output path: {name}"))); + } + Err(error) => return Err(failed(format!("could not validate output path {name}: {error}"))), + } + } + } + #[cfg(windows)] + Err(error) if error.kind() == std::io::ErrorKind::PermissionDenied => probe_files_for_resolved_outputs(&parent, names)?, + Err(error) => return Err(failed(error)), + } + } + Ok(()) +} + +#[cfg(windows)] +fn probe_files_for_resolved_outputs(parent: &Path, names: Vec<(String, std::ffi::OsString)>) -> Result<()> { + use std::os::windows::fs::OpenOptionsExt; + + const FILE_FLAG_DELETE_ON_CLOSE: u32 = 0x0400_0000; + let mut probes = Vec::with_capacity(names.len()); + for (name, probe_name) in names { + let probe = parent.join(probe_name); + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true).share_mode(0).custom_flags(FILE_FLAG_DELETE_ON_CLOSE); + match options.open(&probe) { + Ok(file) => probes.push(file), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + return Err(failed(format!("two files use the same output path: {name}"))); + } + Err(error) => return Err(failed(format!("could not validate output path {name}: {error}"))), + } + } + drop(probes); + Ok(()) +} + +fn same_output_name(left: &std::ffi::OsStr, right: &std::ffi::OsStr) -> bool { + #[cfg(windows)] + { + use std::os::windows::ffi::OsStrExt; + fn key(name: &std::ffi::OsStr) -> Vec { + std::char::decode_utf16(name.encode_wide()) + .map(|decoded| match decoded { + Ok(character) => { + let mut uppercase = character.to_uppercase(); + match (uppercase.next(), uppercase.next()) { + (Some(single), None) => single as u32, + _ => character as u32, + } + } + Err(error) => 0x11_0000 + u32::from(error.unpaired_surrogate()), + }) + .collect() + } + return key(left) == key(right); + } + #[cfg(not(windows))] + { + left == right + } +} + +fn output_probe_name(path: &Path) -> std::io::Result { + let name = path.file_name().ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidInput, "output has no file name"))?; + #[cfg(windows)] + { + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + let is_verbatim = path.components().any(|component| matches!(component, std::path::Component::Prefix(prefix) if matches!(prefix.kind(), std::path::Prefix::Verbatim(_) | std::path::Prefix::VerbatimUNC(_, _) | std::path::Prefix::VerbatimDisk(_)))); + let mut wide: Vec = name.encode_wide().collect(); + if !is_verbatim { + while wide.last().is_some_and(|unit| *unit == u16::from(b'.') || *unit == u16::from(b' ')) { + wide.pop(); + } + } + return Ok(std::ffi::OsString::from_wide(&wide)); + } + #[cfg(not(windows))] + { + Ok(name.to_os_string()) + } +} + /// Publish a PDF atomically while refusing a target created by another process after preflight. pub(super) fn publish_new(path: &Path, bytes: &[u8]) -> Result<()> { let parent = path.parent().ok_or_else(|| failed("output has no parent directory"))?; @@ -212,3 +329,100 @@ pub(super) fn publish_new(path: &Path, bytes: &[u8]) -> Result<()> { } }) } + +#[cfg(test)] +mod tests { + use super::check_unique_outputs; + + #[test] + fn dry_run_path_check_creates_no_probe_files() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("same.pdf"); + let targets = vec![("first.pdf".into(), path.clone()), ("second.pdf".into(), path)]; + assert!(check_unique_outputs(&targets, true).is_err()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 0); + } + + #[cfg(unix)] + #[test] + fn dry_run_detects_output_parent_aliases_without_writing() { + let dir = tempfile::tempdir().unwrap(); + let alias = dir.path().join("alias"); + std::os::unix::fs::symlink(dir.path(), &alias).unwrap(); + let first = dir.path().join("same.pdf"); + let second = alias.join("same.pdf"); + let targets = vec![("first.pdf".into(), first), ("second.pdf".into(), second)]; + assert!(check_unique_outputs(&targets, true).is_err()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 1); + } + + #[test] + fn long_output_names_fit_in_probe_directory() { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join(format!("{}.pdf", "a".repeat(220))); + let second = dir.path().join(format!("{}.pdf", "b".repeat(220))); + let targets = vec![("first.pdf".into(), first), ("second.pdf".into(), second)]; + assert!(check_unique_outputs(&targets, false).is_ok()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 0); + } + + #[cfg(unix)] + #[test] + fn output_probe_paths_preserve_non_utf8_names() { + use std::os::unix::ffi::OsStringExt; + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join(std::ffi::OsString::from_vec(vec![b'o', 0xff])); + let second = dir.path().join(std::ffi::OsString::from_vec(vec![b'o', 0xfe])); + let targets = vec![("first.pdf".into(), first), ("second.pdf".into(), second)]; + assert!(check_unique_outputs(&targets, false).is_ok()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 0); + } + + #[cfg(windows)] + #[test] + fn windows_probe_names_trim_only_non_verbatim_paths() { + use std::ffi::OsString; + use std::path::Path; + + let normal = Path::new(r"C:\root\report.pdf. "); + assert_eq!(super::output_probe_name(normal).unwrap(), OsString::from("report.pdf")); + + let verbatim = Path::new(r"\\?\C:\root\report.pdf. "); + assert_eq!(super::output_probe_name(verbatim).unwrap(), OsString::from("report.pdf. ")); + } + + #[cfg(windows)] + #[test] + fn dry_run_detects_case_only_output_collisions_without_writing() { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("Case.pdf"); + let second = dir.path().join("CASE.pdf"); + let targets = vec![("first.pdf".into(), first), ("second.pdf".into(), second)]; + assert!(check_unique_outputs(&targets, true).is_err()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 0); + } + + #[cfg(windows)] + #[test] + fn permission_denied_child_directory_uses_delete_on_close_file_probes() { + use std::process::Command; + + let dir = tempfile::tempdir().unwrap(); + let identity = Command::new("whoami.exe").args(["/user", "/fo", "csv", "/nh"]).output().unwrap(); + assert!(identity.status.success()); + let identity = String::from_utf8(identity.stdout).unwrap(); + let sid = identity.split('"').nth(3).unwrap(); + let acl = Command::new("icacls.exe").arg(dir.path()).arg("/deny").arg(format!("*{sid}:(AD)")).status().unwrap(); + assert!(acl.success()); + + assert!(matches!(tempfile::tempdir_in(dir.path()), Err(error) if error.kind() == std::io::ErrorKind::PermissionDenied)); + let direct_probe = dir.path().join("direct-probe"); + let file = std::fs::OpenOptions::new().write(true).create_new(true).open(&direct_probe).unwrap(); + drop(file); + std::fs::remove_file(&direct_probe).unwrap(); + + let targets = vec![("first.pdf".into(), dir.path().join("first.pdf")), ("second.pdf".into(), dir.path().join("second.pdf"))]; + assert!(check_unique_outputs(&targets, false).is_ok()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 0); + } +} diff --git a/crates/automation/src/file_access.rs b/crates/automation/src/file_access.rs index dab2760f5d..ff91152567 100644 --- a/crates/automation/src/file_access.rs +++ b/crates/automation/src/file_access.rs @@ -393,7 +393,8 @@ mod tests { fn tools_do_not_touch_the_file_system_without_resolving_the_path() { // Helpers that are only called with a resolved path. `doc_batch` (batch.rs) resolves its // inputs and out_dir, walks only inside them (skipping links) and resolves every - // discovered file again before `process` reads it or `publish` writes it. + // discovered file again before `process` reads it or `publish` writes it. `doc_bates_multi` + // resolves its paths before checking output collisions and using the probe helpers. const RESOLVED_BY_CALLER: &[&str] = &[ "write_atomic", "write_atomic_with", @@ -405,6 +406,8 @@ mod tests { "linked", "walk", "publish", + "check_unique_outputs", + "probe_files_for_resolved_outputs", ]; let mut dirs = vec![Path::new(env!("CARGO_MANIFEST_DIR")).join("src")]; let mut files = vec![]; diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 16371c6a70..8188045b87 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -140,8 +140,8 @@ pub fn tools() -> Vec { .with(schema(json!({ "files": {"type":"array","minItems":1,"maxItems":1000,"description":"PDFs in Bates sequence order.","items":{"type":"object","properties":{"input":{"type":"string","minLength":1,"description":"Input PDF path."},"output":{"type":"string","minLength":1,"description":"New output PDF path; must not exist."},"password":{"type":"string","description":"Open or permissions password, if needed."}},"required":["input","output"],"additionalProperties":false}}, "start_number": {"type":"integer","minimum":1,"description":"First number in the sequence (default 1)."}, "digits": {"type":"integer","minimum":1,"maximum":12,"description":"Zero-padded number width (default 6)."}, - "prefix": {"type":"string","maxLength":128,"description":"Text before the number: at most 128 UTF-8 bytes, without # or >>."}, "suffix": {"type":"string","maxLength":128,"description":"Text after the number: at most 128 UTF-8 bytes, without # or >>."}, - "dry_run": {"type":"boolean","description":"Return each file's inclusive number interval without writing (default false)."} + "prefix": {"type":"string","maxLength":128,"description":"Bates prefix, up to 128 characters; cannot contain # or >>."}, "suffix": {"type":"string","maxLength":128,"description":"Bates suffix, up to 128 characters; cannot contain # or >>."}, + "dry_run": {"type":"boolean","description":"Return each file's inclusive number interval without writing (default false). Output collisions are checked conservatively: Windows compares case-insensitively, while Unix preserves case, so case-sensitive Windows and case-insensitive Unix volumes can differ from normal runs."} }), &["files"])), t("doc_batch", "Process files and folders", "Run convert, ocr and/or optimize over files and folders, one file at a time. recursive defaults to false. Preserve paths relative to each selected folder inside out_dir; duplicate inputs run once. Skip unsupported files, links and the output subtree. Existing outputs receive numbered names and are never replaced. Continue after per-file errors unless stop_on_error is true. Return completed/failed/skipped counts and per-file results. Discovery is bounded to 10000 entries and 64 folder levels. Convert must be first; each step can appear once. OCR uses installed models. Optimize uses Reduce File Size settings, optionally overriding target ppi (threshold 1.5 times ppi; black-and-white images go to the larger of ppi and 300) and JPEG quality; a larger candidate keeps the original bytes. Sources and open documents are unchanged.") .with(schema(json!({ diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index da634490e4..1192050e51 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -2969,6 +2969,15 @@ fn bates_numbers_continue_across_files_and_report_preflight_and_permission_failu {"input":"a.pdf","output":"a-numbered.pdf"} ]); let mut a = auto(&dir); + for invalid_start in [json!(0), json!(-1), json!(1.5), json!("17")] { + let error = err(&mut a, "doc_bates_multi", json!({"files":files,"start_number":invalid_start})); + assert!(matches!(error, ToolError::InvalidArgs(_))); + } + let unicode = ok(&mut a, "doc_bates_multi", json!({"files":files,"prefix":"é".repeat(65),"dry_run":true})); + assert_eq!(unicode["files"].as_array().unwrap().len(), 2); + assert!(matches!(err(&mut a, "doc_bates_multi", json!({"files":files,"prefix":"CASE>>"})), ToolError::InvalidArgs(_))); + assert!(matches!(err(&mut a, "doc_bates_multi", json!({"files":files,"suffix":"A>>B"})), ToolError::InvalidArgs(_))); + let allocation = ok(&mut a, "doc_bates_multi", json!({"files":files,"start_number":50,"dry_run":true})); assert_eq!(allocation["files"][0]["first"], 50); assert_eq!(allocation["files"][0]["last"], 51); @@ -2987,15 +2996,24 @@ fn bates_numbers_continue_across_files_and_report_preflight_and_permission_failu let overflow = err(&mut a, "doc_bates_multi", json!({"files":[{"input":"b.pdf","output":"overflow.pdf"}],"start_number":u64::MAX})); assert!(overflow.to_string().contains("overflows")); assert!(!dir.join("overflow.pdf").exists()); - // A range may end at u64::MAX itself. - let at_max = ok(&mut a, "doc_bates_multi", json!({"files":[{"input":"b.pdf","output":"max.pdf"}],"start_number":u64::MAX - 1,"dry_run":true})); - assert_eq!(at_max["files"][0]["last"], u64::MAX); - for bad in - [json!({"start_number":-3}), json!({"start_number":2.5}), json!({"start_number":"7"}), json!({"prefix":"CASE>>"}), json!({"suffix":"A>>B"})] + std::fs::write(dir.join("one.pdf"), fixture(1)).unwrap(); + let max_number = + ok(&mut a, "doc_bates_multi", json!({"files":[{"input":"one.pdf","output":"one-numbered.pdf"}],"start_number":u64::MAX,"dry_run":true})); + assert_eq!(max_number["files"][0]["first"], u64::MAX); + assert_eq!(max_number["files"][0]["last"], u64::MAX); + + #[cfg(windows)] { - let mut args = json!({"files":[{"input":"b.pdf","output":"bad.pdf"}],"dry_run":true}); - args.as_object_mut().unwrap().extend(bad.as_object().unwrap().clone()); - assert!(a.call("doc_bates_multi", &args).is_err(), "{bad}"); + let collision = err( + &mut a, + "doc_bates_multi", + json!({"files":[ + {"input":"b.pdf","output":"Case.pdf"}, + {"input":"a.pdf","output":"CASE.pdf"} + ]}), + ); + assert!(collision.to_string().contains("same output path")); + assert!(!dir.join("Case.pdf").exists()); } use pdfcraft_engine::{Changes, Edit, Protection, Session};