fix(platform): preserve file visibility when saving replaced files (#5001) (#5048)

This commit is contained in:
Yusuf MANTAM authored and GitHub committed 2026-11-10 21:01:27 +01:00
1 parent b10f85262c
commit 1fc9501cf3
4 files changed
+99 -2

No files matched your search

Generated
+1
View File
@@ -4624,6 +4624,7 @@ version = "0.18.1"
dependencies = [
"async-channel",
"futures-lite",
"log",
"rustix 1.1.5",
"zbus",
]
+1
View File
@@ -9,6 +9,7 @@ license.workspace = true
workspace = true
[dependencies]
log = { workspace = true }
[target.'cfg(target_os = "macos")'.dependencies]
# `write_atomically`: a volume that refuses F_FULLFSYNC gets a plain fsync.
+24 -2
View File
@@ -132,7 +132,8 @@ pub fn staging_suffixes() -> impl Iterator<Item = u64> {
/// (`sync_all`); parent-directory durability after a power loss is not promised. On Unix a file
/// that is replaced keeps its permissions and group, so a private file stays private; a new file gets the
/// usual mode for a new file. On Windows a replaced file stays hidden or system if it was, and
/// plain if it was plain. On Windows and macOS a replaced file keeps its creation date, where
/// plain if it was plain, even where a server hides the dot-named staging file (#5001, #4593, #6029, #6119).
/// On Windows and macOS a replaced file keeps its creation date, where
/// the volume stores one.
/// On macOS, accessible extended attributes of an existing regular destination are copied
/// before flushing; a copy failure refuses the save rather than losing metadata (#2658).
@@ -185,7 +186,28 @@ pub fn write_atomically_with(
keep_created(&file, created);
sync_staged(&file, sync_all)?;
}
std::fs::rename(&tmp, path)
let rename_result = std::fs::rename(&tmp, path);
#[cfg(windows)]
if rename_result.is_ok()
&& let Some(keep) = &keep
{
// Restore attributes only if the saved file's hidden or system bits differ from the
// replaced file's (#5001, #4593, #6029, #6119). Samba/NAS servers mark dot-named
// staging files hidden and persist that flag through the rename; on local disks, the
// staging file already has the right KEPT_ATTRIBUTES. Restoring unconditionally would
// overwrite the newly written file's entire attribute word with the old file's,
// incorrectly wiping the archive bit (which backup tools need) or retaining offline/index
// flags. On servers where attributes differ, restoring permissions is a safe trade to keep
// the file visible. When saving a new file (`keep` is None), no baseline exists.
use std::os::windows::fs::MetadataExt as _;
let saved = std::fs::symlink_metadata(path).map(|m| m.file_attributes() & KEPT_ATTRIBUTES);
if saved.is_ok_and(|a| a != (keep.file_attributes() & KEPT_ATTRIBUTES))
&& let Err(e) = std::fs::set_permissions(path, keep.permissions())
{
log::warn!("could not restore replaced file attributes on {}: {e}", path.display());
}
}
rename_result
})();
if result.is_err() {
let _ = std::fs::remove_file(&tmp);
+73
View File
@@ -448,3 +448,76 @@ fn a_replaced_link_keeps_its_own_attributes_not_its_targets() {
assert_ne!(std::fs::metadata(&referent).unwrap().file_attributes() & HIDDEN, 0, "and still hidden");
let _ = std::fs::remove_dir_all(&dir);
}
/// #5001, #4593: a save overwriting an existing file preserves its original attributes even if
/// the server (such as Samba/QNAP NAS hiding dot-files) marks the temporary staging file hidden.
#[cfg(windows)]
#[test]
fn a_replaced_file_restores_attributes_if_server_hides_staging_file() {
use std::os::windows::fs::{MetadataExt as _, OpenOptionsExt as _};
const HIDDEN: u32 = 0x2;
let dir = test_dir("server-hides-staging");
let target = dir.join("plain.pdf");
std::fs::write(&target, b"OLD").unwrap();
assert_eq!(std::fs::metadata(&target).unwrap().file_attributes() & HIDDEN, 0, "target starts visible");
// Create a temporary hidden reference file to obtain permissions with the HIDDEN attribute.
let hidden_ref = dir.join("hidden_ref");
let _ = std::fs::OpenOptions::new().write(true).create_new(true).attributes(HIDDEN).open(&hidden_ref).unwrap();
let hidden_perm = std::fs::metadata(&hidden_ref).unwrap().permissions();
let _ = std::fs::remove_file(&hidden_ref);
for form in [StagingName::TagThenSuffix, StagingName::SuffixThenTag] {
write_atomically_with(&target, b"NEW", form, [77], |_file| {
// Find the staging file in dir (the file that is not target):
for entry in std::fs::read_dir(&dir)? {
let path = entry?.path();
if path != target {
// Mark the staging file as HIDDEN on disk (simulating Samba hiding dot-files).
std::fs::set_permissions(&path, hidden_perm.clone())?;
assert_ne!(std::fs::metadata(&path)?.file_attributes() & HIDDEN, 0, "staging file was marked hidden");
}
}
Ok(())
})
.unwrap();
assert_eq!(
std::fs::metadata(&target).unwrap().file_attributes() & HIDDEN,
0,
"the saved file remains visible despite server hiding the staging file"
);
assert_eq!(std::fs::read(&target).unwrap(), b"NEW");
}
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
#[cfg(windows)]
fn a_replaced_local_file_keeps_archive_bit_intact() {
use std::os::windows::fs::MetadataExt as _;
let mut dir = std::env::temp_dir();
dir.push(format!("pdfcraft-platform-test-archive-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let target = dir.join("plain.pdf");
std::fs::write(&target, b"OLD").unwrap();
// Clear the archive bit on the replaced file (simulating backup tools like robocopy /M or attrib -A).
const ARCHIVE: u32 = 0x20;
let status = std::process::Command::new("attrib").arg("-A").arg(&target).status().unwrap();
assert!(status.success(), "attrib -A succeeded");
let old_attrs = std::fs::metadata(&target).unwrap().file_attributes();
assert_eq!(old_attrs & ARCHIVE, 0, "archive bit was cleared on the old file");
write_atomically(&target, b"NEW", StagingName::TagThenSuffix).unwrap();
// The newly written file must have the archive bit set by the system (#5001, #4593, #6029, #6119).
let new_attrs = std::fs::metadata(&target).unwrap().file_attributes();
assert_ne!(new_attrs & ARCHIVE, 0, "the newly saved file has the archive bit set by the file system");
assert_eq!(std::fs::read(&target).unwrap(), b"NEW");
let _ = std::fs::remove_dir_all(&dir);
}