fix(ui): keep output file names taken from documents inside the chosen folder

An attachment opened as a tab is named with the same plain file name as
its Save suggestion, and Export certificate names the file the same way
from the certificate subject. Writing files into a chosen folder refuses
any name with a separator, a drive, or that is . or .., so no caller can
write outside the folder.
This commit is contained in:
aiongg
2026-10-16 18:03:46 +00:00
parent f0c9901c39
commit 08b44e858b
6 changed files with 129 additions and 17 deletions
+1 -2
View File
@@ -738,8 +738,7 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) {
match action {
Some(crate::sign_ui::CertAction::Trust(c)) => app.trust_certificate(*c),
Some(crate::sign_ui::CertAction::Export(c)) => {
let pem = crate::sign_ui::certificate_pem(&c);
app.write_files(&[(format!("{}.cer", c.display_name()), std::sync::Arc::new(pem.into_bytes()))], "Export certificate");
app.export_certificate(&c);
}
None => {}
}
+49 -4
View File
@@ -195,9 +195,9 @@ 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")))]
/// A plain file name for a PDF-supplied attachment name: the Save suggestion, and the tab name
/// (which later names files written from that tab). It never changes the stored name or a
/// destination the user selected.
pub(crate) fn attachment_file_name(name: &str) -> String {
let cleaned: String = name
.chars()
@@ -217,6 +217,14 @@ pub(crate) fn attachment_file_name(name: &str) -> String {
if reserved { format!("_{name}") } else { name.to_string() }
}
/// Whether `name` names a file directly inside a folder: not empty, `.` or `..`, with no `/` or
/// `\\`, and no drive (`C:`). On Windows any `:` is refused, which also rules out alternate data
/// streams.
pub(crate) fn is_plain_file_name(name: &str) -> bool {
let drive = matches!(name.as_bytes(), [letter, b':', ..] if letter.is_ascii_alphabetic());
!(name.is_empty() || name == "." || name == ".." || name.contains(['/', '\\']) || drive || (cfg!(windows) && name.contains(':')))
}
/// Pages ▸ Rotate Pages.
#[derive(Clone, Debug, PartialEq)]
pub struct RotateDraft {
@@ -667,7 +675,12 @@ impl PdfCraftApp {
/// [`Self::write_files`], then `after` once every file is written: now, or on a later frame
/// when the user chooses the folder. `after` never runs when writing fails or is cancelled.
/// Every name must be a plain file name ([`is_plain_file_name`]); otherwise nothing is written.
pub(crate) fn write_files_then(&mut self, named: Vec<(String, Arc<Vec<u8>>)>, title: &str, after: impl FnOnce(&mut Self) + Send + 'static) {
if let Some((name, _)) = named.iter().find(|(name, _)| !is_plain_file_name(name)) {
self.notify_fmt("Couldn't write {name}: {e}", &[("name", name), ("e", tl!("the name is not a plain file name"))]);
return;
}
#[cfg(not(target_arch = "wasm32"))]
{
let write = move |app: &mut Self, dir: std::path::PathBuf| {
@@ -996,7 +1009,7 @@ impl PdfCraftApp {
#[cfg(test)]
mod stem_tests {
use super::{attachment_file_name, file_stem_from_user};
use super::{attachment_file_name, file_stem_from_user, is_plain_file_name};
#[test]
fn user_file_names_stay_inside_the_folder_and_writable_everywhere() {
@@ -1049,4 +1062,36 @@ mod stem_tests {
assert_eq!(attachment_file_name(&once), once);
}
}
#[test]
fn write_files_refuses_names_that_leave_the_folder() {
let dir = std::env::temp_dir().join(format!("pdfcraft-write-files-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
let out = dir.join("a").join("out");
std::fs::create_dir_all(&out).unwrap();
let absolute = dir.join("abs.pdf").to_string_lossy().into_owned();
let mut app = crate::PdfCraftApp::new();
app.set_option("language", "en").unwrap();
app.export_dir_override = Some(out.to_string_lossy().into_owned());
for bad in ["../x.pdf", "..", ".", "", "sub/x.pdf", "sub\\x.pdf", "C:x.pdf", absolute.as_str()] {
assert!(!is_plain_file_name(bad), "{bad}");
// A plain name first: nothing is written when any name in the batch is refused.
app.write_files(&[("ok.pdf".into(), std::sync::Arc::new(b"%PDF".to_vec())), (bad.into(), std::sync::Arc::new(b"%PDF".to_vec()))], "t");
assert!(app.toast.as_ref().is_some_and(|t| t.0.contains("not a plain file name")), "{bad}");
}
let names: Vec<_> = walk(&dir);
assert!(names.is_empty(), "{names:?}");
app.write_files(&[("x.pdf".into(), std::sync::Arc::new(b"%PDF".to_vec()))], "t");
assert_eq!(walk(&dir), vec![out.join("x.pdf")]);
let _ = std::fs::remove_dir_all(&dir);
}
fn walk(dir: &std::path::Path) -> Vec<std::path::PathBuf> {
let mut out = Vec::new();
for entry in std::fs::read_dir(dir).unwrap() {
let path = entry.unwrap().path();
if path.is_dir() { out.extend(walk(&path)) } else { out.push(path) }
}
out
}
}
+2 -1
View File
@@ -1520,7 +1520,8 @@ impl PdfCraftApp {
match (data, open) {
(Err(e), _) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &att.name), ("e", &e.to_string())]),
(Ok(bytes), true) => {
if let Err(e) = self.open_bytes(&att.name, None, bytes) {
// The tab name later names files written from the tab (Split, Extract pages).
if let Err(e) = self.open_bytes(&crate::files::attachment_file_name(&att.name), None, bytes) {
self.notify_fmt("Couldn't open {name}: {e}", &[("name", &att.name), ("e", &e.to_string())]);
}
}
+1 -2
View File
@@ -784,8 +784,7 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) {
app.start_signing(page, None, Some(field), None);
}
Some(crate::sign_ui::PanelAction::ExportCertificate(c)) => {
let pem = crate::sign_ui::certificate_pem(&c);
app.write_files(&[(format!("{}.cer", c.display_name()), std::sync::Arc::new(pem.into_bytes()))], "Export certificate");
app.export_certificate(&c);
}
None => {}
}
+23
View File
@@ -266,6 +266,12 @@ fn entry_unusable(u: &sign::windows::Unusable) -> DigitalIdEntry {
}
impl PdfCraftApp {
/// Export certificate: write `c` as PEM into a chosen folder, named after its subject.
pub(crate) fn export_certificate(&mut self, c: &Certificate) {
let name = format!("{}.cer", crate::files::attachment_file_name(&c.display_name()));
self.write_files(&[(name, std::sync::Arc::new(certificate_pem(c).into_bytes()))], "Export certificate");
}
/// Start signing: the rectangle (or field) is known; show Sign with a Digital ID.
pub fn start_signing(&mut self, page: usize, rect: Option<[f64; 4]>, field: Option<String>, certify: Option<u8>) {
self.refresh_os_key_store_ids();
@@ -1496,6 +1502,23 @@ mod tests {
Ok(PathBuf::from(&app.digital_ids[i].path))
}
#[test]
fn an_exported_certificate_is_written_inside_the_chosen_folder() {
let dir = scratch();
let out = dir.join("out");
std::fs::create_dir_all(&out).unwrap();
let key = PrivateKey::generate_p256().unwrap();
let cert = Certificate::self_signed(&Name::build("../x", "", "", "", ""), &key, sign::Time::from_unix(0), 1, &[1]).unwrap();
let mut app = PdfCraftApp::new();
app.export_dir_override = Some(out.to_string_lossy().into_owned());
app.export_certificate(&cert);
let written: Vec<_> = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().file_name()).collect();
assert_eq!(written, vec![std::ffi::OsString::from("out")]);
let pem = std::fs::read_to_string(out.join(".._x.cer")).unwrap();
assert!(pem.starts_with("-----BEGIN CERTIFICATE-----"), "{pem}");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn signature_status_style_gives_each_verdict_its_panel_icon() {
assert_eq!(signature_status_style(SignatureStatus::Valid).0, "circle-check");
+53 -8
View File
@@ -748,20 +748,26 @@ fn a_background_startup_forms_script_notice_waits_until_its_tab_is_selected() {
/// Embedded bytes and the PDF's original /UF name stay independent of the Save suggestion.
fn attachment_fixture() -> Vec<u8> {
let name = "invoice\u{202e}fdp.exe";
attachment_pdf("invoice\u{202e}fdp.exe", b"hello notes")
}
/// A one-page PDF with one embedded file whose /UF name is `name`.
fn attachment_pdf(name: &str, data: &[u8]) -> Vec<u8> {
let uf = std::iter::once(0xfeffu16).chain(name.encode_utf16()).map(|c| format!("{c:04X}")).collect::<String>();
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 objs: [Vec<u8>; 5] = [
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles << /Names [(notes.txt) 5 0 R] >> >> >>".to_vec(),
b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> >>".to_vec(),
[format!("<< /Type /EmbeddedFile /Length {} >>\nstream\n", data.len()).as_bytes(), data, b"\nendstream"].concat(),
format!("<< /Type /Filespec /F (notes.txt) /UF <{uf}> /EF << /F 4 0 R >> >>").into_bytes(),
];
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());
out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes());
out.extend_from_slice(o);
out.extend_from_slice(b"\nendobj\n");
}
let xref = out.len();
out.extend_from_slice(b"xref\n0 6\n0000000000 65535 f \n");
@@ -824,3 +830,42 @@ fn attachment_save_preserves_bytes_metadata_and_selected_destination() {
assert_eq!(doc.info.attachments[index].name, raw_name);
assert_eq!(doc.bytes.as_ref(), &original);
}
/// Every file under `dir`, recursively, sorted.
fn files_under(dir: &std::path::Path) -> Vec<std::path::PathBuf> {
let mut out = Vec::new();
for entry in std::fs::read_dir(dir).unwrap() {
let path = entry.unwrap().path();
if path.is_dir() { out.extend(files_under(&path)) } else { out.push(path) }
}
out.sort();
out
}
#[test]
fn files_written_from_an_attachment_tab_stay_inside_the_chosen_folder() {
let dir = AttachmentDir::new();
let absolute = dir.0.join("elsewhere").join("x.pdf").to_string_lossy().into_owned();
for name in ["../../x.pdf", absolute.as_str()] {
let out = dir.0.join("a").join("b").join("out");
std::fs::create_dir_all(&out).unwrap();
let mut h = harness(&[]);
h.state_mut().open_bytes("outer.pdf", None, attachment_pdf(name, &fixture(2))).unwrap();
h.run_steps(2);
let id = h.state().views[0].id;
h.state_mut().attachment_action(id, 0, true);
h.run_steps(2);
assert_eq!(h.state().views.len(), 2, "{name}: the attachment opens as a tab");
h.state_mut().export_dir_override = Some(out.to_string_lossy().into_owned());
h.state_mut().split_active(&pdfcraft_ui_egui::SplitPlan::By(pdfcraft_engine::SplitBy::PageCount(1)));
let written = files_under(&dir.0);
assert_eq!(written.len(), 2, "{name}: two split parts, got {written:?}");
assert!(written.iter().all(|p| p.parent() == Some(out.as_path())), "{name}: {written:?}");
// Extract writes page 1 under the name Split gave it.
h.state_mut().extract_draft = pdfcraft_ui_egui::ExtractDraft { separate: true, ..Default::default() };
h.state_mut().extract_selection();
assert!(h.state().toast.as_ref().is_some_and(|t| t.0.starts_with("Wrote 1 file")), "{name}: {:?}", h.state().toast);
assert_eq!(files_under(&dir.0), written, "{name}");
std::fs::remove_dir_all(dir.0.join("a")).unwrap();
}
}