diff --git a/crates/platform/Cargo.toml b/crates/platform/Cargo.toml index 1808c4128f..4342565a0d 100644 --- a/crates/platform/Cargo.toml +++ b/crates/platform/Cargo.toml @@ -12,8 +12,8 @@ workspace = true log = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] -# `write_atomically`: a volume that refuses F_FULLFSYNC gets a plain fsync. -rustix = { version = "1", features = ["fs"] } +# Atomic saves use explicit xattr size queries and fall back to plain fsync if F_FULLFSYNC is refused. +rustix = { version = "1.1.6", features = ["fs"] } [target.'cfg(target_os = "linux")'.dependencies] # Already in the dependency graph through the accessibility integration. The portal watcher diff --git a/crates/platform/src/staging.rs b/crates/platform/src/staging.rs index b9b2a80443..a604dd58b5 100644 --- a/crates/platform/src/staging.rs +++ b/crates/platform/src/staging.rs @@ -220,7 +220,7 @@ pub fn write_atomically_with( /// Provenance is managed by macOS for the new file, not writable document metadata. #[cfg(target_os = "macos")] fn preserve_target_xattrs(path: &Path, staged: &std::fs::File) -> std::io::Result<()> { - use rustix::fs::{Mode, OFlags, XattrFlags, fgetxattr, flistxattr, fsetxattr, open}; + use rustix::fs::{Mode, OFlags, XattrFlags, fgetxattr, fgetxattr_query, flistxattr, fsetxattr, open}; use rustix::io::Errno; // A new destination has nothing to retain. Replacing a symlink has always replaced the @@ -257,12 +257,13 @@ fn preserve_target_xattrs(path: &Path, staged: &std::fs::File) -> std::io::Resul continue; } let name = std::ffi::CString::new(name).map_err(|_| std::io::Error::other("invalid extended attribute name"))?; - let size = fgetxattr(&source, &name, &mut [] as &mut [u8])?; + // rustix treats an empty read buffer as reading zero bytes, not a size query. + let size = fgetxattr_query(&source, &name)?; if size > MAX_VALUE_BYTES || size > MAX_TOTAL_BYTES.saturating_sub(total) { return Err(std::io::Error::other("extended attributes exceed the 16 MiB per-value or 64 MiB total limit")); } // Resource forks may return a prefix instead of ERANGE on growth; check the size again. - let value = read_xattr_value(size, |buffer| Ok(fgetxattr(&source, &name, buffer)?))?; + let value = read_xattr_value(size, |buffer| Ok(fgetxattr(&source, &name, buffer)?), || Ok(fgetxattr_query(&source, &name)?))?; fsetxattr(staged, &name, &value, XattrFlags::empty())?; total = total.saturating_add(value.len()); } @@ -271,10 +272,14 @@ fn preserve_target_xattrs(path: &Path, staged: &std::fs::File) -> std::io::Resul /// Read an already size-bounded attribute, refusing a partial result or a changed post-read size. #[cfg(target_os = "macos")] -fn read_xattr_value(size: usize, mut read: impl FnMut(&mut [u8]) -> std::io::Result) -> std::io::Result> { +fn read_xattr_value( + size: usize, + read: impl FnOnce(&mut [u8]) -> std::io::Result, + query_size: impl FnOnce() -> std::io::Result, +) -> std::io::Result> { let mut value = vec![0u8; size.max(1)]; let len = read(value.as_mut_slice())?; - let after = read(&mut [])?; + let after = query_size()?; if len != size || after != size { return Err(std::io::Error::other("extended attribute changed size while saving")); } diff --git a/crates/platform/src/staging/tests.rs b/crates/platform/src/staging/tests.rs index da1e015e15..699ba9bbf9 100644 --- a/crates/platform/src/staging/tests.rs +++ b/crates/platform/src/staging/tests.rs @@ -130,6 +130,7 @@ fn atomic_saves_keep_finder_tags_and_binary_extended_attributes() { ("com.apple.metadata:_kMDItemUserTags", b"PdfCraft\n6"), ("com.pdfcraft.test.binary", b"\0\xff\x80original\0"), ("com.pdfcraft.test.empty", b""), + ("com.apple.ResourceFork", b"\0\xfffork"), ]; std::fs::write(&target, b"OLD").unwrap(); for (name, value) in attributes { @@ -188,7 +189,7 @@ fn a_failed_flush_keeps_the_original_extended_attributes() { #[cfg(target_os = "macos")] #[test] fn a_growing_resource_fork_refuses_publication_and_keeps_original() { - use rustix::fs::{XattrFlags, fgetxattr, fsetxattr, getxattr, setxattr}; + use rustix::fs::{XattrFlags, fgetxattr_query, fsetxattr, getxattr, setxattr}; let dir = test_dir("xattrs-growing-fork"); let target = dir.join("out.pdf"); let name = "com.apple.ResourceFork"; @@ -197,17 +198,18 @@ fn a_growing_resource_fork_refuses_publication_and_keeps_original() { setxattr(&target, name, b"ABC", XattrFlags::empty()).unwrap(); let source = std::fs::File::open(&target).unwrap(); let result = write_atomically_with(&target, b"REPLACEMENT PDF", form, [23], |staged| { - let size = fgetxattr(&source, name, &mut [] as &mut [u8])?; - let value = read_xattr_value(size, |buffer| { - if buffer.is_empty() { - return Ok(fgetxattr(&source, name, buffer)?); - } - // HFS can return the prefix that fits instead of ERANGE. APFS need not - // have that behaviour: model the kernel response, not a timing race. - setxattr(&target, name, b"ABCD", XattrFlags::empty())?; - buffer.get_mut(..3).unwrap().copy_from_slice(b"ABC"); - Ok(3) - })?; + let size = fgetxattr_query(&source, name)?; + let value = read_xattr_value( + size, + |buffer| { + // HFS can return the prefix that fits instead of ERANGE. APFS need not + // have that behaviour: model the kernel response, not a timing race. + setxattr(&target, name, b"ABCD", XattrFlags::empty())?; + buffer.get_mut(..3).unwrap().copy_from_slice(b"ABC"); + Ok(3) + }, + || Ok(fgetxattr_query(&source, name)?), + )?; fsetxattr(staged, name, &value, XattrFlags::empty())?; Ok(()) }); @@ -227,29 +229,29 @@ fn a_growing_resource_fork_refuses_publication_and_keeps_original() { #[test] fn attribute_reads_require_a_complete_value_and_stable_size() { for bytes in [&b""[..], &b"\0\xffB"[..]] { - let value = read_xattr_value(bytes.len(), |buffer| { - if !buffer.is_empty() { + let value = read_xattr_value( + bytes.len(), + |buffer| { buffer.get_mut(..bytes.len()).unwrap().copy_from_slice(bytes); - } - Ok(bytes.len()) - }) + Ok(bytes.len()) + }, + || Ok(bytes.len()), + ) .unwrap(); assert_eq!(value, bytes); } for (before, read_len, after) in [(3, 2, 3), (3, 3, 4), (0, 1, 1), (3, 2, 2)] { - let result = read_xattr_value(before, |buffer| Ok(if buffer.is_empty() { after } else { read_len })); + let result = read_xattr_value(before, |_| Ok(read_len), || Ok(after)); assert!(result.unwrap_err().to_string().contains("changed size")); } - let result = read_xattr_value(3, |buffer| { - if buffer.is_empty() { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "size recheck refused")) } else { Ok(3) } - }); + let result = read_xattr_value(3, |_| Ok(3), || Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "size recheck refused"))); assert_eq!(result.unwrap_err().kind(), std::io::ErrorKind::PermissionDenied); } #[cfg(target_os = "macos")] #[test] fn oversized_extended_attributes_refuse_the_save_without_replacing_the_target() { - use rustix::fs::{XattrFlags, getxattr, setxattr}; + use rustix::fs::{XattrFlags, getxattr_query, setxattr}; let dir = test_dir("xattrs-limit"); let target = dir.join("out.pdf"); std::fs::write(&target, b"OLD").unwrap(); @@ -267,7 +269,7 @@ fn oversized_extended_attributes_refuse_the_save_without_replacing_the_target() assert!(e.to_string().contains("Save As"), "{e}"); assert!(!flushed, "metadata failure must stop before flush and rename"); assert_eq!(std::fs::read(&target).unwrap(), b"OLD"); - let size = getxattr(&target, "com.apple.ResourceFork", &mut [] as &mut [u8]).unwrap(); + let size = getxattr_query(&target, "com.apple.ResourceFork").unwrap(); assert_eq!(size, fork.len()); assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 1); std::fs::remove_dir_all(dir).unwrap(); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 77132de42f..fdac3dbdbe 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -75,9 +75,8 @@ objc2-app-kit = { version = "0.3", default-features = false, features = ["std", objc2-foundation = { version = "0.3", default-features = false, features = ["std", "NSArray", "NSGeometry", "NSBundle", "NSURL", "NSString", "NSError"] } block2 = "0.6" plist = "1.10" -# A plain fsync where a volume refuses the full flush `sync_all` asks for (SMB, FUSE); already in -# the tree through pdfcraft-cli. -rustix = { version = "1", features = ["fs"] } +# Query and read Finder association attributes without following symbolic links. +rustix = { version = "1.1.6", features = ["fs"] } [target.'cfg(windows)'.dependencies] # Auto interface language: the Windows display language, without starting a process. diff --git a/crates/ui-egui/src/platform/file_watch.rs b/crates/ui-egui/src/platform/file_watch.rs index 85ec4d0f5b..539b98361a 100644 --- a/crates/ui-egui/src/platform/file_watch.rs +++ b/crates/ui-egui/src/platform/file_watch.rs @@ -623,14 +623,18 @@ mod tests { #[test] fn a_large_binding_does_not_block_a_document_save() { let (_dir, path) = TempDir::new("large-binding-save", &fixture(2)); - xattr::set(&path, "com.apple.LaunchServices.OpenWith", &vec![b'x'; 65537]).unwrap(); + let binding = vec![b'x'; 65537]; + xattr::set(&path, "com.apple.LaunchServices.OpenWith", &binding).unwrap(); xattr::set(&path, "com.apple.quarantine", b"0082;1234;Preview;").unwrap(); let mut h = harness(&path); let app = h.state_mut(); app.views[0].select_pages(&[0]); app.apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 }); assert!(app.save_active(SaveTarget::InPlace), "the document save uses upstream preservation limits"); - assert_eq!(xattr::get(&path, "com.apple.LaunchServices.OpenWith").unwrap().unwrap().len(), 65537); + let mut saved = vec![0; binding.len()]; + let count = rustix::fs::lgetxattr(&path, "com.apple.LaunchServices.OpenWith", saved.as_mut_slice()).unwrap(); + assert_eq!(count, binding.len()); + assert_eq!(saved, binding); assert_eq!(xattr::get(&path, "com.apple.quarantine").unwrap(), Some(b"0082;1234;Preview;".to_vec())); } diff --git a/crates/ui-egui/src/platform/macos_association.rs b/crates/ui-egui/src/platform/macos_association.rs index 2116481bd5..d81f058000 100644 --- a/crates/ui-egui/src/platform/macos_association.rs +++ b/crates/ui-egui/src/platform/macos_association.rs @@ -41,26 +41,30 @@ impl OpeningMetadata { /// Read a last-component-symlink-safe attribute with a strict allocation cap. fn read_attribute(path: &Path, name: &str) -> io::Result>> { - read_attribute_with(|buffer| rustix::fs::lgetxattr(path, name, buffer)) + read_attribute_with(|| rustix::fs::lgetxattr_query(path, name), |buffer| rustix::fs::lgetxattr(path, name, buffer)) } /// Probe the length before allocating. A concurrently grown value fails safely; unreadable /// attributes or values beyond the parser cap skip recovery without affecting open/save. -fn read_attribute_with(mut get: impl FnMut(&mut [u8]) -> rustix::io::Result) -> io::Result>> { +fn read_attribute_with( + query_size: impl FnOnce() -> rustix::io::Result, + read: impl FnOnce(&mut [u8]) -> rustix::io::Result, +) -> io::Result>> { use rustix::io::Errno; let unreadable = |error| match error { Errno::RANGE => Err(io::Error::new(io::ErrorKind::InvalidData, "opening metadata exceeds 64 KiB or changed while reading")), _ => Ok(None), }; - let len = match get(&mut []) { + let len = match query_size() { Ok(len) if len <= MAX_ATTRIBUTE => len, Ok(_) => return unreadable(Errno::RANGE), Err(e) => return unreadable(e), }; - let mut bytes = vec![0; len]; - match get(bytes.as_mut_slice()) { - Ok(len) if len <= bytes.len() => { - bytes.truncate(len); + // Read through a nonempty buffer so a zero-length attribute can still be checked for growth. + let mut bytes = vec![0; len.max(1)]; + match read(bytes.as_mut_slice()) { + Ok(count) if count <= len => { + bytes.truncate(count); Ok(Some(bytes.into_boxed_slice().into_vec())) } Ok(_) => unreadable(Errno::RANGE), @@ -285,12 +289,16 @@ mod tests { let dir = directory(); let file = dir.join("out.pdf"); std::fs::write(&file, b"original PDF").unwrap(); - xattr::set(&file, OPEN_WITH, &vec![b'x'; MAX_ATTRIBUTE + 1]).unwrap(); + let binding = vec![b'x'; MAX_ATTRIBUTE + 1]; + xattr::set(&file, OPEN_WITH, &binding).unwrap(); xattr::set(&file, QUARANTINE, b"0082;1234;Preview;").unwrap(); assert!(OpeningMetadata::read(&file).is_err()); crate::editing::write_atomically(file.to_str().unwrap(), b"new PDF").unwrap(); assert_eq!(std::fs::read(&file).unwrap(), b"new PDF"); - assert_eq!(xattr::get(&file, OPEN_WITH).unwrap().unwrap().len(), MAX_ATTRIBUTE + 1); + let mut saved = vec![0; binding.len()]; + let count = rustix::fs::lgetxattr(&file, OPEN_WITH, saved.as_mut_slice()).unwrap(); + assert_eq!(count, binding.len()); + assert_eq!(saved, binding); assert_eq!(xattr::get(&file, QUARANTINE).unwrap(), Some(b"0082;1234;Preview;".to_vec())); std::fs::remove_dir_all(dir).unwrap(); } @@ -393,30 +401,29 @@ mod tests { /// and skips recovery if the value grows between the probe and read. #[test] fn attribute_probes_bound_allocation_and_detect_growth() { - let mut calls = 0; - let small = read_attribute_with(|out| { - calls += 1; - if calls == 1 { - assert!(out.is_empty()); - return Ok(3); - } - assert_eq!(out.len(), 3); - out.copy_from_slice(b"abc"); - Ok(3) - }) + let small = read_attribute_with( + || Ok(3), + |out| { + assert_eq!(out.len(), 3); + out.copy_from_slice(b"abc"); + Ok(3) + }, + ) .unwrap() .unwrap(); assert_eq!(small, b"abc"); assert_eq!(small.capacity(), 3); - assert!(read_attribute_with(|_| Ok(MAX_ATTRIBUTE + 1)).is_err()); - let mut calls = 0; - assert!( - read_attribute_with(|_| { - calls += 1; - if calls == 1 { Ok(3) } else { Err(rustix::io::Errno::RANGE) } - }) - .is_err() + assert!(read_attribute_with(|| Ok(MAX_ATTRIBUTE + 1), |_| panic!("oversized attribute must not be read")).is_err()); + assert!(read_attribute_with(|| Ok(3), |_| Err(rustix::io::Errno::RANGE)).is_err()); + let empty = read_attribute_with( + || Ok(0), + |out| { + assert_eq!(out.len(), 1); + Ok(0) + }, ); + assert_eq!(empty.unwrap(), Some(Vec::new())); + assert!(read_attribute_with(|| Ok(0), |_| Ok(1)).is_err()); } /// Volumes may expose no readable attributes with errors beyond ENOTSUP. @@ -424,7 +431,7 @@ mod tests { fn unreadable_attributes_are_best_effort() { use rustix::io::Errno; for errno in [Errno::NOSYS, Errno::INVAL, Errno::PERM, Errno::ACCESS, Errno::IO] { - assert_eq!(read_attribute_with(|_| Err(errno)).unwrap(), None); + assert_eq!(read_attribute_with(|| Err(errno), |_| panic!("unreadable attribute must not be read")).unwrap(), None); } } @@ -433,8 +440,8 @@ mod tests { fn unsupported_attribute_volumes_are_distinct_from_read_failures() { use rustix::io::Errno; for errno in [Errno::NOATTR, Errno::NOTSUP, Errno::OPNOTSUPP] { - assert_eq!(read_attribute_with(|_| Err(errno)).unwrap(), None, "{errno}"); + assert_eq!(read_attribute_with(|| Err(errno), |_| panic!("unsupported attribute must not be read")).unwrap(), None, "{errno}"); } - assert!(read_attribute_with(|_| Err(Errno::RANGE)).is_err()); + assert!(read_attribute_with(|| Err(Errno::RANGE), |_| panic!("failed size query must not read the attribute")).is_err()); } }