Merge pull request #1649 from storytold/fix/1620-save-fallback

Part of #1620: a save falls back when a folder refuses the temporary file or the rename
This commit is contained in:
BFlat 2026-10-13 13:29:57 +07:00 committed by GitHub
commit d443cb031c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 171 additions and 6 deletions

View File

@ -2,6 +2,10 @@
//! or interrupted write never leaves a damaged file behind (the old one stays as it was).
//! [`write_new_with`] creates a file the same way without ever replacing one.
//!
//! Where a folder refuses the temporary file or the rename (some network shares, a file another
//! program holds open), the new content is first written and flushed elsewhere and only then copied
//! into the file itself; if that copy fails, the temporary file stays and the error names it.
//!
//! On macOS `File::sync_all` is a full flush (`F_FULLFSYNC`), which SMB shares refuse with
//! `ENOTSUP` while a plain `fsync` works: [`sync_file`] falls back to the plain one there (#1620).
@ -16,18 +20,88 @@ pub fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> {
}
/// Write `path` atomically with `fill`, which writes the content into the temporary file. When
/// anything fails (`fill`, flushing, the rename) the temporary file is removed and `path` is left
/// untouched.
/// `fill` or flushing fails the temporary file is removed and `path` is left untouched. When the
/// folder refuses the temporary file or the rename, the content is copied into `path` instead (see
/// the module docs).
pub fn write_atomic_with(path: &Path, fill: impl FnOnce(&mut File) -> io::Result<()>) -> io::Result<()> {
write_atomic_stepping(path, fill, create_new, |tmp, target| std::fs::rename(tmp, target))
}
fn create_new(path: &Path) -> io::Result<File> {
OpenOptions::new().write(true).create_new(true).open(path)
}
/// [`write_atomic_with`], with `create` making the temporary file beside the target and `rename`
/// moving it over the target.
fn write_atomic_stepping(
path: &Path,
fill: impl FnOnce(&mut File) -> io::Result<()>,
create: impl FnOnce(&Path) -> io::Result<File>,
rename: impl FnOnce(&Path, &Path) -> io::Result<()>,
) -> io::Result<()> {
let target = resolve_link(path)?;
let tmp = temp_path(&target)?;
let mut f = OpenOptions::new().write(true).create_new(true).open(&tmp)?;
let beside = temp_path(&target)?;
// `refused`: why the folder takes no temporary file (it is in the system's folder then).
let (tmp, mut f, refused) = match create(&beside) {
Ok(f) => (beside, f, None),
// A folder that isn't there, a full volume: nothing can be saved there.
Err(e) if matches!(e.kind(), io::ErrorKind::NotFound | io::ErrorKind::StorageFull | io::ErrorKind::QuotaExceeded) => return Err(e),
Err(e) => {
let Some(tmp) = beside.file_name().map(|n| std::env::temp_dir().join(n)) else { return Err(e) };
let Ok(f) = create_private(&tmp) else { return Err(e) };
(tmp, f, Some(e))
}
};
let filled = fill(&mut f).and_then(|()| sync_file(&f));
drop(f);
filled.and_then(|()| keep_permissions(&target, &tmp)).and_then(|()| std::fs::rename(&tmp, &target)).inspect_err(|_| {
if let Err(e) = filled {
// Best effort: the temporary file is all there is to clean up.
let _ = std::fs::remove_file(&tmp);
})
return Err(e);
}
let refused = match refused {
Some(e) => e,
None => {
// Best effort: a volume without Unix permissions (SMB, FAT) refuses them, and has none
// to keep.
let _ = keep_permissions(&target, &tmp);
let Err(e) = rename(&tmp, &target) else { return Ok(()) };
e
}
};
write_over(&tmp, &target, refused)
}
/// The content of `tmp`, which is whole and flushed, copied into `target` itself, and `tmp`
/// removed: for a folder that `refused` the atomic replacement. When `target` can't be opened for
/// writing it is untouched: `tmp` is removed and `refused` returned. When the copy fails `target`
/// is damaged: `tmp` stays and the error says where it is.
fn write_over(tmp: &Path, target: &Path, refused: io::Error) -> io::Result<()> {
let opened = File::open(tmp).and_then(|from| Ok((from, OpenOptions::new().write(true).create(true).truncate(false).open(target)?)));
let Ok((mut from, mut to)) = opened else {
// Best effort: the temporary file is all there is to clean up.
let _ = std::fs::remove_file(tmp);
return Err(refused);
};
let copied = to.set_len(0).and_then(|()| io::copy(&mut from, &mut to)).and_then(|_| sync_file(&to));
drop((from, to));
match copied {
Ok(()) => {
// Best effort: the file is saved, so the copy is no longer needed.
let _ = std::fs::remove_file(tmp);
Ok(())
}
Err(e) => Err(io::Error::new(e.kind(), format!("{e} (the new content is kept in {})", tmp.display()))),
}
}
/// A new file for its owner only (Unix): the system's temporary folder is shared.
fn create_private(path: &Path) -> io::Result<File> {
let mut options = OpenOptions::new();
options.write(true).create_new(true);
#[cfg(unix)]
std::os::unix::fs::OpenOptionsExt::mode(&mut options, 0o600);
options.open(path)
}
/// Create `path` with the content `fill` writes, never replacing a file or a link of that name. The
@ -227,6 +301,97 @@ mod tests {
let _ = std::fs::remove_dir_all(&d);
}
fn denied<T>() -> io::Result<T> {
Err(io::Error::new(io::ErrorKind::PermissionDenied, "refused by the share"))
}
/// A folder that refuses the rename over the file (#1620: some network shares): the new content,
/// whole in the temporary file, is copied into the file itself. A new file is created.
#[test]
fn a_refused_rename_copies_the_content_into_the_file() {
let d = dir("no-rename");
let path = d.join("doc.vectorcraft");
std::fs::write(&path, b"original, and longer").unwrap();
write_atomic_stepping(&path, |f| f.write_all(b"new"), create_new, |_, _| denied()).unwrap();
assert_eq!(std::fs::read(&path).unwrap(), b"new");
write_atomic_stepping(&d.join("other.vectorcraft"), |f| f.write_all(b"other"), create_new, |_, _| denied()).unwrap();
assert_eq!(std::fs::read(d.join("other.vectorcraft")).unwrap(), b"other");
assert_eq!(leftovers(&d, "doc.vectorcraft"), ["other.vectorcraft"], "the temporary files are removed");
// A failed write never gets as far as the file.
let r = write_atomic_stepping(&path, |f| f.write_all(b"half").and(Err(io::Error::other("disk full"))), create_new, |_, _| denied());
assert_eq!(r.unwrap_err().to_string(), "disk full");
assert_eq!(std::fs::read(&path).unwrap(), b"new");
let _ = std::fs::remove_dir_all(&d);
}
/// A refused rename where the file can't be written either (here: a folder of that name): the
/// rename's error, nothing changed and no temporary file left.
#[test]
fn a_refused_rename_over_a_file_that_cannot_be_written_changes_nothing() {
let d = dir("no-rename-no-write");
let path = d.join("doc.vectorcraft");
std::fs::create_dir(&path).unwrap();
let r = write_atomic_stepping(&path, |f| f.write_all(b"new"), create_new, |_, _| denied());
assert_eq!(r.unwrap_err().to_string(), "refused by the share");
assert!(path.is_dir() && leftovers(&d, "doc.vectorcraft").is_empty());
let _ = std::fs::remove_dir_all(&d);
}
/// A refused rename and a copy into the file that fails (here: its flush): the file is damaged,
/// so the temporary file with the whole new content stays and the error names it.
#[test]
fn a_failed_copy_into_the_file_keeps_the_temporary_file() {
let d = dir("no-rename-copy-fails");
let path = d.join("doc.vectorcraft");
std::fs::write(&path, b"original").unwrap();
let r = write_atomic_stepping(
&path,
|f| f.write_all(b"new"),
create_new,
|_, _| {
// From here on flushing fails (also the plain `fsync` macOS falls back to).
REFUSE_FULL_SYNC.with(|c| c.set(true));
PLAIN_FSYNC_FAILS.with(|c| c.set(Some(5)));
denied()
},
);
REFUSE_FULL_SYNC.with(|c| c.set(false));
PLAIN_FSYNC_FAILS.with(|c| c.set(None));
let message = r.unwrap_err().to_string();
let left = leftovers(&d, "doc.vectorcraft");
let [tmp] = left.as_slice() else { panic!("one temporary file: {left:?}") };
assert!(message.contains(tmp.as_str()), "{message}");
assert_eq!(std::fs::read(d.join(tmp)).unwrap(), b"new");
let _ = std::fs::remove_dir_all(&d);
}
/// A folder that takes no temporary file (#1620: a share that refuses hidden or new names) but
/// lets the file be written: the content goes through the system's temporary folder, and no
/// rename is tried. A folder that isn't there is still an error.
#[test]
fn a_folder_that_refuses_the_temporary_file_still_saves() {
let d = dir("no-temp");
let path = d.join("doc.vectorcraft");
std::fs::write(&path, b"original, and longer").unwrap();
let renamed = std::cell::Cell::new(false);
let rename = |_: &Path, _: &Path| {
renamed.set(true);
Ok(())
};
write_atomic_stepping(&path, |f| f.write_all(b"new"), |_| denied(), rename).unwrap();
assert_eq!(std::fs::read(&path).unwrap(), b"new");
assert!(!renamed.get() && leftovers(&d, "doc.vectorcraft").is_empty());
// The file can't be written either (a folder of that name): the folder's error.
let folder = d.join("folder");
std::fs::create_dir(&folder).unwrap();
let r = write_atomic_stepping(&folder, |f| f.write_all(b"new"), |_| denied(), |_, _| Ok(()));
assert_eq!(r.unwrap_err().to_string(), "refused by the share");
let r = write_atomic_stepping(&path, |f| f.write_all(b"lost"), |_| Err(io::Error::from(io::ErrorKind::NotFound)), |_, _| Ok(()));
assert_eq!(r.map_err(|e| e.kind()), Err(io::ErrorKind::NotFound));
assert_eq!(std::fs::read(&path).unwrap(), b"new");
let _ = std::fs::remove_dir_all(&d);
}
#[test]
fn write_new_never_replaces_what_is_there() {
let d = dir("new");