shifathosn
/
pdfcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Security: create save staging files with create_new (follow-up to #164) (#319) 

* Security: create the --root staging file with create_new

Follow-up to #164. write_atomic staged every save at the fixed name
.<name>.pdfcraft-tmp and wrote it with fs::write, which follows a
symbolic link and writes through a hard link. So a link planted there,
in a folder someone else can write to, redirected the bytes to any file
the link pointed at.

The staging file now gets a random 64-bit suffix (std's RandomState,
keyed from the operating system's random source) and is opened with
OpenOptions::create_new, which fails if anything already has the name
(a file, a hard link, a symbolic link even when dangling), on Windows
(CREATE_NEW) as on Unix (O_EXCL). A taken name is skipped, never
opened; Windows reports a folder at the name as "access denied", which
counts as taken when something is there. After 16 taken names the save
fails and nothing is overwritten. A failed write or rename removes only
the file this save created. At most 128 bytes of the target's name go
into the staging name, so it fits every system's name limit.

The #164 tests that planted a sentinel at the old fixed name now also
compare folder listings, and say that the "is a folder" refusal is what
they check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Security: create the desktop app's save staging file with create_new

The same fix as the previous commit, for the desktop app.
editing::write_atomically, which Save, Save As, Export, Print to PDF,
signing and crash recovery all use, staged at
.<name>.pdfcraft-<pid>.tmp and opened it with File::create, which
follows a symbolic link and writes through a hard link. A process id is
easy to guess, so a link planted beside a document in a shared folder
could redirect a save into another file.

The staging file now gets a random 64-bit suffix and is opened with
OpenOptions::create_new; a taken name (on Windows, a folder reports
"access denied") is skipped, never opened, and after 16 the save fails
without overwriting anything. At most 128 bytes of the document's name
go into the staging name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
jcl-bit 2026-10-08 12:19:16 -07:00 committed by GitHub
parent d801407a9f
commit f70c4caac9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 449 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
apps/pdfcraft-cli/tests/run_root.rs
View File

@ -61,11 +61,19 @@ fn script_image_output_is_confined_to_the_root() {
assert!(!base.join("outside/escaped.png").exists() && !base.join("outside/new").exists());
assert_eq!(std::fs::read_to_string(base.join("outside/existing.txt")).unwrap(), "SENTINEL\n");
// "." is the root itself: refused, without staging a temporary file beside the root.
// "." is the root itself: refused, without staging a temporary file beside the root. The
// file at the old fixed staging name and the listing are canaries; staging names are random.
std::fs::write(base.join(".root.pdfcraft-tmp"), "SENTINEL\n").unwrap();
let listing = || {
let mut names: Vec<_> = std::fs::read_dir(&base).unwrap().map(|e| e.unwrap().file_name()).collect();
names.sort();
names
};
let before = listing();
let o = render_to(&base, &base, Some(&root), ".");
assert!(!o.status.success() && String::from_utf8_lossy(&o.stderr).contains("is a folder"), "{}", String::from_utf8_lossy(&o.stderr));
assert_eq!(std::fs::read_to_string(base.join(".root.pdfcraft-tmp")).unwrap(), "SENTINEL\n");
assert_eq!(listing(), before, "nothing was left beside the root");
// A relative path resolves inside the root, not in the working directory.
let o = render_to(&base, &base, Some(&root), "relative.png");

212
crates/automation/src/lib.rs
View File

@ -1720,6 +1720,12 @@ fn child(dir: &Path, name: &str) -> PathBuf {
/// Write via a temporary file in the same directory, then rename over the target.
fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
write_atomic_with(path, bytes, staging_suffixes())
}
/// [`write_atomic`], trying the staging names that `suffixes` give.
fn write_atomic_with(path: &Path, bytes: &[u8], suffixes: impl IntoIterator<Item = u64>) -> Result<()> {
use std::io::Write;
// A folder can't be replaced, and staging beside it could land outside the root: "." names
// the root itself, whose parent isn't ours to write in.
if path.is_dir() {
@ -1728,14 +1734,62 @@ fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
let dir = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(Path::new("."));
let name = path.file_name().ok_or_else(|| failed(format!("{}: not a file path", path.display())))?;
std::fs::create_dir_all(dir).map_err(|e| failed(format!("{}: {e}", dir.display())))?;
let tmp = dir.join(format!(".{}.pdfcraft-tmp", name.to_string_lossy()));
std::fs::write(&tmp, bytes).map_err(|e| failed(format!("{}: {e}", tmp.display())))?;
let (tmp, file) = create_staging(dir, &name.to_string_lossy(), suffixes).map_err(|e| failed(format!("{}: {e}", dir.display())))?;
// Closed at the end of the block, before the rename.
let written = {
let mut file = file;
file.write_all(bytes)
};
if let Err(e) = written {
let _ = std::fs::remove_file(&tmp);
return Err(failed(format!("{}: {e}", tmp.display())));
}
std::fs::rename(&tmp, path).map_err(|e| {
let _ = std::fs::remove_file(&tmp);
failed(format!("{}: {e}", path.display()))
})
}
/// How many staging names [`create_staging`] tries. A random 64-bit name is only taken if someone
/// put a file there on purpose, so running out means refusing, not trying harder.
const STAGING_ATTEMPTS: usize = 16;
/// Create a new, empty staging file in `dir` for the file `name`, one name per suffix. It is
/// opened with `create_new`, which fails if anything already has the name (a file, a hard link,
/// a symbolic link even when dangling, a folder), on Windows as everywhere else; such a name is
/// skipped, never opened, so a file planted at the staging path can't receive or redirect the
/// write.
fn create_staging(dir: &Path, name: &str, suffixes: impl IntoIterator<Item = u64>) -> std::io::Result<(PathBuf, std::fs::File)> {
// At most 128 bytes of the target's name, cut between characters, so the staging name fits
// the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is.
let mut stem = String::new();
for c in name.chars() {
if stem.len() + c.len_utf8() > 128 {
break;
}
stem.push(c);
}
for suffix in suffixes.into_iter().take(STAGING_ATTEMPTS) {
let tmp = dir.join(format!(".{stem}.{suffix:016x}.pdfcraft-tmp"));
match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
Ok(file) => return Ok((tmp, file)),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
// Windows reports a folder at the name as "access denied"; it is taken all the same.
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied && tmp.symlink_metadata().is_ok() => {}
Err(e) => return Err(e),
}
}
Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken"))
}
/// Unpredictable staging-name suffixes, so a name can't be planted in advance. `RandomState` is
/// keyed from the operating system's random source.
fn staging_suffixes() -> impl Iterator<Item = u64> {
use std::hash::BuildHasher;
let state = std::hash::RandomState::new();
(0u64..).map(move |i| state.hash_one(i))
}
#[cfg(test)]
mod tests {
use super::*;
@ -1790,4 +1844,158 @@ mod tests {
assert!(foreign_share(Path::new(r"\\server\other\x.pdf"), shared));
assert!(foreign_share(Path::new(r"\\attacker\docs\x.pdf"), shared));
}
/// A fresh, empty folder for one staging test.
fn staging_dir(test: &str) -> PathBuf {
let dir = std::env::temp_dir().join(format!("pdfcraft-staging-{}-{test}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
dir
}
fn staged(dir: &Path, suffix: u64) -> PathBuf {
dir.join(format!(".out.pdf.{suffix:016x}.pdfcraft-tmp"))
}
fn read(p: &Path) -> String {
std::fs::read_to_string(p).unwrap()
}
/// A symbolic link to a file, where the system allows one (Windows needs Developer Mode or an
/// administrator for it).
fn file_symlink(target: &Path, link: &Path) -> std::io::Result<()> {
#[cfg(unix)]
{
std::os::unix::fs::symlink(target, link)
}
#[cfg(windows)]
{
std::os::windows::fs::symlink_file(target, link)
}
#[cfg(not(any(unix, windows)))]
{
Err(std::io::Error::other(format!("no symbolic links here: {} {}", target.display(), link.display())))
}
}
#[test]
fn staging_never_writes_through_a_file_planted_at_its_name() {
let dir = staging_dir("planted");
// A file elsewhere that a planted link points at.
let outside = dir.join("outside.txt");
std::fs::write(&outside, "PRECIOUS").unwrap();
let target = dir.join("out.pdf");
// A hard link needs no privileges on any system, and writing to it writes to `outside`.
std::fs::hard_link(&outside, staged(&dir, 1)).unwrap();
std::fs::write(staged(&dir, 2), "PLANTED").unwrap();
write_atomic_with(&target, b"NEW", [1, 2, 3]).unwrap();
assert_eq!(read(&target), "NEW");
assert_eq!(read(&outside), "PRECIOUS");
assert_eq!(read(&staged(&dir, 1)), "PRECIOUS");
assert_eq!(read(&staged(&dir, 2)), "PLANTED");
assert!(!staged(&dir, 3).exists(), "the staging file was renamed into place");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn staging_never_writes_through_a_symbolic_link_planted_at_its_name() {
let dir = staging_dir("symlink");
let outside = dir.join("outside.txt");
std::fs::write(&outside, "PRECIOUS").unwrap();
let target = dir.join("out.pdf");
if let Err(e) = file_symlink(&outside, &staged(&dir, 1)) {
eprintln!("symbolic links not checked: {e}");
return;
}
write_atomic_with(&target, b"NEW", [1, 2]).unwrap();
assert_eq!(read(&target), "NEW");
assert_eq!(read(&outside), "PRECIOUS");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn staging_never_creates_a_file_through_a_dangling_link_at_its_name() {
let dir = staging_dir("dangling");
let unborn = dir.join("created-through-a-link.txt");
let target = dir.join("out.pdf");
if let Err(e) = file_symlink(&unborn, &staged(&dir, 1)) {
eprintln!("symbolic links not checked: {e}");
return;
}
write_atomic_with(&target, b"NEW", [1, 2]).unwrap();
assert_eq!(read(&target), "NEW");
assert!(!unborn.exists(), "nothing was created through the dangling link");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn staging_gives_up_rather_than_reuse_a_taken_name() {
let dir = staging_dir("taken");
let target = dir.join("out.pdf");
std::fs::write(&target, "OLD").unwrap();
for s in 1..=STAGING_ATTEMPTS as u64 {
std::fs::write(staged(&dir, s), "PLANTED").unwrap();
}
assert!(write_atomic_with(&target, b"NEW", 1..).is_err());
assert_eq!(read(&target), "OLD");
for s in 1..=STAGING_ATTEMPTS as u64 {
assert_eq!(read(&staged(&dir, s)), "PLANTED");
}
let _ = std::fs::remove_dir_all(&dir);
}
/// The names in a folder: what a test can see was left behind.
fn listing(dir: &Path) -> Vec<String> {
let mut names: Vec<_> = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
names.sort();
names
}
#[test]
fn staging_names_differ_between_saves_and_fit_long_names() {
let suffixes: std::collections::HashSet<u64> = staging_suffixes().take(64).collect();
assert_eq!(suffixes.len(), 64);
assert_ne!(staging_suffixes().next(), staging_suffixes().next(), "each save draws new names");
// 60 four-byte characters: a 244-byte name, within every system's limit. Its staging name
// must be too (on Linux the whole name in it would be 275 bytes).
let dir = staging_dir("long");
let name = format!("{}.pdf", "\u{1F600}".repeat(60));
write_atomic(&dir.join(&name), b"NEW").unwrap();
assert_eq!(read(&dir.join(&name)), "NEW");
assert_eq!(listing(&dir), [name], "no staging file is left behind");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn a_folder_at_the_staging_name_is_left_alone() {
let dir = staging_dir("folder");
std::fs::create_dir(staged(&dir, 1)).unwrap();
std::fs::write(staged(&dir, 1).join("inside.txt"), "PLANTED").unwrap();
let target = dir.join("out.pdf");
write_atomic_with(&target, b"NEW", [1, 2]).unwrap();
assert_eq!(read(&target), "NEW");
assert_eq!(read(&staged(&dir, 1).join("inside.txt")), "PLANTED");
let _ = std::fs::remove_dir_all(&dir);
}
/// Windows refuses to replace a read-only file, so the rename fails: the staging file must not
/// be left behind.
#[cfg(windows)]
#[test]
fn a_failed_rename_removes_the_staging_file() {
let dir = staging_dir("readonly");
let target = dir.join("out.pdf");
std::fs::write(&target, "OLD").unwrap();
let mut perms = std::fs::metadata(&target).unwrap().permissions();
perms.set_readonly(true);
std::fs::set_permissions(&target, perms.clone()).unwrap();
let e = write_atomic_with(&target, b"NEW", [7]).unwrap_err();
assert!(e.to_string().contains(&target.display().to_string()), "the rename failed, not the staging: {e}");
assert_eq!(read(&target), "OLD");
assert_eq!(listing(&dir), ["out.pdf"], "the staging file was removed");
#[allow(clippy::permissions_set_readonly_false)]
perms.set_readonly(false);
std::fs::set_permissions(&target, perms).unwrap();
let _ = std::fs::remove_dir_all(&dir);
}
}

13
crates/automation/tests/automation.rs
View File

@ -405,12 +405,20 @@ fn root_refusals_do_not_reveal_what_exists_outside() {
#[test]
fn writing_to_a_folder_touches_nothing_beside_it() {
// "." names the root itself. Saving there used to stage its temporary file next to the
// root, outside it, overwriting and then deleting any file of that name.
// root, outside it, overwriting and then deleting any file of that name. Staging names are
// random now and a failed rename removes the staging file, so the "is a folder" refusal is
// what this checks; the listings and the file at the old staging name are canaries.
let (base, root) = sandbox("root-itself");
let mut a = auto(&root);
let beside = base.join(".root.pdfcraft-tmp");
std::fs::write(&beside, "SENTINEL").unwrap();
std::fs::create_dir_all(root.join("folder")).unwrap();
let listing = |dir: &Path| {
let mut names: Vec<_> = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().file_name()).collect();
names.sort();
names
};
let (base_before, folder_before) = (listing(&base), listing(&root.join("folder")));
let doc = ok(&mut a, "doc_open", json!({ "path": "inside.pdf" }))["doc"].as_u64().unwrap();
for p in [".", "", "folder", "folder/"] {
let e = a.call("doc_save", &json!({ "doc": doc, "path": p })).unwrap_err();
@ -419,7 +427,8 @@ fn writing_to_a_folder_touches_nothing_beside_it() {
let png = vec![1, 2, 3];
assert!(a.write_output(".", &png).is_err());
assert_eq!(std::fs::read_to_string(&beside).unwrap(), "SENTINEL");
assert!(!root.join(".folder.pdfcraft-tmp").exists());
assert_eq!(listing(&base), base_before, "nothing was left beside the root");
assert_eq!(listing(&root.join("folder")), folder_before, "nothing was left in the folder");
// `image_save` adds an extension when the path has none, which turned "." into `root.png`
// beside the root.
ok(&mut a, "doc_export_images", json!({ "doc": doc, "folder": "src", "dpi": 18 }));

221
crates/ui-egui/src/editing.rs
View File

@ -492,14 +492,23 @@ fn comment_page(edit: &Edit) -> Option<usize> {
/// Write via a temporary file in the same directory and rename over the target, so a crash or
/// full disk never leaves a half-written PDF where the original was.
pub fn write_atomically(path: &str, bytes: &[u8]) -> std::io::Result<()> {
write_atomically_with(path, bytes, staging_suffixes())
}
/// [`write_atomically`], trying the staging names that `suffixes` give.
fn write_atomically_with(path: &str, bytes: &[u8], suffixes: impl IntoIterator<Item = u64>) -> std::io::Result<()> {
use std::io::Write;
let target = std::path::Path::new(path);
let dir = target.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(std::path::Path::new("."));
let tmp = dir.join(format!(".{}.pdfcraft-{}.tmp", target.file_name().and_then(|n| n.to_str()).unwrap_or("save"), std::process::id()));
let name = target.file_name().and_then(|n| n.to_str()).unwrap_or("save");
let (tmp, f) = create_staging(dir, name, suffixes)?;
let result = (|| {
let mut f = std::fs::File::create(&tmp)?;
f.write_all(bytes)?;
f.sync_all()?;
// Closed at the end of the block, before the rename.
{
let mut f = f;
f.write_all(bytes)?;
f.sync_all()?;
}
std::fs::rename(&tmp, target)
})();
if result.is_err() {
@ -508,6 +517,50 @@ pub fn write_atomically(path: &str, bytes: &[u8]) -> std::io::Result<()> {
result
}
/// How many staging names [`create_staging`] tries. A random 64-bit name is only taken if someone
/// put a file there on purpose, so running out means refusing, not trying harder.
const STAGING_ATTEMPTS: usize = 16;
/// Create a new, empty staging file in `dir` for the file `name`, one name per suffix. It is
/// opened with `create_new`, which fails if anything already has the name (a file, a hard link,
/// a symbolic link even when dangling, a folder), on Windows as everywhere else; such a name is
/// skipped, never opened, so a file planted at the staging path can't receive or redirect the
/// save.
fn create_staging(
dir: &std::path::Path,
name: &str,
suffixes: impl IntoIterator<Item = u64>,
) -> std::io::Result<(std::path::PathBuf, std::fs::File)> {
// At most 128 bytes of the target's name, cut between characters, so the staging name fits
// the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is.
let mut stem = String::new();
for c in name.chars() {
if stem.len() + c.len_utf8() > 128 {
break;
}
stem.push(c);
}
for suffix in suffixes.into_iter().take(STAGING_ATTEMPTS) {
let tmp = dir.join(format!(".{stem}.pdfcraft-{suffix:016x}.tmp"));
match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
Ok(file) => return Ok((tmp, file)),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
// Windows reports a folder at the name as "access denied"; it is taken all the same.
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied && tmp.symlink_metadata().is_ok() => {}
Err(e) => return Err(e),
}
}
Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken"))
}
/// Unpredictable staging-name suffixes, so a name can't be planted in advance. `RandomState` is
/// keyed from the operating system's random source.
fn staging_suffixes() -> impl Iterator<Item = u64> {
use std::hash::BuildHasher;
let state = std::hash::RandomState::new();
(0u64..).map(move |i| state.hash_one(i))
}
#[cfg(not(target_arch = "wasm32"))]
fn short_name(path: &str) -> String {
std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string())
@ -586,3 +639,163 @@ fn bookmark_at<'a>(items: &'a [pdfcraft_render::OutlineItem], path: &[usize]) ->
let item = items.get(*first)?;
if rest.is_empty() { Some(item) } else { bookmark_at(&item.children, rest) }
}
#[cfg(test)]
mod tests {
use super::{STAGING_ATTEMPTS, staging_suffixes, write_atomically, write_atomically_with};
use std::path::{Path, PathBuf};
/// A fresh, empty folder for one staging test.
fn staging_dir(test: &str) -> PathBuf {
let dir = std::env::temp_dir().join(format!("pdfcraft-ui-staging-{}-{test}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
dir
}
fn staged(dir: &Path, suffix: u64) -> PathBuf {
dir.join(format!(".out.pdf.pdfcraft-{suffix:016x}.tmp"))
}
fn read(p: &Path) -> String {
std::fs::read_to_string(p).unwrap()
}
/// A symbolic link to a file, where the system allows one (Windows needs Developer Mode or an
/// administrator for it).
fn file_symlink(target: &Path, link: &Path) -> std::io::Result<()> {
#[cfg(unix)]
{
std::os::unix::fs::symlink(target, link)
}
#[cfg(windows)]
{
std::os::windows::fs::symlink_file(target, link)
}
#[cfg(not(any(unix, windows)))]
{
Err(std::io::Error::other(format!("no symbolic links here: {} {}", target.display(), link.display())))
}
}
#[test]
fn saving_never_writes_through_a_file_planted_at_the_staging_name() {
let dir = staging_dir("planted");
let outside = dir.join("outside.txt");
std::fs::write(&outside, "PRECIOUS").unwrap();
let target = dir.join("out.pdf");
let path = target.to_string_lossy().into_owned();
// A hard link needs no privileges on any system, and writing to it writes to `outside`.
std::fs::hard_link(&outside, staged(&dir, 1)).unwrap();
std::fs::write(staged(&dir, 2), "PLANTED").unwrap();
write_atomically_with(&path, b"NEW", [1, 2, 3]).unwrap();
assert_eq!(read(&target), "NEW");
assert_eq!(read(&outside), "PRECIOUS");
assert_eq!(read(&staged(&dir, 1)), "PRECIOUS");
assert_eq!(read(&staged(&dir, 2)), "PLANTED");
assert!(!staged(&dir, 3).exists(), "the staging file was renamed into place");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn saving_never_writes_through_a_symbolic_link_at_the_staging_name() {
let dir = staging_dir("symlink");
let outside = dir.join("outside.txt");
std::fs::write(&outside, "PRECIOUS").unwrap();
let target = dir.join("out.pdf");
if let Err(e) = file_symlink(&outside, &staged(&dir, 1)) {
eprintln!("symbolic links not checked: {e}");
return;
}
write_atomically_with(&target.to_string_lossy(), b"NEW", [1, 2]).unwrap();
assert_eq!(read(&target), "NEW");
assert_eq!(read(&outside), "PRECIOUS");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn saving_never_creates_a_file_through_a_dangling_link_at_the_staging_name() {
let dir = staging_dir("dangling");
let unborn = dir.join("created-through-a-link.txt");
let target = dir.join("out.pdf");
if let Err(e) = file_symlink(&unborn, &staged(&dir, 1)) {
eprintln!("symbolic links not checked: {e}");
return;
}
write_atomically_with(&target.to_string_lossy(), b"NEW", [1, 2]).unwrap();
assert_eq!(read(&target), "NEW");
assert!(!unborn.exists(), "nothing was created through the dangling link");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn a_folder_at_the_staging_name_is_left_alone() {
let dir = staging_dir("folder");
std::fs::create_dir(staged(&dir, 1)).unwrap();
std::fs::write(staged(&dir, 1).join("inside.txt"), "PLANTED").unwrap();
let target = dir.join("out.pdf");
write_atomically_with(&target.to_string_lossy(), b"NEW", [1, 2]).unwrap();
assert_eq!(read(&target), "NEW");
assert_eq!(read(&staged(&dir, 1).join("inside.txt")), "PLANTED");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn saving_gives_up_rather_than_reuse_a_taken_name() {
let dir = staging_dir("taken");
let target = dir.join("out.pdf");
std::fs::write(&target, "OLD").unwrap();
for s in 1..=STAGING_ATTEMPTS as u64 {
std::fs::write(staged(&dir, s), "PLANTED").unwrap();
}
assert!(write_atomically_with(&target.to_string_lossy(), b"NEW", 1..).is_err());
assert_eq!(read(&target), "OLD");
for s in 1..=STAGING_ATTEMPTS as u64 {
assert_eq!(read(&staged(&dir, s)), "PLANTED");
}
let _ = std::fs::remove_dir_all(&dir);
}
/// The names in a folder: what a test can see was left behind.
fn listing(dir: &Path) -> Vec<String> {
let mut names: Vec<_> = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
names.sort();
names
}
#[test]
fn saving_a_long_name_leaves_only_the_saved_file() {
assert_ne!(staging_suffixes().next(), staging_suffixes().next(), "each save draws new names");
// 60 four-byte characters: a 244-byte name, within every system's limit. Its staging name
// must be too (on Linux the whole name in it would be 275 bytes).
let dir = staging_dir("clean");
let name = format!("{}.pdf", "\u{1F600}".repeat(60));
let path = dir.join(&name).to_string_lossy().into_owned();
write_atomically(&path, b"NEW").unwrap();
write_atomically(&path, b"NEWER").unwrap();
assert_eq!(read(&dir.join(&name)), "NEWER");
assert_eq!(listing(&dir), [name], "no staging file is left behind");
let _ = std::fs::remove_dir_all(&dir);
}
/// Windows refuses to replace a read-only file, so the rename fails: the staging file must not
/// be left behind.
#[cfg(windows)]
#[test]
fn a_failed_save_removes_the_staging_file() {
let dir = staging_dir("readonly");
let target = dir.join("out.pdf");
std::fs::write(&target, "OLD").unwrap();
let mut perms = std::fs::metadata(&target).unwrap().permissions();
perms.set_readonly(true);
std::fs::set_permissions(&target, perms.clone()).unwrap();
let e = write_atomically_with(&target.to_string_lossy(), b"NEW", [7]).unwrap_err();
assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied, "the rename failed: {e}");
assert_eq!(read(&target), "OLD");
assert_eq!(listing(&dir), ["out.pdf"], "the staging file was removed");
#[allow(clippy::permissions_set_readonly_false)]
perms.set_readonly(false);
std::fs::set_permissions(&target, perms).unwrap();
let _ = std::fs::remove_dir_all(&dir);
}
}

4
parity/acrobat-features.toml
View File

@ -274,8 +274,8 @@ milestone = "M1"
status = "shipped"
commands = ["file.save"]
tools = ["doc_save"]
evidence = ["crates/ui-egui/tests/editing.rs::save_writes_an_incremental_update_and_clears_dirty", "crates/engine/src/tests.rs::save_is_incremental_stamps_mod_date_and_rebases", "crates/organize/src/tests.rs::successive_incremental_saves_stack_revisions"]
notes = ""
evidence = ["crates/ui-egui/tests/editing.rs::save_writes_an_incremental_update_and_clears_dirty", "crates/engine/src/tests.rs::save_is_incremental_stamps_mod_date_and_rebases", "crates/organize/src/tests.rs::successive_incremental_saves_stack_revisions", "crates/automation/src/lib.rs::staging_never_writes_through_a_file_planted_at_its_name", "crates/ui-egui/src/editing.rs::saving_never_writes_through_a_file_planted_at_the_staging_name"]
notes = "The atomic write stages to a new, randomly named file created with create_new, so a file or link planted at the staging path is never written through (desktop app, automation and CLI)."
[[feature]]
id = "core.save-as-full-rewrite"