diff --git a/crates/ui-egui/src/files.rs b/crates/ui-egui/src/files.rs index cd54c775..f087a441 100644 --- a/crates/ui-egui/src/files.rs +++ b/crates/ui-egui/src/files.rs @@ -190,6 +190,28 @@ pub(crate) fn file_stem_from_user(name: &str) -> String { if reserved { format!("_{stem}") } else { stem.to_string() } } +/// A safe native Save suggestion for a PDF-supplied attachment name. +/// This changes only the suggestion, never the stored name or the selected destination. +#[cfg(any(test, not(target_arch = "wasm32")))] +pub(crate) fn attachment_file_name(name: &str) -> String { + let cleaned: String = name + .chars() + .filter(|c| !c.is_control() && !matches!(c, '\u{061c}' | '\u{200e}' | '\u{200f}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}')) + .map(|c| if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }) + .collect(); + let name = cleaned.trim_start().trim_end_matches(|c: char| c == '.' || c.is_whitespace()); + if name.is_empty() { + return "attachment".into(); + } + let device = name.split('.').next().unwrap_or_default().trim_end().to_ascii_uppercase(); + let reserved = matches!(device.as_str(), "CON" | "PRN" | "AUX" | "NUL") + || device + .strip_prefix("COM") + .or_else(|| device.strip_prefix("LPT")) + .is_some_and(|number| matches!(number, "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "¹" | "²" | "³")); + if reserved { format!("_{name}") } else { name.to_string() } +} + /// Pages ▸ Rotate Pages. #[derive(Clone, Debug, PartialEq)] pub struct RotateDraft { @@ -969,7 +991,7 @@ impl PdfCraftApp { #[cfg(test)] mod stem_tests { - use super::file_stem_from_user; + use super::{attachment_file_name, file_stem_from_user}; #[test] fn user_file_names_stay_inside_the_folder_and_writable_everywhere() { @@ -986,4 +1008,40 @@ mod stem_tests { let long = file_stem_from_user(&"文".repeat(300)); assert!(long.len() <= 200 && long.chars().all(|c| c == '文'), "{}", long.len()); } + #[test] + fn attachment_save_name_removes_bidi_without_hiding_the_extension() { + assert_eq!(attachment_file_name("invoice\u{202e}fdp.exe"), "invoicefdp.exe"); + for c in [ + '\u{061c}', '\u{200e}', '\u{200f}', '\u{202a}', '\u{202b}', '\u{202c}', '\u{202d}', '\u{202e}', '\u{2066}', '\u{2067}', '\u{2068}', + '\u{2069}', + ] { + assert_eq!(attachment_file_name(&format!("a{c}b.pdf")), "ab.pdf"); + } + } + #[test] + fn attachment_save_name_preserves_ordinary_unicode_and_extensions() { + for name in ["report.pdf", "archive.tar.gz", "مرحبا.pdf", "שלום.txt", "資料.pdf", "می\u{200c}روم.txt", "👩\u{200d}💻.png", "COM10.txt"] + { + assert_eq!(attachment_file_name(name), name); + } + } + #[test] + fn attachment_save_name_handles_controls_paths_and_device_names() { + assert_eq!(attachment_file_name("a\u{0}\n\u{85}b.pdf"), "ab.pdf"); + assert_eq!(attachment_file_name("../a\\b:c*?\"<>|.pdf"), ".._a_b_c______.pdf"); + for name in ["NUL.tar.gz", "con.txt", "COM1.exe", "LPT9.txt", "COM¹.txt", "LPT³.pdf"] { + assert_eq!(attachment_file_name(name), format!("_{name}")); + } + } + #[test] + fn attachment_save_name_has_a_stable_nonempty_fallback() { + for name in ["", " ", ".", "..", " . . ", "\u{202e}", "\u{0}\n"] { + assert_eq!(attachment_file_name(name), "attachment"); + } + assert_eq!(attachment_file_name(" report.pdf . \u{2003}."), "report.pdf"); + for name in ["invoice\u{202e}fdp.exe", "../a.pdf", "NUL.txt", " . . ", "report.pdf . ", "資料.pdf"] { + let once = attachment_file_name(name); + assert_eq!(attachment_file_name(&once), once); + } + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 11f5fc35..ff84c3ee 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1429,12 +1429,14 @@ impl PdfCraftApp { } (Ok(bytes), false) => { #[cfg(not(target_arch = "wasm32"))] - self.ask_one(pickers::Ask::Save(rfd::AsyncFileDialog::new().set_file_name(&att.name)), None, move |app, path| { - match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { + self.ask_one( + pickers::Ask::Save(rfd::AsyncFileDialog::new().set_file_name(crate::files::attachment_file_name(&att.name))), + None, + move |app, path| match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { Ok(()) => app.notify_fmt("Saved {name}", &[("name", &path.display().to_string())]), Err(e) => app.notify_fmt("Couldn't save: {e}", &[("e", &e.to_string())]), - } - }); + }, + ); #[cfg(target_arch = "wasm32")] self.notify_fmt("Downloading attachments on the web arrives with M3.10 ({n} bytes ready)", &[("n", &bytes.len().to_string())]); } diff --git a/crates/ui-egui/tests/pickers.rs b/crates/ui-egui/tests/pickers.rs index 99b3f0c5..dda27c1d 100644 --- a/crates/ui-egui/tests/pickers.rs +++ b/crates/ui-egui/tests/pickers.rs @@ -711,3 +711,82 @@ fn a_background_startup_forms_script_notice_waits_until_its_tab_is_selected() { h.run_steps(2); assert!(h.state().toast.is_none(), "the notice is delivered once"); } + +/// Embedded bytes and the PDF's original /UF name stay independent of the Save suggestion. +fn attachment_fixture() -> Vec { + let name = "invoice\u{202e}fdp.exe"; + let uf = std::iter::once(0xfeffu16).chain(name.encode_utf16()).map(|c| format!("{c:04X}")).collect::(); + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles << /Names [(notes.txt) 5 0 R] >> >> >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_string(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> >>".to_string(), + "<< /Type /EmbeddedFile /Length 11 >>\nstream\nhello notes\nendstream".to_string(), + format!("<< /Type /Filespec /F (notes.txt) /UF <{uf}> /EF << /F 4 0 R >> >>"), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(b"xref\n0 6\n0000000000 65535 f \n"); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size 6 /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n").as_bytes()); + out +} + +struct AttachmentDir(std::path::PathBuf); + +impl AttachmentDir { + fn new() -> Self { + use std::sync::atomic::{AtomicU64, Ordering}; + static NEXT: AtomicU64 = AtomicU64::new(0); + for _ in 0..128 { + let path = std::env::temp_dir().join(format!("pdfcraft-attachment-save-{}-{}", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed))); + match std::fs::create_dir(&path) { + Ok(()) => return Self(path), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => panic!("creating attachment Save test directory: {error}"), + } + } + panic!("no unused attachment Save test directory after 128 attempts"); + } +} + +impl Drop for AttachmentDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +#[test] +fn attachment_save_preserves_bytes_metadata_and_selected_destination() { + let dir = AttachmentDir::new(); + let chosen = dir.0.join("user chosen 資料.bin"); + let original = attachment_fixture(); + let mut h = harness(&[]); + h.state_mut().open_bytes("attachment.pdf", None, original.clone()).unwrap(); + h.run_steps(2); + let id = h.state().views[0].id; + let raw_name = "invoice\u{202e}fdp.exe"; + let index = h.state().session.get(id).unwrap().info.attachments.iter().position(|a| a.name == raw_name).unwrap(); + h.state_mut().pick_override = Some(vec![chosen.to_string_lossy().into_owned()]); + h.state_mut().attachment_action(id, index, false); + h.run_steps(2); + assert_eq!(std::fs::read(&chosen).unwrap(), b"hello notes"); + let doc = h.state().session.get(id).unwrap(); + assert_eq!(doc.info.attachments[index].name, raw_name); + assert_eq!(doc.bytes.as_ref(), &original); + assert!(!dir.0.join("invoicefdp.exe").exists()); + std::fs::remove_file(&chosen).unwrap(); + h.state_mut().pick_override = Some(Vec::new()); + h.state_mut().attachment_action(id, index, false); + h.run_steps(2); + assert_eq!(std::fs::read_dir(&dir.0).unwrap().count(), 0); + let doc = h.state().session.get(id).unwrap(); + assert_eq!(doc.info.attachments[index].name, raw_name); + assert_eq!(doc.bytes.as_ref(), &original); +}