diff --git a/README.md b/README.md index 479b462e..92dbf9d4 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ rows are the enforced release-mode bounds plus one dated observation. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rdocx | 1,092,256 compressed bytes, 6,498,484 member bytes, 36 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | +| Crates.io archive: rdocx | 1,092,383 compressed bytes, 6,499,347 member bytes, 36 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | | Large-document layout throughput | minimum 250 pages/s, observed 31,019.1 pages/s | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 one-page paragraphs with deterministic fonts | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | pages per wall-clock second | 2026-09-19 | | Large-document layout peak allocation | maximum 64 MiB, observed 29.03 MiB | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 one-page paragraphs with deterministic fonts | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | peak live allocation | 2026-09-19 | | Large-document PDF throughput | minimum 1,000 pages/s, observed 60,058.0 pages/s | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 deterministic layout pages | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | pages per wall-clock second | 2026-09-19 | diff --git a/crates/oxml-opc/README.md b/crates/oxml-opc/README.md index 20a3cc78..ea6f87bf 100644 --- a/crates/oxml-opc/README.md +++ b/crates/oxml-opc/README.md @@ -18,7 +18,7 @@ The archive row is regenerated from the package that carries this README. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: oxml-opc | 92,122 compressed bytes, 355,510 member bytes, 12 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `oxml-opc` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-19 | +| Crates.io archive: oxml-opc | 96,724 compressed bytes, 373,059 member bytes, 12 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `oxml-opc` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-19 | ## Use it when diff --git a/crates/oxml-opc/src/lib.rs b/crates/oxml-opc/src/lib.rs index ddde09c6..81ea7732 100644 --- a/crates/oxml-opc/src/lib.rs +++ b/crates/oxml-opc/src/lib.rs @@ -17,7 +17,7 @@ mod signature; pub use content_types::{ContentType, ContentTypes}; pub use error::OpcError; -pub use package::{OpcPackage, PackagePart, PackageReadLimits}; +pub use package::{OpcPackage, PackagePart, PackageReadLimits, write_atomic_file}; pub use relationship::{Relationship, Relationships}; #[cfg(feature = "digital-signatures")] pub use signature::{ diff --git a/crates/oxml-opc/src/package.rs b/crates/oxml-opc/src/package.rs index 8927ee03..7be5e8a7 100644 --- a/crates/oxml-opc/src/package.rs +++ b/crates/oxml-opc/src/package.rs @@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet}; use std::io::{Read, Seek, SeekFrom, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use zip::ZipWriter; use zip::read::ZipArchive; @@ -248,10 +248,21 @@ impl OpcPackage { } /// Save the OPC package to a file path. + /// + /// The complete ZIP is built in memory and published through + /// [`write_atomic_file`], so a failed save leaves an existing file as it + /// was. pub fn save>(&self, path: P) -> Result<()> { - let serialized = self.serialize()?; - let file = std::fs::File::create(path)?; - Self::write_serialized(file, serialized) + let mut output = std::io::Cursor::new(Vec::new()); + self.write_to(&mut output)?; + write_atomic_file( + path.as_ref(), + output.get_ref(), + "oxml-opc", + "invalid package file name", + "could not allocate package save staging file", + )?; + Ok(()) } /// Write the OPC package to any writer. @@ -708,6 +719,199 @@ impl Default for OpcPackage { } } +/// Replace the file at `path` with `bytes` through a synced sibling file. +/// +/// The bytes are written to `.{file name}.{staging_tag}-{process id}-{attempt}.tmp` +/// beside the file being replaced, synced, and renamed over it, so readers see +/// the old file or the new one and never a partial write. A file name too long +/// for that staging name is shortened in it. Any failure removes the staged +/// file and leaves the destination as it was. A symbolic link at `path` is +/// followed and kept, and the file it names is replaced. On Unix the +/// replacement keeps the permission bits of the file it replaces, from the +/// moment the staged file is created. The owner, extended attributes, and +/// other hard links of that file are not carried over. A file this process +/// cannot open for writing is refused and left as it was. +/// +/// A path that names a device or a FIFO, such as `/dev/null`, has no file to +/// replace. It takes the bytes in place, as a plain write would. +/// +/// `staging_tag` is a fragment of a file name and must not contain a path +/// separator. +/// +/// # Errors +/// +/// Returns `invalid_name_message` when `path` names no file, the error of +/// opening the file to replace for writing, such as +/// [`PermissionDenied`](std::io::ErrorKind::PermissionDenied) when it is +/// read-only, an +/// [`InvalidInput`](std::io::ErrorKind::InvalidInput) error when `path` goes +/// through more than 40 symbolic links, `exhausted_message` when every staging +/// name is taken, and the I/O error of a failed write, staging, sync, or +/// rename. +pub fn write_atomic_file( + path: &Path, + bytes: &[u8], + staging_tag: &str, + invalid_name_message: &'static str, + exhausted_message: &'static str, +) -> std::io::Result<()> { + // Renaming over a device or a FIFO would put a regular file in its place. + // The kernel follows every link here, including /dev/stdout. + if std::fs::metadata(path).is_ok_and(|metadata| !metadata.is_file() && !metadata.is_dir()) { + return std::fs::write(path, bytes); + } + let path = resolve_symbolic_links(path)?; + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + let file_name = path.file_name().ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidInput, invalid_name_message) + })?; + let existing = std::fs::metadata(&path).ok(); + // A rename needs write access to the directory, not to the file it + // replaces. Opening the file for writing, without truncating it, refuses + // the files the in-place write refused, such as a read-only file or + // another user's file. + if existing.as_ref().is_some_and(std::fs::Metadata::is_file) { + std::fs::OpenOptions::new().write(true).open(&path)?; + } + // Only Unix mode bits carry over. On Windows the staged file keeps its + // default attributes. + let permissions = existing + .filter(|_| cfg!(unix)) + .map(|metadata| metadata.permissions()); + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + // Creating the staged file with the kept mode means it is never more open + // than the file it replaces. The umask can narrow that mode, so it is set + // again once the file exists. + #[cfg(unix)] + if let Some(permissions) = &permissions { + use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + options.mode(permissions.mode()); + } + // wasm targets have no process id, and std panics when asked for one. The + // retry below covers two processes that share a staging name. + let process_id = if cfg!(target_family = "wasm") { + 0 + } else { + std::process::id() + }; + for attempt in 0..128_u8 { + let temporary = parent.join(staging_file_name( + file_name, + &format!(".{staging_tag}-{process_id}-{attempt}.tmp"), + )); + let mut file = match options.open(&temporary) { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(error), + }; + let result = permissions + .clone() + .map_or(Ok(()), |permissions| file.set_permissions(permissions)) + .and_then(|()| file.write_all(bytes)) + .and_then(|()| file.sync_all()); + drop(file); + let result = result.and_then(|()| replace_file(&temporary, &path)); + if result.is_err() { + let _ = std::fs::remove_file(&temporary); + } + return result; + } + Err(std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + exhausted_message, + )) +} + +/// Name a staged file `.{file name}{suffix}`, keeping it within 255 bytes. +/// +/// Common file systems refuse a longer name, so a long file name keeps only +/// the prefix that fits, cut on a character boundary. +fn staging_file_name(file_name: &std::ffi::OsStr, suffix: &str) -> std::ffi::OsString { + let available = 255_usize.saturating_sub(1 + suffix.len()); + let mut name = std::ffi::OsString::from("."); + if file_name.len() <= available { + name.push(file_name); + } else { + let file_name = file_name.to_string_lossy(); + let mut end = available.min(file_name.len()); + while !file_name.is_char_boundary(end) { + end -= 1; + } + name.push(&file_name[..end]); + } + name.push(suffix); + name +} + +/// Follow symbolic links from `path` to the file a save replaces. +/// +/// A relative link target resolves against the directory holding the link, +/// and a dangling link resolves to the missing file it names. +fn resolve_symbolic_links(path: &Path) -> std::io::Result { + let mut resolved = path.to_path_buf(); + // Up to 40 links are followed, the Linux limit, so 41 paths are checked. + // A longer chain is treated as a loop. + for _ in 0..=40 { + match std::fs::symlink_metadata(&resolved) { + Ok(metadata) if metadata.file_type().is_symlink() => { + let target = std::fs::read_link(&resolved)?; + resolved = match resolved.parent() { + Some(parent) => parent.join(target), + None => target, + }; + } + _ => return Ok(resolved), + } + } + Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "too many levels of symbolic links", + )) +} + +#[cfg(not(target_os = "windows"))] +fn replace_file(source: &Path, destination: &Path) -> std::io::Result<()> { + std::fs::rename(source, destination) +} + +#[cfg(target_os = "windows")] +fn replace_file(source: &Path, destination: &Path) -> std::io::Result<()> { + use std::os::windows::ffi::OsStrExt; + + const MOVEFILE_REPLACE_EXISTING: u32 = 0x1; + const MOVEFILE_WRITE_THROUGH: u32 = 0x8; + + #[link(name = "kernel32")] + unsafe extern "system" { + fn MoveFileExW( + existing_file_name: *const u16, + new_file_name: *const u16, + flags: u32, + ) -> i32; + } + + let source: Vec = source.as_os_str().encode_wide().chain(Some(0)).collect(); + let destination: Vec = destination + .as_os_str() + .encode_wide() + .chain(Some(0)) + .collect(); + // SAFETY: both path buffers are NUL-terminated and remain alive for the call. + let replaced = unsafe { + MoveFileExW( + source.as_ptr(), + destination.as_ptr(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + }; + if replaced == 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } +} + #[cfg(test)] fn docx_package() -> OpcPackage { let mut package = OpcPackage::with_main_part( @@ -963,6 +1167,246 @@ mod tests { std::fs::remove_file(destination).unwrap(); } + fn save_test_directory(label: &str) -> PathBuf { + let directory = + std::env::temp_dir().join(format!("oxml-opc-save-{label}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&directory); + std::fs::create_dir_all(&directory).unwrap(); + directory + } + + fn staging_files(directory: &Path) -> Vec { + std::fs::read_dir(directory) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| path.extension().is_some_and(|extension| extension == "tmp")) + .collect() + } + + fn package_bytes(package: &OpcPackage) -> Vec { + let mut output = std::io::Cursor::new(Vec::new()); + package.write_to(&mut output).unwrap(); + output.into_inner() + } + + #[cfg(unix)] + #[test] + fn save_replaces_an_existing_file_by_rename_and_keeps_its_mode() { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + + let directory = save_test_directory("rename"); + let destination = directory.join("existing.docx"); + std::fs::write(&destination, b"previous bytes").unwrap(); + std::fs::set_permissions(&destination, std::fs::Permissions::from_mode(0o600)).unwrap(); + let previous_inode = std::fs::metadata(&destination).unwrap().ino(); + + let package = docx_package(); + package.save(&destination).unwrap(); + + // A new inode shows the file was replaced by rename, not rewritten in place. + let metadata = std::fs::metadata(&destination).unwrap(); + assert_ne!(metadata.ino(), previous_inode); + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + assert_eq!( + std::fs::read(&destination).unwrap(), + package_bytes(&package) + ); + assert!(staging_files(&directory).is_empty()); + std::fs::remove_dir_all(directory).unwrap(); + } + + #[cfg(unix)] + #[test] + fn save_through_a_symbolic_link_replaces_the_file_it_names() { + use std::os::unix::fs::PermissionsExt as _; + + let directory = save_test_directory("symlink"); + let links = directory.join("links"); + std::fs::create_dir(&links).unwrap(); + let target = directory.join("target.docx"); + std::fs::write(&target, b"previous bytes").unwrap(); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o640)).unwrap(); + let link = links.join("link.docx"); + std::os::unix::fs::symlink("../target.docx", &link).unwrap(); + + let package = docx_package(); + package.save(&link).unwrap(); + + assert!( + std::fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!( + std::fs::read_link(&link).unwrap(), + Path::new("../target.docx") + ); + assert_eq!(std::fs::read(&target).unwrap(), package_bytes(&package)); + assert_eq!( + std::fs::metadata(&target).unwrap().permissions().mode() & 0o7777, + 0o640 + ); + + // A dangling link names the file the save creates. + std::fs::remove_file(&target).unwrap(); + package.save(&link).unwrap(); + assert!( + std::fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!(std::fs::read(&target).unwrap(), package_bytes(&package)); + + // A chain of 40 links is followed, and one more link is refused as a loop. + std::fs::remove_file(&target).unwrap(); + let mut head = target.clone(); + for index in 0..40 { + let chained = links.join(format!("chain-{index}.docx")); + std::os::unix::fs::symlink(&head, &chained).unwrap(); + head = chained; + } + package.save(&head).unwrap(); + assert_eq!(std::fs::read(&target).unwrap(), package_bytes(&package)); + let too_long = links.join("chain-40.docx"); + std::os::unix::fs::symlink(&head, &too_long).unwrap(); + assert!(package.save(&too_long).is_err()); + assert!(staging_files(&directory).is_empty()); + assert!(staging_files(&links).is_empty()); + std::fs::remove_dir_all(directory).unwrap(); + } + + #[test] + fn failed_save_keeps_the_destination_and_leaves_no_staging_file() { + let directory = save_test_directory("failure"); + + // Renaming the staged file over a directory fails after staging. + let occupied = directory.join("directory.docx"); + std::fs::create_dir(&occupied).unwrap(); + std::fs::write(occupied.join("inner.xml"), b"inner bytes").unwrap(); + assert!(docx_package().save(&occupied).is_err()); + assert_eq!( + std::fs::read(occupied.join("inner.xml")).unwrap(), + b"inner bytes" + ); + assert!(staging_files(&directory).is_empty()); + + // Exhausted staging names fail before the destination is touched. + let destination = directory.join("existing.docx"); + std::fs::write(&destination, b"previous bytes").unwrap(); + for attempt in 0..128_u8 { + let staging = directory.join(format!( + ".existing.docx.oxml-opc-{}-{attempt}.tmp", + std::process::id() + )); + std::fs::write(staging, b"occupied").unwrap(); + } + let error = docx_package().save(&destination).unwrap_err(); + assert!( + error + .to_string() + .contains("could not allocate package save staging file"), + "{error}" + ); + assert_eq!(std::fs::read(&destination).unwrap(), b"previous bytes"); + assert_eq!(staging_files(&directory).len(), 128); + std::fs::remove_dir_all(directory).unwrap(); + } + + #[cfg(unix)] + #[test] + fn save_to_a_device_or_fifo_writes_in_place_and_keeps_the_node() { + use std::os::unix::fs::FileTypeExt as _; + + let package = docx_package(); + package.save("/dev/null").unwrap(); + assert!( + std::fs::metadata("/dev/null") + .unwrap() + .file_type() + .is_char_device() + ); + + let directory = save_test_directory("fifo"); + let fifo = directory.join("pipe.docx"); + let status = std::process::Command::new("mkfifo") + .arg(&fifo) + .status() + .unwrap(); + assert!(status.success()); + let reader = { + let fifo = fifo.clone(); + std::thread::spawn(move || std::fs::read(fifo).unwrap()) + }; + package.save(&fifo).unwrap(); + // Checked before the join, so a save that replaced the FIFO fails here + // instead of leaving the test waiting on a reader that never gets data. + assert!( + std::fs::symlink_metadata(&fifo) + .unwrap() + .file_type() + .is_fifo() + ); + assert_eq!(reader.join().unwrap(), package_bytes(&package)); + assert!(staging_files(&directory).is_empty()); + std::fs::remove_dir_all(directory).unwrap(); + } + + #[cfg(unix)] + #[test] + fn save_refuses_a_read_only_file_and_leaves_it_as_it_was() { + use std::os::unix::fs::PermissionsExt as _; + + let directory = save_test_directory("read-only"); + let destination = directory.join("protected.docx"); + std::fs::write(&destination, b"previous bytes").unwrap(); + // 0o464 leaves write bits for the group but none for the owner, who + // cannot write the file either. + for mode in [0o444, 0o464] { + std::fs::set_permissions(&destination, std::fs::Permissions::from_mode(mode)).unwrap(); + // Root writes a read-only file in place, so it may replace it too. + if std::fs::OpenOptions::new() + .write(true) + .open(&destination) + .is_ok() + { + continue; + } + let error = docx_package().save(&destination).unwrap_err(); + assert!( + matches!(&error, OpcError::Io(error) if error.kind() == std::io::ErrorKind::PermissionDenied), + "{mode:o}: {error}" + ); + assert_eq!(std::fs::read(&destination).unwrap(), b"previous bytes"); + } + assert!(staging_files(&directory).is_empty()); + std::fs::remove_dir_all(directory).unwrap(); + } + + // Unix only: on Windows the full path passes MAX_PATH, and the MoveFileExW + // rename takes it without the `\\?\` prefix that std adds for long paths. + #[cfg(unix)] + #[test] + fn save_shortens_the_staging_name_of_a_long_file_name() { + let directory = save_test_directory("long-name"); + // A name of 255 bytes in 235 characters fills the limit of ext4 and APFS. + // The shortened staging name ends inside the run of three-byte + // characters, so it has to cut on a character boundary. + let destination = + directory.join(format!("{}{}.docx", "a".repeat(220), "\u{6587}".repeat(10))); + let package = docx_package(); + package.save(&destination).unwrap(); + std::fs::write(&destination, b"previous bytes").unwrap(); + package.save(&destination).unwrap(); + assert_eq!( + std::fs::read(&destination).unwrap(), + package_bytes(&package) + ); + assert!(staging_files(&directory).is_empty()); + std::fs::remove_dir_all(directory).unwrap(); + } + #[test] fn bounded_reader_rejects_too_many_entries() { let archive = package_zip(&[ diff --git a/crates/rdocx/src/document.rs b/crates/rdocx/src/document.rs index 5c4b2b46..1cdccb7f 100644 --- a/crates/rdocx/src/document.rs +++ b/crates/rdocx/src/document.rs @@ -14,7 +14,7 @@ use oxml_chart::{CT_ChartSpace, ChartData, ChartKind}; use oxml_core::app_properties::AppProperties; use oxml_opc::content_types; use oxml_opc::relationship::rel_types; -use oxml_opc::{OpcPackage, PackageReadLimits}; +use oxml_opc::{OpcPackage, PackageReadLimits, write_atomic_file}; use oxml_sml::Workbook; use quick_xml::Writer; use quick_xml::XmlVersion; @@ -11625,6 +11625,7 @@ impl Document { write_atomic_file( path.as_ref(), &bytes, + "rdocx", "invalid Word package file name", "could not allocate Word package save staging file", )?; @@ -11988,6 +11989,11 @@ impl Document { } /// Save the document to a file path. + /// + /// The package is staged in a synced sibling file and renamed over `path` + /// through [`oxml_opc::write_atomic_file`], so a failed save leaves an + /// existing file as it was. A symbolic link at `path` is kept and the file + /// it names is replaced, and on Unix that file keeps its permission bits. pub fn save>(&mut self, path: P) -> Result<()> { let mut candidate = self.clone_for_staging(); candidate.prepare_staged_output()?; @@ -12019,6 +12025,7 @@ impl Document { write_atomic_file( path.as_ref(), &bytes, + "rdocx", "invalid file name", "could not allocate encrypted-save staging file", )?; @@ -22974,86 +22981,6 @@ impl Document { } } -pub(crate) fn write_atomic_file( - path: &Path, - bytes: &[u8], - invalid_name_message: &'static str, - exhausted_message: &'static str, -) -> std::io::Result<()> { - let parent = path.parent().unwrap_or_else(|| Path::new(".")); - let file_name = path.file_name().ok_or_else(|| { - std::io::Error::new(std::io::ErrorKind::InvalidInput, invalid_name_message) - })?; - for attempt in 0..128_u8 { - let mut temporary_name = std::ffi::OsString::from("."); - temporary_name.push(file_name); - temporary_name.push(format!(".rdocx-{}-{attempt}.tmp", std::process::id())); - let temporary = parent.join(temporary_name); - let mut file = match std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&temporary) - { - Ok(file) => file, - Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, - Err(error) => return Err(error), - }; - let result = std::io::Write::write_all(&mut file, bytes).and_then(|()| file.sync_all()); - drop(file); - let result = result.and_then(|()| replace_file(&temporary, path)); - if result.is_err() { - let _ = std::fs::remove_file(&temporary); - } - return result; - } - Err(std::io::Error::new( - std::io::ErrorKind::AlreadyExists, - exhausted_message, - )) -} - -#[cfg(not(target_os = "windows"))] -pub(crate) fn replace_file(source: &Path, destination: &Path) -> std::io::Result<()> { - std::fs::rename(source, destination) -} - -#[cfg(target_os = "windows")] -pub(crate) fn replace_file(source: &Path, destination: &Path) -> std::io::Result<()> { - use std::os::windows::ffi::OsStrExt; - - const MOVEFILE_REPLACE_EXISTING: u32 = 0x1; - const MOVEFILE_WRITE_THROUGH: u32 = 0x8; - - #[link(name = "kernel32")] - unsafe extern "system" { - fn MoveFileExW( - existing_file_name: *const u16, - new_file_name: *const u16, - flags: u32, - ) -> i32; - } - - let source: Vec = source.as_os_str().encode_wide().chain(Some(0)).collect(); - let destination: Vec = destination - .as_os_str() - .encode_wide() - .chain(Some(0)) - .collect(); - // SAFETY: both path buffers are NUL-terminated and remain alive for the call. - let replaced = unsafe { - MoveFileExW( - source.as_ptr(), - destination.as_ptr(), - MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, - ) - }; - if replaced == 0 { - Err(std::io::Error::last_os_error()) - } else { - Ok(()) - } -} - impl Default for Document { fn default() -> Self { Self::new() diff --git a/crates/rdocx/src/epub.rs b/crates/rdocx/src/epub.rs index 0105065c..313d6732 100644 --- a/crates/rdocx/src/epub.rs +++ b/crates/rdocx/src/epub.rs @@ -63,9 +63,10 @@ impl Document { /// Serialize and atomically save EPUB, returning lossy-conversion diagnostics. pub fn save_epub>(&self, path: P) -> Result> { let result = self.to_epub_bytes()?; - crate::document::write_atomic_file( + oxml_opc::write_atomic_file( path.as_ref(), &result.bytes, + "rdocx", "invalid EPUB file name", "could not allocate EPUB-save staging file", )?; diff --git a/crates/rdocx/src/flat_opc.rs b/crates/rdocx/src/flat_opc.rs index 630ac92d..648a1e72 100644 --- a/crates/rdocx/src/flat_opc.rs +++ b/crates/rdocx/src/flat_opc.rs @@ -9,13 +9,13 @@ use base64::engine::general_purpose::STANDARD as BASE64; use oxml_core::xml::validate_strict_xml_1_0; use oxml_opc::content_types::{self, ContentTypes}; use oxml_opc::relationship::{Relationships, rel_types}; -use oxml_opc::{OpcPackage, PackageReadLimits}; +use oxml_opc::{OpcPackage, PackageReadLimits, write_atomic_file}; use quick_xml::events::{BytesEnd, BytesStart, BytesText, Event}; use quick_xml::name::{Namespace, ResolveResult}; use quick_xml::reader::NsReader; use quick_xml::{Writer, XmlVersion}; -use crate::document::{Document, write_atomic_file}; +use crate::document::Document; use crate::error::{Error, Result}; const PACKAGE_NAMESPACE: &[u8] = b"http://schemas.microsoft.com/office/2006/xmlPackage"; @@ -99,6 +99,7 @@ impl Document { write_atomic_file( path.as_ref(), &bytes, + "rdocx", "invalid Flat OPC file name", "could not allocate Flat OPC save staging file", )?; diff --git a/crates/rdocx/src/html.rs b/crates/rdocx/src/html.rs index 6ec00fb4..831b3b51 100644 --- a/crates/rdocx/src/html.rs +++ b/crates/rdocx/src/html.rs @@ -552,9 +552,10 @@ impl Document { pub fn save_mhtml>(&self, path: P) -> Result> { let result = self.to_mhtml_bytes()?; - crate::document::write_atomic_file( + oxml_opc::write_atomic_file( path.as_ref(), &result.bytes, + "rdocx", "MHTML output path has no file name", "could not allocate an MHTML temporary file", )?; diff --git a/crates/rdocx/src/odt.rs b/crates/rdocx/src/odt.rs index c40d9657..5d4aad82 100644 --- a/crates/rdocx/src/odt.rs +++ b/crates/rdocx/src/odt.rs @@ -138,9 +138,10 @@ impl Document { /// Serialize and save ODT to a path, returning lossy-conversion diagnostics. pub fn save_odt>(&self, path: P) -> Result> { let result = self.to_odt_bytes()?; - crate::document::write_atomic_file( + oxml_opc::write_atomic_file( path.as_ref(), &result.bytes, + "rdocx", "invalid file name", "could not allocate ODT-save staging file", )?; diff --git a/crates/rdocx/src/rtf.rs b/crates/rdocx/src/rtf.rs index 615a470a..f33f4061 100644 --- a/crates/rdocx/src/rtf.rs +++ b/crates/rdocx/src/rtf.rs @@ -88,9 +88,10 @@ impl Document { /// Serialize and save RTF to a path, returning lossy-conversion diagnostics. pub fn save_rtf>(&self, path: P) -> Result> { let result = self.to_rtf_bytes()?; - crate::document::write_atomic_file( + oxml_opc::write_atomic_file( path.as_ref(), &result.bytes, + "rdocx", "invalid file name", "could not allocate RTF-save staging file", )?; diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index d143b3c7..f5715a01 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -13480,6 +13480,80 @@ fn unused_fixed_prefix_declarations_do_not_reject_safe_raw_replay() { assert_eq!(reopened.paragraph(1).unwrap().text(), "changed"); } +#[cfg(unix)] +#[test] +fn path_saves_replace_the_file_by_rename_and_keep_links_and_permissions() { + use std::fs::{self, Permissions}; + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + use std::path::Path; + + let directory = std::env::temp_dir().join(format!("rdocx-atomic-save-{}", std::process::id())); + let _ = fs::remove_dir_all(&directory); + let links = directory.join("links"); + fs::create_dir_all(&links).unwrap(); + let staging_files = |directory: &Path| { + fs::read_dir(directory) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| path.extension().is_some_and(|extension| extension == "tmp")) + .collect::>() + }; + let mode = |path: &Path| fs::metadata(path).unwrap().permissions().mode() & 0o7777; + let mut document = Document::new(); + document.add_paragraph("replacement"); + + // The plain save replaces an existing file by rename and keeps its mode. + let destination = directory.join("existing.docx"); + fs::write(&destination, b"previous bytes").unwrap(); + fs::set_permissions(&destination, Permissions::from_mode(0o600)).unwrap(); + let previous_inode = fs::metadata(&destination).unwrap().ino(); + document.save(&destination).unwrap(); + assert_ne!(fs::metadata(&destination).unwrap().ino(), previous_inode); + assert_eq!(mode(&destination), 0o600); + assert_eq!( + fs::read(&destination).unwrap(), + document.to_bytes().unwrap() + ); + + // A save through a symbolic link replaces the file it names and keeps the link. + let target = directory.join("linked.docx"); + fs::write(&target, b"previous bytes").unwrap(); + fs::set_permissions(&target, Permissions::from_mode(0o640)).unwrap(); + let link = links.join("link.docx"); + std::os::unix::fs::symlink("../linked.docx", &link).unwrap(); + document.save(&link).unwrap(); + assert!( + fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!(fs::read_link(&link).unwrap(), Path::new("../linked.docx")); + assert_eq!(fs::read(&target).unwrap(), document.to_bytes().unwrap()); + assert_eq!(mode(&target), 0o640); + + // The savers that already staged their output now keep the mode too. + let flat = directory.join("existing.xml"); + fs::write(&flat, b"previous bytes").unwrap(); + fs::set_permissions(&flat, Permissions::from_mode(0o600)).unwrap(); + document.save_flat_opc(&flat).unwrap(); + assert_eq!( + fs::read(&flat).unwrap(), + document.to_flat_opc_bytes().unwrap() + ); + assert_eq!(mode(&flat), 0o600); + + // A save that cannot replace its destination leaves it and no staged file. + let occupied = directory.join("directory.docx"); + fs::create_dir(&occupied).unwrap(); + assert!(document.save(&occupied).is_err()); + assert!(occupied.is_dir()); + + assert!(staging_files(&directory).is_empty()); + assert!(staging_files(&links).is_empty()); + fs::remove_dir_all(directory).unwrap(); +} + #[test] fn unused_root_default_namespace_allows_atomic_save() { let task_namespace = "http://schemas.microsoft.com/office/tasks/2019/documenttasks"; diff --git a/crates/rpptx/README.md b/crates/rpptx/README.md index 34eb7caa..017b0a81 100644 --- a/crates/rpptx/README.md +++ b/crates/rpptx/README.md @@ -22,7 +22,7 @@ presentation, notes, handout, PDF, and animation outputs. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rpptx | 407,658 compressed bytes, 2,122,094 member bytes, 16 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rpptx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | +| Crates.io archive: rpptx | 407,138 compressed bytes, 2,120,988 member bytes, 16 members | 0.12.1 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rpptx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | ## Use it when diff --git a/crates/rpptx/src/lib.rs b/crates/rpptx/src/lib.rs index 362154d5..a71f229d 100644 --- a/crates/rpptx/src/lib.rs +++ b/crates/rpptx/src/lib.rs @@ -64,7 +64,7 @@ use oxml_opc::relationship::{Relationship, rel_types}; pub use oxml_opc::{ CoveredRelationship, SignatureIssue, SignatureReport, SignerCertificateIdentity, }; -use oxml_opc::{OpcError, OpcPackage, Relationships}; +use oxml_opc::{OpcError, OpcPackage, Relationships, write_atomic_file}; #[cfg(feature = "render")] pub use rpptx_layout::timeline::{ EvaluatedFrameState, EvaluatedMediaState, MediaPlaybackPhase, TimelinePosition, @@ -780,92 +780,6 @@ fn register_content_type( } } -#[cfg(all(feature = "agile-encryption", not(target_arch = "wasm32")))] -fn write_atomic_file(path: &Path, bytes: &[u8]) -> std::io::Result<()> { - use std::io::Write as _; - - let parent = path.parent().unwrap_or_else(|| Path::new(".")); - let file_name = path.file_name().ok_or_else(|| { - std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid file name") - })?; - for attempt in 0..128_u8 { - let mut temporary_name = std::ffi::OsString::from("."); - temporary_name.push(file_name); - temporary_name.push(format!(".rpptx-{}-{attempt}.tmp", std::process::id())); - let temporary = parent.join(temporary_name); - let mut file = match std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&temporary) - { - Ok(file) => file, - Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, - Err(error) => return Err(error), - }; - let result = file.write_all(bytes).and_then(|()| file.sync_all()); - drop(file); - let result = result.and_then(|()| replace_file(&temporary, path)); - if result.is_err() { - let _ = std::fs::remove_file(&temporary); - } - return result; - } - Err(std::io::Error::new( - std::io::ErrorKind::AlreadyExists, - "could not allocate encrypted-save staging file", - )) -} - -#[cfg(all( - feature = "agile-encryption", - not(target_arch = "wasm32"), - not(target_os = "windows") -))] -fn replace_file(source: &Path, destination: &Path) -> std::io::Result<()> { - std::fs::rename(source, destination) -} - -#[cfg(all( - feature = "agile-encryption", - not(target_arch = "wasm32"), - target_os = "windows" -))] -fn replace_file(source: &Path, destination: &Path) -> std::io::Result<()> { - use std::os::windows::ffi::OsStrExt; - - const MOVEFILE_REPLACE_EXISTING: u32 = 0x1; - const MOVEFILE_WRITE_THROUGH: u32 = 0x8; - - #[link(name = "kernel32")] - unsafe extern "system" { - fn MoveFileExW( - existing_file_name: *const u16, - new_file_name: *const u16, - flags: u32, - ) -> i32; - } - - let source: Vec = source.as_os_str().encode_wide().chain(Some(0)).collect(); - let destination: Vec = destination - .as_os_str() - .encode_wide() - .chain(Some(0)) - .collect(); - // SAFETY: both path buffers are NUL-terminated and remain alive for the call. - let replaced = unsafe { - MoveFileExW( - source.as_ptr(), - destination.as_ptr(), - MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, - ) - }; - if replaced == 0 { - Err(std::io::Error::last_os_error()) - } else { - Ok(()) - } -} - impl Presentation { /// Creates an empty 16:9 presentation from the bundled standard template. #[cfg(feature = "default-template")] @@ -1513,13 +1427,25 @@ impl Presentation { } /// Saves the deterministic package bytes to a `.pptx` path. + /// + /// The bytes are staged in a synced sibling file and renamed over `path` + /// through [`oxml_opc::write_atomic_file`], so a failed save leaves an + /// existing file as it was. A symbolic link at `path` is kept and the file + /// it names is replaced, and on Unix that file keeps its permission bits. pub fn save>(&self, path: P) -> Result<()> { debug_assert!( self.validate().is_empty(), "invalid presentation at path save boundary: {:?}", self.validate() ); - std::fs::write(path, self.to_bytes()?).map_err(OpcError::from)?; + write_atomic_file( + path.as_ref(), + &self.to_bytes()?, + "rpptx", + "invalid PowerPoint package file name", + "could not allocate PowerPoint package save staging file", + ) + .map_err(OpcError::from)?; Ok(()) } @@ -1527,7 +1453,14 @@ impl Presentation { #[cfg(all(feature = "agile-encryption", not(target_arch = "wasm32")))] pub fn save_encrypted>(&self, path: P, password: &str) -> Result<()> { let bytes = self.to_encrypted_bytes(password)?; - write_atomic_file(path.as_ref(), &bytes).map_err(OpcError::from)?; + write_atomic_file( + path.as_ref(), + &bytes, + "rpptx", + "invalid file name", + "could not allocate encrypted-save staging file", + ) + .map_err(OpcError::from)?; Ok(()) } @@ -1537,12 +1470,21 @@ impl Presentation { } /// Saves an output copy with the selected modern package class. + /// + /// The file is replaced the way [`Presentation::save`] replaces it. pub fn save_as_package_class>( &self, path: P, class: PresentationPackageClass, ) -> Result<()> { - std::fs::write(path, self.to_bytes_as(class)?).map_err(OpcError::from)?; + write_atomic_file( + path.as_ref(), + &self.to_bytes_as(class)?, + "rpptx", + "invalid PowerPoint package file name", + "could not allocate PowerPoint package save staging file", + ) + .map_err(OpcError::from)?; Ok(()) } diff --git a/crates/rpptx/src/odp.rs b/crates/rpptx/src/odp.rs index cd943eae..fba45e4f 100644 --- a/crates/rpptx/src/odp.rs +++ b/crates/rpptx/src/odp.rs @@ -106,7 +106,14 @@ impl Presentation { /// Atomically saves deterministic ODP and returns ordered diagnostics. pub fn save_odp>(&self, path: P) -> Result> { let result = self.to_odp_bytes()?; - write_atomic(path.as_ref(), &result.bytes).map_err(OpcError::from)?; + write_atomic_file( + path.as_ref(), + &result.bytes, + "rpptx-odp", + "invalid ODP file name", + "could not allocate ODP save staging file", + ) + .map_err(OpcError::from)?; Ok(result.diagnostics) } } @@ -1480,79 +1487,3 @@ fn write_entry( .write_all(bytes) .map_err(|error| odp_error(Some(name), 0, format!("cannot write ODP entry: {error}"))) } - -fn write_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> { - let parent = path.parent().unwrap_or_else(|| Path::new(".")); - let file_name = path.file_name().ok_or_else(|| { - std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid ODP file name") - })?; - for attempt in 0..128_u8 { - let temporary = parent.join(format!( - ".{}.rpptx-odp-{}-{attempt}.tmp", - file_name.to_string_lossy(), - std::process::id() - )); - let mut file = match std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&temporary) - { - Ok(file) => file, - Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, - Err(error) => return Err(error), - }; - let result = file.write_all(bytes).and_then(|()| file.sync_all()); - drop(file); - let result = result.and_then(|()| replace_atomic(&temporary, path)); - if result.is_err() { - let _ = std::fs::remove_file(&temporary); - } - return result; - } - Err(std::io::Error::new( - std::io::ErrorKind::AlreadyExists, - "could not allocate ODP save staging file", - )) -} - -#[cfg(not(target_os = "windows"))] -fn replace_atomic(source: &Path, destination: &Path) -> std::io::Result<()> { - std::fs::rename(source, destination) -} - -#[cfg(target_os = "windows")] -fn replace_atomic(source: &Path, destination: &Path) -> std::io::Result<()> { - use std::os::windows::ffi::OsStrExt; - - const MOVEFILE_REPLACE_EXISTING: u32 = 0x1; - const MOVEFILE_WRITE_THROUGH: u32 = 0x8; - - #[link(name = "kernel32")] - unsafe extern "system" { - fn MoveFileExW( - existing_file_name: *const u16, - new_file_name: *const u16, - flags: u32, - ) -> i32; - } - - let source: Vec = source.as_os_str().encode_wide().chain(Some(0)).collect(); - let destination: Vec = destination - .as_os_str() - .encode_wide() - .chain(Some(0)) - .collect(); - // SAFETY: both buffers are NUL-terminated and remain alive for this call. - let replaced = unsafe { - MoveFileExW( - source.as_ptr(), - destination.as_ptr(), - MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, - ) - }; - if replaced == 0 { - Err(std::io::Error::last_os_error()) - } else { - Ok(()) - } -} diff --git a/crates/rpptx/tests/integration.rs b/crates/rpptx/tests/integration.rs index cbf98946..5624a597 100644 --- a/crates/rpptx/tests/integration.rs +++ b/crates/rpptx/tests/integration.rs @@ -13008,6 +13008,81 @@ fn assert_f223_relationships_equal( } } +#[cfg(unix)] +#[test] +fn path_saves_replace_the_file_by_rename_and_keep_links_and_permissions() { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + + let directory = f222_temp_directory("atomic-save"); + let links = directory.join("links"); + fs::create_dir_all(&links).unwrap(); + let staging_files = |directory: &Path| { + fs::read_dir(directory) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| path.extension().is_some_and(|extension| extension == "tmp")) + .collect::>() + }; + let mode = |path: &Path| fs::metadata(path).unwrap().permissions().mode() & 0o7777; + let presentation = f222_source_presentation(); + + // The plain save replaces an existing file by rename and keeps its mode. + let destination = directory.join("existing.pptx"); + fs::write(&destination, b"previous bytes").unwrap(); + fs::set_permissions(&destination, fs::Permissions::from_mode(0o600)).unwrap(); + let previous_inode = fs::metadata(&destination).unwrap().ino(); + presentation.save(&destination).unwrap(); + assert_ne!(fs::metadata(&destination).unwrap().ino(), previous_inode); + assert_eq!(mode(&destination), 0o600); + assert_eq!( + fs::read(&destination).unwrap(), + presentation.to_bytes().unwrap() + ); + + // A save through a symbolic link replaces the file it names and keeps the link. + let target = directory.join("linked.ppsx"); + fs::write(&target, b"previous bytes").unwrap(); + fs::set_permissions(&target, fs::Permissions::from_mode(0o640)).unwrap(); + let link = links.join("link.ppsx"); + std::os::unix::fs::symlink("../linked.ppsx", &link).unwrap(); + presentation.save_as_show(&link).unwrap(); + assert!( + fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!(fs::read_link(&link).unwrap(), Path::new("../linked.ppsx")); + assert_eq!( + fs::read(&target).unwrap(), + presentation + .to_bytes_as(PresentationPackageClass::Slideshow) + .unwrap() + ); + assert_eq!(mode(&target), 0o640); + + // The saver that already staged its output now keeps the mode too. + let odp = directory.join("existing.odp"); + fs::write(&odp, b"previous bytes").unwrap(); + fs::set_permissions(&odp, fs::Permissions::from_mode(0o600)).unwrap(); + presentation.save_odp(&odp).unwrap(); + assert_eq!( + fs::read(&odp).unwrap(), + presentation.to_odp_bytes().unwrap().bytes + ); + assert_eq!(mode(&odp), 0o600); + + // A save that cannot replace its destination leaves it and no staged file. + let occupied = directory.join("directory.pptx"); + fs::create_dir(&occupied).unwrap(); + assert!(presentation.save(&occupied).is_err()); + assert!(occupied.is_dir()); + + assert!(staging_files(&directory).is_empty()); + assert!(staging_files(&links).is_empty()); + fs::remove_dir_all(directory).unwrap(); +} + #[test] fn save_as_show_changes_only_the_main_content_type() { let presentation = Presentation::from_bytes(&package_bytes(fixture_package())).unwrap(); diff --git a/scripts/readme_doctests.py b/scripts/readme_doctests.py index e24c643b..efdd06b0 100644 --- a/scripts/readme_doctests.py +++ b/scripts/readme_doctests.py @@ -380,17 +380,17 @@ class ReadmeCase: "oxml-drawing": (161_119, 1_128_372, 24), "oxml-layout": (4_623_324, 9_227_483, 51), "oxml-media": (12_252, 50_992, 6), - "oxml-opc": (92_122, 355_510, 12), + "oxml-opc": (96_724, 373_059, 12), "oxml-pdf": (66_015, 304_432, 14), "oxml-sml": (12_511, 49_803, 6), - "rdocx": (1_092_256, 6_498_484, 36), + "rdocx": (1_092_383, 6_499_347, 36), "rdocx-cli": (33_805, 145_256, 8), "rdocx-html": (15_486, 63_894, 11), "rdocx-layout": (255_752, 1_385_701, 15), "rdocx-opc": (3_655, 9_668, 6), "rdocx-oxml": (367_500, 2_380_047, 32), "rdocx-pdf": (8_111, 26_758, 6), - "rpptx": (407_658, 2_122_094, 16), + "rpptx": (407_138, 2_120_988, 16), "rpptx-chart": (6_648, 21_136, 6), "rpptx-cli": (36_709, 159_585, 8), "rpptx-layout": (79_109, 458_112, 11),