mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-24 01:47:08 -06:00
Saving writes a temporary file and renames it over the target, so everything attached to the old file went with the file that was unlinked. Several of those are security decisions, so a save quietly undid them: a download lost its Mark of the Web, an explicit ACL fell back to whatever the folder hands down. New crates/format/src/atomic/meta.rs, reached from atomic_write through two AtomicFs methods (copy_permissions became copy_metadata; replace_existing is new and defaults to rename, so the failure-injecting test file systems are unchanged): - Extended attributes (#6603): flistxattr/fgetxattr/fsetxattr through safe rustix, per attribute, best effort. macOS Finder tags, com.apple.quarantine (the Mark of the Web's macOS counterpart) and Linux user.* all ride along. Attributes the system owns refuse to be written and are skipped; com.apple.decmpfs is skipped deliberately, since it describes storage the new bytes do not have. - macOS ACL (#7073): a new macOS-only exacl dependency (MIT). fcopyfile(COPYFILE_ACL) is an unsafe fn and fclonefileat(CLONE_ACL) is APFS-only and would mean writing every save as clone-then-truncate; the reasoning is in docs/decisions.md. A file with no ACL is left alone. - Windows (#7094, #7096): ReplaceFileW through winsafe's kernel feature (MIT, no dependencies, a safe wrapper; the workspace forbids writing the FFI call here). One call keeps the destination's DACL, its named streams including Zone.Identifier, and its creation time. - Automation write path (#7095): AuthorizedWorkspace::write copies the replaced file's attributes through the two cap-std descriptors, never a path, so a request path still cannot escape the write root. Permissions there remain #4963; ReplaceFileW is path-based and so is not used on that path. The order inside copy_metadata matters: attributes and ACL first, permission bits last. Writing an attribute needs the destination open for writing, so copying a 0444 mode onto the temporary file first locks you out of your own temporary file. Covered by a regression test. Atomicity is not weakened. The metadata step runs on the temporary file before the swap, never touches bytes, and never fails a save. Windows is the one place the swap itself changed, and a backup file name is passed because it is a correctness requirement, not an option: ReplaceFileW without one documents ERROR_UNABLE_TO_MOVE_REPLACEMENT as leaving the replaced file deleted and the replacement under its temporary name, which is a failed save that loses the user's document. With a backup name the same failure leaves both files alone. ERROR_UNABLE_TO_MOVE_REPLACEMENT_2, the one error that does move things, is unwound by hand; every other error falls back to the plain rename, i.e. to the previous behaviour. THE WINDOWS PATHS ARE UNVERIFIED ON WINDOWS. They were written and reviewed on a Mac. The #[cfg(windows)] tests -- Zone.Identifier, a second named stream, icacls output, creation time -- have never executed. They do compile and pass clippy -D warnings against x86_64-pc-windows-msvc, which is as far as a Mac gets. CI's Windows job is the first real run, and #7094 in particular (Mark of the Web surviving a replace) is the whole point of the change and is the part no one has observed working. Tested on macOS: a real chmod +a entry and a real Finder tag both survive a save (checked with ls -le and xattr, and as unit tests), plus a 64 KiB attribute value, a read-only file, the mode, and the automation path. Still open: Linux POSIX ACLs are not carried over (system.posix_acl_access is not reported by listxattr, so it needs its own step; not part of these issues), and macOS creation time is still reset by a save (that half of #6603 needs setattrlist, which has no safe wrapper here).
This commit is contained in:
skylar taylor-barrick
Generated
+28
-13
@@ -849,7 +849,7 @@ dependencies = [
|
||||
"maybe-owned",
|
||||
"rustix 1.1.5",
|
||||
"rustix-linux-procfs",
|
||||
"windows-sys 0.60.2",
|
||||
"windows-sys 0.61.2",
|
||||
"winx",
|
||||
]
|
||||
|
||||
@@ -1891,7 +1891,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1929,6 +1929,18 @@ dependencies = [
|
||||
"pin-project-lite",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "exacl"
|
||||
version = "0.13.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "8de9e551ced9d700a31dacb8b168129640240fcd7a279a3fa4d59608b84ebcae"
|
||||
dependencies = [
|
||||
"bitflags 2.13.2",
|
||||
"log",
|
||||
"scopeguard",
|
||||
"uuid",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "exr"
|
||||
version = "1.74.2"
|
||||
@@ -3028,7 +3040,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46"
|
||||
dependencies = [
|
||||
"hermit-abi",
|
||||
"libc",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -3690,7 +3702,7 @@ dependencies = [
|
||||
"objc2-foundation 0.3.2",
|
||||
"png 0.18.1",
|
||||
"thiserror 2.0.21",
|
||||
"windows-sys 0.60.2",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -4436,7 +4448,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -4693,6 +4705,7 @@ dependencies = [
|
||||
"photocraft-geom",
|
||||
"photocraft-io",
|
||||
"rmcp",
|
||||
"rustix 1.1.5",
|
||||
"schemars",
|
||||
"serde",
|
||||
"serde_json",
|
||||
@@ -4818,6 +4831,7 @@ version = "0.9.0"
|
||||
dependencies = [
|
||||
"blake3",
|
||||
"crc32fast",
|
||||
"exacl",
|
||||
"flate2",
|
||||
"libc",
|
||||
"photocraft-codecs",
|
||||
@@ -4831,6 +4845,7 @@ dependencies = [
|
||||
"serde",
|
||||
"serde_json",
|
||||
"thiserror 2.0.21",
|
||||
"winsafe",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -5469,7 +5484,7 @@ dependencies = [
|
||||
"once_cell",
|
||||
"socket2",
|
||||
"tracing",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -5988,7 +6003,7 @@ dependencies = [
|
||||
"errno",
|
||||
"libc",
|
||||
"linux-raw-sys 0.12.1",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -6055,7 +6070,7 @@ dependencies = [
|
||||
"security-framework",
|
||||
"security-framework-sys",
|
||||
"webpki-root-certs",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -6693,7 +6708,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"windows-sys 0.60.2",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -6920,10 +6935,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd"
|
||||
dependencies = [
|
||||
"fastrand",
|
||||
"getrandom 0.3.4",
|
||||
"getrandom 0.4.3",
|
||||
"once_cell",
|
||||
"rustix 1.1.5",
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -7323,7 +7338,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e"
|
||||
dependencies = [
|
||||
"memoffset",
|
||||
"tempfile",
|
||||
"windows-sys 0.60.2",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -8083,7 +8098,7 @@ version = "0.1.11"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
|
||||
dependencies = [
|
||||
"windows-sys 0.59.0",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -30,3 +30,7 @@ cap-std = "4"
|
||||
[dev-dependencies]
|
||||
rmcp = { version = "3.5", default-features = false, features = ["client", "server", "macros", "transport-io"] }
|
||||
tokio = { version = "1", features = ["rt-multi-thread", "macros", "io-std", "sync", "net", "io-util", "time"] }
|
||||
|
||||
# Reading back an extended attribute the write path should have preserved (#7095).
|
||||
[target.'cfg(unix)'.dev-dependencies]
|
||||
rustix = { version = "1.1", default-features = false, features = ["std", "fs"] }
|
||||
|
||||
@@ -74,6 +74,13 @@ impl AuthorizedWorkspace {
|
||||
/// as [`photocraft_format::atomic_write`], through the directory capability). On failure
|
||||
/// the previous file is untouched and the temporary file is removed.
|
||||
///
|
||||
/// Replacing a file carries its extended attributes — macOS Finder tags, Linux `user.*` —
|
||||
/// onto the file that takes its place (#7095), through the two open descriptors rather than
|
||||
/// their paths, so nothing here reaches outside the write root to finish a write. The rest
|
||||
/// of what [`photocraft_format::atomic_write`] preserves does not follow: its permission
|
||||
/// step is tracked in #4963, and the Windows `ReplaceFileW` swap is path-based, which this
|
||||
/// path may not use.
|
||||
///
|
||||
/// The parent directory must already exist. `cap-std` performs path
|
||||
/// resolution and file creation relative to the held directory handle, so
|
||||
/// a non-existent final target is supported without ambient path access.
|
||||
@@ -85,10 +92,20 @@ impl AuthorizedWorkspace {
|
||||
let tmp = parent.join(photocraft_format::atomic::temp_name(&leaf));
|
||||
let mut options = OpenOptions::new();
|
||||
options.write(true).create_new(true);
|
||||
let written = root.dir.open_with(&tmp, &options).and_then(|mut file| {
|
||||
let written = root.dir.open_with(&tmp, &options).and_then(|file| {
|
||||
let mut file = file.into_std();
|
||||
file.write_all(bytes)?;
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::fd::AsFd;
|
||||
// Best effort, and only when there is a file being replaced: a save still
|
||||
// succeeds for a document whose Finder tag could not come along.
|
||||
if let Ok(old) = root.dir.open(&relative) {
|
||||
let _ = photocraft_format::atomic::meta::copy_xattrs(old.as_fd(), file.as_fd());
|
||||
}
|
||||
}
|
||||
// Falls back to a plain fsync on shares that refuse a full flush (#1336).
|
||||
photocraft_format::atomic::sync_file(&file.into_std())
|
||||
photocraft_format::atomic::sync_file(&file)
|
||||
});
|
||||
let renamed = written.and_then(|()| photocraft_format::atomic::retry_rename(RenameRetry::platform(), || root.dir.rename(&tmp, &root.dir, &relative)));
|
||||
if let Err(e) = renamed {
|
||||
@@ -572,6 +589,32 @@ mod tests {
|
||||
assert!(temp_files(&inside).is_empty());
|
||||
}
|
||||
|
||||
/// #7095: replacing a file through the automation path keeps the old file's extended
|
||||
/// attributes, the same as a save through `atomic_write` does.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn write_keeps_the_replaced_files_extended_attributes() {
|
||||
#[cfg(target_os = "macos")]
|
||||
const TAG: &str = "com.apple.metadata:_kMDItemUserTags";
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
const TAG: &str = "user.photocraft.test";
|
||||
|
||||
let (inside, _, workspace) = roots("xattr");
|
||||
let path = inside.join("doc.psd");
|
||||
workspace.write("doc.psd", b"original").unwrap();
|
||||
if rustix::fs::setxattr(&path, TAG, b"tagged", rustix::fs::XattrFlags::empty()).is_err() {
|
||||
return; // a file system without extended attributes: nothing to assert
|
||||
}
|
||||
|
||||
workspace.write("doc.psd", b"replacement, longer").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"replacement, longer");
|
||||
let mut buf = [0u8; 64];
|
||||
let len = rustix::fs::getxattr(&path, TAG, buf.as_mut_slice()).expect("the attribute was dropped");
|
||||
assert_eq!(&buf[..len], b"tagged");
|
||||
assert!(temp_files(&inside).is_empty());
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn write_into_read_only_folder_keeps_the_original() {
|
||||
|
||||
@@ -34,5 +34,16 @@ rustix = { version = "1.1", default-features = false, features = ["std", "fs"] }
|
||||
[target.'cfg(target_vendor = "apple")'.dependencies]
|
||||
libc = "0.2"
|
||||
|
||||
# Replacing a file keeps its ACL (#7073). `fcopyfile(COPYFILE_ACL)` is an `unsafe fn` and
|
||||
# `fclonefileat(CLONE_ACL)` is APFS-only; exacl is a safe wrapper over acl_get_file/acl_set_file.
|
||||
[target.'cfg(target_os = "macos")'.dependencies]
|
||||
exacl = "0.13"
|
||||
|
||||
# `ReplaceFileW` is how Windows replaces a file without dropping its DACL, named streams
|
||||
# (Zone.Identifier, the Mark of the Web) and creation time (#7094, #7096). winsafe's `kernel`
|
||||
# feature is a dependency-free safe wrapper; the workspace forbids writing the FFI call here.
|
||||
[target.'cfg(windows)'.dependencies]
|
||||
winsafe = { version = "0.0.29", features = ["kernel"] }
|
||||
|
||||
[dev-dependencies]
|
||||
proptest = { workspace = true }
|
||||
|
||||
+33
-10
@@ -21,8 +21,15 @@
|
||||
//! this module is never reached there; on `wasm32` every call returns the `std` "unsupported"
|
||||
//! error.
|
||||
//!
|
||||
//! Replacing a file also carries its metadata across — permission bits, extended attributes,
|
||||
//! the "downloaded from the internet" mark and the access-control list — so that saving a
|
||||
//! document does not quietly undo a security decision someone made about the file. That part is
|
||||
//! [`meta`], and it is best effort: it never fails a save and never touches the bytes.
|
||||
//!
|
||||
//! [`atomic_write_with`] takes an [`AtomicFs`] so tests can inject failures at each step.
|
||||
|
||||
pub mod meta;
|
||||
|
||||
use std::io::{self, Write};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
@@ -44,9 +51,16 @@ pub trait AtomicFs {
|
||||
fn remove(&self, path: &Path) -> io::Result<()>;
|
||||
/// Flush a directory entry change (Unix); a no-op elsewhere.
|
||||
fn sync_dir(&self, dir: &Path) -> io::Result<()>;
|
||||
/// Copy the destination's permissions onto the temporary file, so replacing a file keeps its
|
||||
/// mode. Best effort.
|
||||
fn copy_permissions(&self, from: &Path, to: &Path) -> io::Result<()>;
|
||||
/// Copy the metadata a replacement should keep — mode, extended attributes, ACL — from the
|
||||
/// file about to be replaced onto the temporary file. Best effort; see [`meta`].
|
||||
fn copy_metadata(&self, from: &Path, to: &Path) -> io::Result<()>;
|
||||
/// Replace the **existing** file `to` with `from`. Everywhere but Windows this is
|
||||
/// [`rename`](Self::rename); on Windows it keeps the destination's DACL, named streams and
|
||||
/// creation time ([`meta::replace_existing`]). The default is `rename`, which is what the
|
||||
/// failure-injecting test file systems want.
|
||||
fn replace_existing(&self, from: &Path, to: &Path) -> io::Result<()> {
|
||||
self.rename(from, to)
|
||||
}
|
||||
}
|
||||
|
||||
/// The real file system.
|
||||
@@ -87,9 +101,11 @@ impl AtomicFs for RealFs {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
fn copy_permissions(&self, from: &Path, to: &Path) -> io::Result<()> {
|
||||
let perms = std::fs::metadata(from)?.permissions();
|
||||
std::fs::set_permissions(to, perms)
|
||||
fn copy_metadata(&self, from: &Path, to: &Path) -> io::Result<()> {
|
||||
meta::copy_metadata(from, to)
|
||||
}
|
||||
fn replace_existing(&self, from: &Path, to: &Path) -> io::Result<()> {
|
||||
meta::replace_existing(from, to)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -190,12 +206,19 @@ pub fn atomic_write_with_progress(fs: &dyn AtomicFs, retry: RenameRetry, path: &
|
||||
}
|
||||
let tmp = tmp.ok_or_else(|| io::Error::new(io::ErrorKind::AlreadyExists, format!("{}: could not create a temporary file", dir.display())))?;
|
||||
|
||||
// Keep the existing file's mode (e.g. 0600) instead of the process default.
|
||||
if target.exists() {
|
||||
let _ = fs.copy_permissions(&target, &tmp);
|
||||
// Keep what the file being replaced carried: its mode (e.g. 0600) instead of the process
|
||||
// default, its extended attributes, and its ACL. Best effort, and before the swap, so the
|
||||
// file the user ends up with never passes through a more permissive state.
|
||||
let replacing = target.exists();
|
||||
if replacing {
|
||||
let _ = fs.copy_metadata(&target, &tmp);
|
||||
}
|
||||
|
||||
if let Err(e) = retry_rename(retry, || fs.rename(&tmp, &target)) {
|
||||
// Replacing an existing file goes through `replace_existing`, which on Windows is the one
|
||||
// call that can keep the destination's DACL and named streams; creating a new one is a
|
||||
// plain rename, with nothing to preserve.
|
||||
let swap = || if replacing { fs.replace_existing(&tmp, &target) } else { fs.rename(&tmp, &target) };
|
||||
if let Err(e) = retry_rename(retry, swap) {
|
||||
let _ = fs.remove(&tmp);
|
||||
return Err(rename_error(e, path));
|
||||
}
|
||||
|
||||
@@ -0,0 +1,273 @@
|
||||
//! What a replacement keeps from the file it replaces.
|
||||
//!
|
||||
//! Saving writes a new file and renames it over the old one, so the user gets a file with the
|
||||
//! right bytes and none of the old file's history: everything the operating system and other
|
||||
//! programs had attached to the file that was just unlinked is gone. Some of it is a security
|
||||
//! decision — *this came from the internet*, *only these people may read it* — so a save that
|
||||
//! drops it quietly weakens the user's protections (#6603, #7073, #7094, #7095, #7096).
|
||||
//!
|
||||
//! | | macOS | Linux | Windows |
|
||||
//! |---|---|---|---|
|
||||
//! | permission bits | [`copy_metadata`] | [`copy_metadata`] | [`copy_metadata`] (read-only flag) |
|
||||
//! | extended attributes (Finder tags, `user.*`) | `copy_xattrs` | `copy_xattrs` | — |
|
||||
//! | "downloaded from the internet" | `com.apple.quarantine`, an xattr | — | `Zone.Identifier`, a named stream kept by [`replace_existing`] |
|
||||
//! | access-control list | `copy_acl` (`exacl`) | not carried over (see below) | DACL, kept by [`replace_existing`] |
|
||||
//! | creation time | not carried over (#6603) | — | kept by [`replace_existing`] |
|
||||
//!
|
||||
//! Everything here except [`replace_existing`] is **best effort and never touches the file's
|
||||
//! bytes**: a save that cannot carry a piece of metadata across is still a save, because a
|
||||
//! document the user can open matters more than a Finder tag. [`replace_existing`] *is* the
|
||||
//! replacement step, so it holds the atomicity guarantee instead: it never leaves the
|
||||
//! destination missing, and it falls back to a plain rename rather than fail a save.
|
||||
//!
|
||||
//! Not carried over, and why: Linux POSIX ACLs live in `system.posix_acl_access`, which
|
||||
//! `listxattr` does not report, so copying the listed attributes misses them; they would need
|
||||
//! their own step. macOS creation time needs `setattrlist`, which has no safe wrapper here.
|
||||
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
|
||||
#[cfg(unix)]
|
||||
use std::os::fd::BorrowedFd;
|
||||
|
||||
/// Largest attribute-name list [`copy_xattrs`] will read. Names are short; a file with a
|
||||
/// megabyte of them is not something a save should spend memory on.
|
||||
#[cfg(unix)]
|
||||
const MAX_XATTR_NAMES: usize = 1 << 20;
|
||||
|
||||
/// Largest single attribute value [`copy_xattrs`] will carry over. Finder tags and `user.*`
|
||||
/// attributes are tens of bytes; a resource fork can be large, and past this it is not worth
|
||||
/// holding a copy of it in memory during a save.
|
||||
#[cfg(unix)]
|
||||
const MAX_XATTR_VALUE: usize = 16 << 20;
|
||||
|
||||
/// First read size for an attribute list or value; grown on `ERANGE`.
|
||||
#[cfg(unix)]
|
||||
const XATTR_CHUNK: usize = 4 << 10;
|
||||
|
||||
/// Copy, best effort, everything [`crate::atomic::atomic_write`] should carry from the file it
|
||||
/// is about to replace (`from`) onto the temporary file that will replace it (`to`).
|
||||
///
|
||||
/// Errors are the caller's to ignore: each piece is attempted on its own, and the first failure
|
||||
/// is returned only so a caller that wants to log one can.
|
||||
pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> {
|
||||
// The permission bits go on last. Writing an attribute needs the destination open for
|
||||
// writing, and replacing a read-only (0444) file would copy that mode onto the temporary
|
||||
// file and then be unable to touch it again.
|
||||
#[cfg(unix)]
|
||||
let attributes = copy_xattrs_at(from, to);
|
||||
#[cfg(not(unix))]
|
||||
let attributes = Ok(());
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
let acl = copy_acl(from, to);
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
let acl = Ok(());
|
||||
|
||||
// A save over a 0600 file must not widen it to whatever the process default is.
|
||||
let permissions = std::fs::metadata(from).and_then(|m| std::fs::set_permissions(to, m.permissions()));
|
||||
|
||||
attributes.and(acl).and(permissions)
|
||||
}
|
||||
|
||||
/// [`copy_metadata`]'s extended-attribute step, by path: open both files and hand the
|
||||
/// descriptors to [`copy_xattrs`].
|
||||
#[cfg(unix)]
|
||||
fn copy_xattrs_at(from: &Path, to: &Path) -> io::Result<()> {
|
||||
use std::os::fd::AsFd;
|
||||
let old = std::fs::File::open(from)?;
|
||||
// Writing an attribute needs write access to the file, not only to its directory.
|
||||
let new = std::fs::OpenOptions::new().write(true).open(to)?;
|
||||
copy_xattrs(old.as_fd(), new.as_fd())
|
||||
}
|
||||
|
||||
/// Copy every extended attribute of `from` onto `to`, best effort, through the two open files
|
||||
/// rather than their paths.
|
||||
///
|
||||
/// Descriptors, not paths, because the automation write path holds capability-checked handles
|
||||
/// and must never reach back out to an ambient path to finish a write (#7095). The same reason
|
||||
/// makes this the right shape for [`copy_metadata`] too.
|
||||
///
|
||||
/// An attribute that cannot be read or cannot be set is skipped: `com.apple.provenance` and
|
||||
/// `com.apple.rootless` on macOS and `security.*` on Linux belong to the system and refuse to
|
||||
/// be written by an ordinary process, and failing a document save over one would be absurd.
|
||||
/// `Err` means the attribute *list* could not be read, so nothing was copied.
|
||||
#[cfg(unix)]
|
||||
pub fn copy_xattrs(from: BorrowedFd<'_>, to: BorrowedFd<'_>) -> io::Result<()> {
|
||||
use rustix::fs::{XattrFlags, fsetxattr};
|
||||
|
||||
for name in list_xattrs(from)?.split(|b| *b == 0) {
|
||||
if name.is_empty() || skip_xattr(name) {
|
||||
continue;
|
||||
}
|
||||
let Some(value) = read_xattr(from, name) else { continue };
|
||||
let _ = fsetxattr(to, name, &value, XattrFlags::empty());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Attributes that describe how the *old* file's bytes are stored rather than anything about
|
||||
/// the document, and so must never be pinned onto a file holding different bytes.
|
||||
#[cfg(unix)]
|
||||
fn skip_xattr(name: &[u8]) -> bool {
|
||||
// `com.apple.decmpfs` marks a file whose data lives compressed inside the attribute itself.
|
||||
// The kernel hides it from a plain `listxattr` and refuses to set it, but a file claiming
|
||||
// compressed storage it does not have would read as garbage, so never try.
|
||||
name == b"com.apple.decmpfs"
|
||||
}
|
||||
|
||||
/// The NUL-separated attribute names of `fd`.
|
||||
#[cfg(unix)]
|
||||
fn list_xattrs(fd: BorrowedFd<'_>) -> io::Result<Vec<u8>> {
|
||||
let mut buf = vec![0u8; XATTR_CHUNK];
|
||||
loop {
|
||||
match rustix::fs::flistxattr(fd, buf.as_mut_slice()) {
|
||||
Ok(len) => {
|
||||
buf.truncate(len.min(buf.len()));
|
||||
return Ok(buf);
|
||||
}
|
||||
Err(e) if e == rustix::io::Errno::RANGE && buf.len() < MAX_XATTR_NAMES => {
|
||||
buf.resize(buf.len().saturating_mul(4).min(MAX_XATTR_NAMES), 0);
|
||||
}
|
||||
Err(e) => return Err(e.into()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The value of one attribute, or `None` if it cannot be read or is larger than
|
||||
/// [`MAX_XATTR_VALUE`]. A missing value is normal: another process may remove an attribute
|
||||
/// between the listing and the read.
|
||||
#[cfg(unix)]
|
||||
fn read_xattr(fd: BorrowedFd<'_>, name: &[u8]) -> Option<Vec<u8>> {
|
||||
let mut buf = vec![0u8; XATTR_CHUNK];
|
||||
loop {
|
||||
match rustix::fs::fgetxattr(fd, name, buf.as_mut_slice()) {
|
||||
Ok(len) => {
|
||||
buf.truncate(len.min(buf.len()));
|
||||
return Some(buf);
|
||||
}
|
||||
Err(e) if e == rustix::io::Errno::RANGE && buf.len() < MAX_XATTR_VALUE => {
|
||||
buf.resize(buf.len().saturating_mul(4).min(MAX_XATTR_VALUE), 0);
|
||||
}
|
||||
Err(_) => return None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Copy an explicit macOS access-control list (`chmod +a`) from `from` onto `to` (#7073).
|
||||
///
|
||||
/// `fcopyfile(COPYFILE_ACL)` is the obvious call and is an `unsafe fn`, which this workspace
|
||||
/// forbids; `fclonefileat` with `CLONE_ACL` is safe but only works on APFS and would mean
|
||||
/// writing every save as a clone-then-truncate. `exacl` is a small, safe wrapper over
|
||||
/// `acl_get_file`/`acl_set_file` that works on every file system that has ACLs at all.
|
||||
///
|
||||
/// An old file with no ACL leaves `to` alone: the new file may have picked up the folder's
|
||||
/// inherited entries, and taking access away is a worse surprise than leaving it as the folder
|
||||
/// says it should be.
|
||||
#[cfg(target_os = "macos")]
|
||||
fn copy_acl(from: &Path, to: &Path) -> io::Result<()> {
|
||||
let entries = exacl::getfacl(from, None)?;
|
||||
if entries.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
exacl::setfacl(&[to], &entries, None)
|
||||
}
|
||||
|
||||
/// Replace the **existing** file `to` with `from`, keeping what the destination owns rather than
|
||||
/// its bytes.
|
||||
///
|
||||
/// Everywhere but Windows this is exactly [`std::fs::rename`], and the metadata is carried by
|
||||
/// [`copy_metadata`] onto `from` beforehand. On Windows it is `ReplaceFileW`, which preserves
|
||||
/// the replaced file's DACL (#7096), its named streams — `Zone.Identifier`, the Mark of the Web
|
||||
/// (#7094) — its creation time, object identifier, compression and encryption, in the one call
|
||||
/// Microsoft documents for the save-through-a-temporary-file pattern. None of those have a safe
|
||||
/// path-based API to copy by hand.
|
||||
pub fn replace_existing(from: &Path, to: &Path) -> io::Result<()> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
windows_replace(from, to)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
std::fs::rename(from, to)
|
||||
}
|
||||
}
|
||||
|
||||
/// `ReplaceFileW`, with the plain rename as the fallback. See [`replace_existing`].
|
||||
///
|
||||
/// **A backup name is not optional.** `ReplaceFile` without one deletes the replaced file before
|
||||
/// the final rename, and documents `ERROR_UNABLE_TO_MOVE_REPLACEMENT`: "the replaced file no
|
||||
/// longer exists and the replacement file exists under its original name" — the user's document
|
||||
/// gone, its replacement left under a hidden temporary name. With a backup name the same failure
|
||||
/// leaves both files where they were. So this always passes one, and deletes it on the way out.
|
||||
///
|
||||
/// The failures and what each leaves behind (from the `ReplaceFileW` reference):
|
||||
///
|
||||
/// - `ERROR_UNABLE_TO_MOVE_REPLACEMENT_2` (1177) is the only one that moves anything: the
|
||||
/// replacement keeps its name but has already inherited the destination's streams and
|
||||
/// attributes, and the destination's old content is at the backup name. Finishing the rename
|
||||
/// by hand completes the save *with* its metadata; if even that fails, the backup goes back to
|
||||
/// the destination so the user's file is where they left it.
|
||||
/// - Every other error leaves the replaced and replacement files under their own names, so a
|
||||
/// plain rename still saves the document — without the metadata, which is where this started.
|
||||
#[cfg(windows)]
|
||||
fn windows_replace(from: &Path, to: &Path) -> io::Result<()> {
|
||||
use winsafe::co::REPLACEFILE;
|
||||
|
||||
/// `ERROR_UNABLE_TO_MOVE_REPLACEMENT_2`: the destination now lives under the backup name.
|
||||
const MOVE_REPLACEMENT_2: i32 = 1177;
|
||||
|
||||
let backup = backup_path(to);
|
||||
let paths = (to.to_str(), from.to_str(), backup.as_deref().and_then(Path::to_str));
|
||||
// `ReplaceFileW` takes `&str`; a path that is not valid Unicode, or one past MAX_PATH, is
|
||||
// the plain rename's job.
|
||||
let (Some(replaced), Some(replacement), Some(backup_str)) = paths else {
|
||||
return std::fs::rename(from, to);
|
||||
};
|
||||
|
||||
let flags = REPLACEFILE::IGNORE_MERGE_ERRORS | REPLACEFILE::IGNORE_ACL_ERRORS;
|
||||
match winsafe::ReplaceFile(replaced, replacement, Some(backup_str), flags) {
|
||||
Ok(()) => {
|
||||
remove_backup(backup.as_deref());
|
||||
Ok(())
|
||||
}
|
||||
Err(e) if e.raw() as i32 == MOVE_REPLACEMENT_2 => match std::fs::rename(from, to) {
|
||||
Ok(()) => {
|
||||
remove_backup(backup.as_deref());
|
||||
Ok(())
|
||||
}
|
||||
Err(rename_failed) => {
|
||||
if let Some(backup) = backup.as_deref() {
|
||||
let _ = std::fs::rename(backup, to);
|
||||
}
|
||||
Err(rename_failed)
|
||||
}
|
||||
},
|
||||
Err(_) => {
|
||||
remove_backup(backup.as_deref());
|
||||
std::fs::rename(from, to)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A unique name beside `to` for `ReplaceFile` to park the replaced file under. It shares the
|
||||
/// temporary-file suffix, so the save's own cleanup and the tests' leftover check cover it.
|
||||
#[cfg(windows)]
|
||||
fn backup_path(to: &Path) -> Option<std::path::PathBuf> {
|
||||
let name = to.file_name()?.to_string_lossy().into_owned();
|
||||
let dir = match to.parent() {
|
||||
Some(d) if !d.as_os_str().is_empty() => d,
|
||||
_ => Path::new("."),
|
||||
};
|
||||
Some(dir.join(super::temp_name(&name)))
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn remove_backup(backup: Option<&Path>) {
|
||||
if let Some(backup) = backup {
|
||||
let _ = std::fs::remove_file(backup);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
@@ -0,0 +1,305 @@
|
||||
//! What a save keeps from the file it replaces.
|
||||
//!
|
||||
//! The Unix tests run for real. The Windows ones exercise `ReplaceFileW` through
|
||||
//! [`crate::atomic::atomic_write`] and can only run on Windows, so they are `#[cfg(windows)]`
|
||||
//! and are covered by the Windows CI job, not by a developer on a Mac.
|
||||
|
||||
use crate::atomic::atomic_write;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
|
||||
struct TempDir(PathBuf);
|
||||
impl TempDir {
|
||||
fn new(tag: &str) -> Self {
|
||||
static N: AtomicU64 = AtomicU64::new(0);
|
||||
let d = std::env::temp_dir().join(format!("pc-meta-{tag}-{}-{}", std::process::id(), N.fetch_add(1, Ordering::Relaxed)));
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
std::fs::create_dir_all(&d).unwrap();
|
||||
Self(d)
|
||||
}
|
||||
fn join(&self, name: &str) -> PathBuf {
|
||||
self.0.join(name)
|
||||
}
|
||||
}
|
||||
impl Drop for TempDir {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::fs::remove_dir_all(&self.0);
|
||||
}
|
||||
}
|
||||
|
||||
/// No temporary or backup file is left beside the saved document.
|
||||
fn assert_no_temp(dir: &Path) {
|
||||
let leftovers: Vec<_> =
|
||||
std::fs::read_dir(dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().into_owned()).filter(|n| n.ends_with(".tmp")).collect();
|
||||
assert!(leftovers.is_empty(), "temporary files left behind: {leftovers:?}");
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------- extended attributes (Unix)
|
||||
|
||||
#[cfg(unix)]
|
||||
mod xattrs {
|
||||
use super::*;
|
||||
|
||||
/// macOS wants a reverse-DNS name; Linux only lets an unprivileged process write `user.*`.
|
||||
#[cfg(target_os = "macos")]
|
||||
const TAG: &str = "com.apple.metadata:_kMDItemUserTags";
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
const TAG: &str = "user.photocraft.test";
|
||||
|
||||
fn set(path: &Path, name: &str, value: &[u8]) -> bool {
|
||||
rustix::fs::setxattr(path, name, value, rustix::fs::XattrFlags::empty()).is_ok()
|
||||
}
|
||||
|
||||
fn get(path: &Path, name: &str) -> Option<Vec<u8>> {
|
||||
let mut buf = vec![0u8; 1 << 20];
|
||||
let len = rustix::fs::getxattr(path, name, buf.as_mut_slice()).ok()?;
|
||||
buf.truncate(len);
|
||||
Some(buf)
|
||||
}
|
||||
|
||||
/// #6603: the attributes of the replaced file — a Finder tag, a `user.*` attribute — are on
|
||||
/// the file the user is left with.
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_its_extended_attributes() {
|
||||
let d = TempDir::new("xattr");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
if !set(&p, TAG, b"tagged") {
|
||||
// tmpfs and some CI file systems have no xattr support; nothing to assert.
|
||||
return;
|
||||
}
|
||||
|
||||
atomic_write(&p, b"second, longer").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(&p).unwrap(), b"second, longer");
|
||||
assert_eq!(get(&p, TAG).as_deref(), Some(&b"tagged"[..]), "the replaced file's attribute was dropped");
|
||||
assert_no_temp(&d.0);
|
||||
}
|
||||
|
||||
/// A large value still comes across (and is not silently truncated).
|
||||
#[test]
|
||||
fn a_large_attribute_value_survives() {
|
||||
let d = TempDir::new("xattr-big");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
// Comfortably past the first read buffer, so the grow-on-ERANGE path runs.
|
||||
let value = vec![0xABu8; 64 << 10];
|
||||
if !set(&p, TAG, &value) {
|
||||
return;
|
||||
}
|
||||
|
||||
atomic_write(&p, b"second").unwrap();
|
||||
|
||||
assert_eq!(get(&p, TAG), Some(value));
|
||||
}
|
||||
|
||||
/// Copying the mode must not get in the way of copying the attributes: a read-only file is
|
||||
/// still writable to us *before* its mode lands on the temporary file, and only then.
|
||||
#[test]
|
||||
fn a_read_only_file_keeps_its_attributes_and_its_mode() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let d = TempDir::new("xattr-ro");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
if !set(&p, TAG, b"tagged") {
|
||||
return;
|
||||
}
|
||||
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o444)).unwrap();
|
||||
|
||||
atomic_write(&p, b"second, longer").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(&p).unwrap(), b"second, longer");
|
||||
assert_eq!(get(&p, TAG).as_deref(), Some(&b"tagged"[..]), "a read-only file lost its attribute");
|
||||
assert_eq!(std::fs::metadata(&p).unwrap().permissions().mode() & 0o777, 0o444);
|
||||
}
|
||||
|
||||
/// Creating a file that was not there sets no attributes and does not fail.
|
||||
#[test]
|
||||
fn creating_a_new_file_needs_no_source() {
|
||||
let d = TempDir::new("xattr-new");
|
||||
let p = d.join("fresh.psd");
|
||||
atomic_write(&p, b"bytes").unwrap();
|
||||
assert_eq!(std::fs::read(&p).unwrap(), b"bytes");
|
||||
assert!(get(&p, TAG).is_none());
|
||||
}
|
||||
|
||||
/// The descriptor-based copy is what the automation write path uses (#7095): it works on two
|
||||
/// open files and never looks at a path.
|
||||
#[test]
|
||||
fn copy_xattrs_works_on_descriptors() {
|
||||
use std::os::fd::AsFd;
|
||||
let d = TempDir::new("xattr-fd");
|
||||
let (from, to) = (d.join("from"), d.join("to"));
|
||||
std::fs::write(&from, b"a").unwrap();
|
||||
std::fs::write(&to, b"b").unwrap();
|
||||
if !set(&from, TAG, b"carried") {
|
||||
return;
|
||||
}
|
||||
|
||||
let old = std::fs::File::open(&from).unwrap();
|
||||
let new = std::fs::OpenOptions::new().write(true).open(&to).unwrap();
|
||||
super::super::copy_xattrs(old.as_fd(), new.as_fd()).unwrap();
|
||||
|
||||
assert_eq!(get(&to, TAG).as_deref(), Some(&b"carried"[..]));
|
||||
assert_eq!(std::fs::read(&to).unwrap(), b"b", "copying attributes must not touch the bytes");
|
||||
}
|
||||
}
|
||||
|
||||
// --------------------------------------------------------------------------- mode (Unix)
|
||||
|
||||
/// The mode was already kept before any of this; it still is.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_its_mode() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let d = TempDir::new("mode");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o600)).unwrap();
|
||||
|
||||
atomic_write(&p, b"second").unwrap();
|
||||
|
||||
assert_eq!(std::fs::metadata(&p).unwrap().permissions().mode() & 0o777, 0o600);
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------------------- ACL (macOS)
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
mod acl {
|
||||
use super::*;
|
||||
|
||||
/// #7073: an ACL entry added with `chmod +a` is still there after a save.
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_its_acl() {
|
||||
let d = TempDir::new("acl");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
|
||||
// The same thing `chmod +a` does: one explicit entry for the user running the test, so
|
||||
// no second account has to exist.
|
||||
let Ok(user) = std::env::var("USER") else { return };
|
||||
let entries = [exacl::AclEntry::allow_user(&user, exacl::Perm::READ | exacl::Perm::WRITE, None)];
|
||||
if exacl::setfacl(&[&p], &entries, None).is_err() {
|
||||
// A file system without ACL support (a RAM disk, some CI volumes).
|
||||
return;
|
||||
}
|
||||
let before = exacl::getfacl(&p, None).unwrap();
|
||||
assert!(!before.is_empty(), "the test could not set an ACL to begin with");
|
||||
|
||||
atomic_write(&p, b"second, longer").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(&p).unwrap(), b"second, longer");
|
||||
assert_eq!(exacl::getfacl(&p, None).unwrap(), before, "the replaced file's ACL was dropped");
|
||||
assert_no_temp(&d.0);
|
||||
}
|
||||
|
||||
/// A file with no ACL keeps having no ACL, rather than gaining an empty one.
|
||||
#[test]
|
||||
fn a_file_without_an_acl_is_left_alone() {
|
||||
let d = TempDir::new("acl-none");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
let before = exacl::getfacl(&p, None).unwrap();
|
||||
|
||||
atomic_write(&p, b"second").unwrap();
|
||||
|
||||
assert_eq!(exacl::getfacl(&p, None).unwrap(), before);
|
||||
}
|
||||
}
|
||||
|
||||
// --------------------------------------------------------------- named streams and ACL (Windows)
|
||||
|
||||
/// Unverified on a Mac: these run only on Windows (CI's Windows job).
|
||||
#[cfg(windows)]
|
||||
mod windows {
|
||||
use super::*;
|
||||
|
||||
/// Open a named stream of `path`, e.g. `Zone.Identifier`.
|
||||
fn stream(path: &Path, name: &str) -> PathBuf {
|
||||
let mut s = path.as_os_str().to_os_string();
|
||||
s.push(":");
|
||||
s.push(name);
|
||||
PathBuf::from(s)
|
||||
}
|
||||
|
||||
/// #7094: a file marked as downloaded from the internet is still marked after a save, so the
|
||||
/// shell and other applications keep warning about it.
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_its_mark_of_the_web() {
|
||||
let d = TempDir::new("motw");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
let motw = b"[ZoneTransfer]\r\nZoneId=3\r\n";
|
||||
std::fs::write(stream(&p, "Zone.Identifier"), motw).unwrap();
|
||||
|
||||
atomic_write(&p, b"second, longer").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(&p).unwrap(), b"second, longer");
|
||||
assert_eq!(std::fs::read(stream(&p, "Zone.Identifier")).unwrap(), motw, "the Mark of the Web was dropped");
|
||||
assert_no_temp(&d.0);
|
||||
}
|
||||
|
||||
/// Any other named stream rides along too, because `ReplaceFileW` keeps the lot.
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_other_named_streams() {
|
||||
let d = TempDir::new("ads");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
std::fs::write(stream(&p, "photocraft.test"), b"side data").unwrap();
|
||||
|
||||
atomic_write(&p, b"second").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(stream(&p, "photocraft.test")).unwrap(), b"side data");
|
||||
}
|
||||
|
||||
/// #7096: an explicit ACL entry survives, instead of the file falling back to whatever the
|
||||
/// folder hands down. Compared as the string `icacls` prints, which is what a user checks.
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_its_explicit_acl() {
|
||||
let d = TempDir::new("dacl");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
|
||||
// Grant the current user an entry that inheritance would not have produced.
|
||||
let user = std::env::var("USERNAME").unwrap_or_default();
|
||||
if user.is_empty() {
|
||||
return;
|
||||
}
|
||||
let granted = std::process::Command::new("icacls").arg(&p).arg("/grant").arg(format!("{user}:(RX)")).output();
|
||||
if !granted.map(|o| o.status.success()).unwrap_or(false) {
|
||||
return;
|
||||
}
|
||||
let before = std::process::Command::new("icacls").arg(&p).output().unwrap().stdout;
|
||||
|
||||
atomic_write(&p, b"second, longer").unwrap();
|
||||
|
||||
let after = std::process::Command::new("icacls").arg(&p).output().unwrap().stdout;
|
||||
assert_eq!(String::from_utf8_lossy(&after), String::from_utf8_lossy(&before), "the explicit ACL was dropped");
|
||||
assert_no_temp(&d.0);
|
||||
}
|
||||
|
||||
/// `ReplaceFileW` also keeps the creation time, which the rename used to reset (#6603).
|
||||
#[test]
|
||||
fn replacing_a_file_keeps_its_creation_time() {
|
||||
let d = TempDir::new("born");
|
||||
let p = d.join("doc.psd");
|
||||
atomic_write(&p, b"first").unwrap();
|
||||
let before = std::fs::metadata(&p).unwrap().created().unwrap();
|
||||
|
||||
std::thread::sleep(std::time::Duration::from_millis(50));
|
||||
atomic_write(&p, b"second").unwrap();
|
||||
|
||||
assert_eq!(std::fs::metadata(&p).unwrap().created().unwrap(), before);
|
||||
}
|
||||
|
||||
/// Creating a file where there was none still works: there is nothing to replace, so the
|
||||
/// write never reaches `ReplaceFileW`.
|
||||
#[test]
|
||||
fn creating_a_new_file_still_works() {
|
||||
let d = TempDir::new("new");
|
||||
let p = d.join("fresh.psd");
|
||||
atomic_write(&p, b"bytes").unwrap();
|
||||
assert_eq!(std::fs::read(&p).unwrap(), b"bytes");
|
||||
assert_no_temp(&d.0);
|
||||
}
|
||||
}
|
||||
@@ -69,8 +69,8 @@ impl AtomicFs for Faulty {
|
||||
fn sync_dir(&self, dir: &Path) -> io::Result<()> {
|
||||
RealFs.sync_dir(dir)
|
||||
}
|
||||
fn copy_permissions(&self, from: &Path, to: &Path) -> io::Result<()> {
|
||||
RealFs.copy_permissions(from, to)
|
||||
fn copy_metadata(&self, from: &Path, to: &Path) -> io::Result<()> {
|
||||
RealFs.copy_metadata(from, to)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5,3 +5,30 @@
|
||||
Smart Object instances carry a document-scoped contents ID separately from their layer ID. Normal duplicates preserve it; New Smart Object via Copy creates a new one. Contents edits update the matching instances together, retaining their individual placement and filters. Copies into another document remap contents IDs as a batch, preserving sharing within the copied layers without joining existing destination objects.
|
||||
|
||||
Native files persist this ID. Legacy embedded objects without an ID remain independent: identical file bytes do not prove that two objects were intended to share edits. Legacy nonempty linked paths and PSD source identifiers retain their explicit sharing relationship. PSD export keys embedded sources by contents identity as well as bytes, so independent equal-content objects stay independent.
|
||||
|
||||
## 2026-10-17: keeping a replaced file's ACL and named streams (#7073, #7094, #7096)
|
||||
|
||||
Saving replaces a file by renaming a new one over it, which drops everything the old file
|
||||
carried. Carrying the security-relevant parts across needs platform calls that have no safe API
|
||||
in `std`, and the workspace forbids `unsafe`, so each platform took a dependency rather than an
|
||||
exception.
|
||||
|
||||
**macOS ACLs: `exacl`, not `fclonefileat`.** The two routes that avoid `unsafe` are a macOS-only
|
||||
`exacl` dependency (a safe wrapper over `acl_get_file`/`acl_set_file`) and `fclonefileat` with
|
||||
the undocumented-in-`rustix` `CLONE_ACL` bit. The clone keeps the ACL, but only on APFS, and it
|
||||
means writing every save as clone-then-truncate-then-write with a fallback for every other file
|
||||
system — restructuring the one write path the whole app's durability rests on, to preserve an
|
||||
ACL. `exacl` is a best-effort step beside the existing permission copy, it works wherever ACLs
|
||||
do, and a failure cannot affect the bytes.
|
||||
|
||||
**Windows: `ReplaceFileW` via `winsafe`, with a backup name.** Copying the `Zone.Identifier`
|
||||
stream by hand with `std` would cover the Mark of the Web but not the DACL, and enumerating every
|
||||
stream needs `FindFirstStreamW`. `ReplaceFileW` is the call Microsoft documents for the
|
||||
save-through-a-temporary-file pattern and preserves the DACL, named streams, creation time and
|
||||
object identifier in one step; `winsafe`'s `kernel` feature wraps it safely and pulls in nothing.
|
||||
|
||||
The backup-file argument is a correctness requirement, not an option: `ReplaceFileW` without one
|
||||
documents `ERROR_UNABLE_TO_MOVE_REPLACEMENT` as leaving the replaced file deleted and the
|
||||
replacement under its temporary name — a failed save that loses the user's document. With a
|
||||
backup name the same failure leaves both files alone, and the one failure that does move things
|
||||
(`ERROR_UNABLE_TO_MOVE_REPLACEMENT_2`) is unwound in `atomic/meta.rs`.
|
||||
|
||||
Reference in New Issue
Block a user