masi/solvecraft
1
0
Fork 0
mirror of https://github.com/bherbruck/solidcraft.git synced 2026-11-03 02:25:55 +00:00
Code Issues Packages Projects Releases 1 Wiki Activity

Lock files: never written or read through a link (#10951)

Browse Source 
A design's lock file went through the same writer as the design, which follows a symbolic link
so a linked design is saved where it lives. The lock now has a writer of its own that refuses
anything at its path but a regular file and replaces only that name. Reading a lock takes only a
regular file and no more than a lock's size. Saving a design through a link works as before.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
bherbruck
2026-11-02 03:53:28 +00:00
committed by GitHub
parent ec39a8ebb4
commit 9636188a0f
2 changed files with 160 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/io/src/design_lock.rs
+21 -5
View File
@@ -15,6 +15,7 @@
//! The browser build keeps neither: it has no shared folders, and no clock in `std`.
use serde::{Deserialize, Serialize};
use std::io::Read;
use std::path::{Path, PathBuf};
/// How often an open design's lock is written again.
@@ -109,16 +110,29 @@ fn user_name() -> String {
["USER", "USERNAME", "LOGNAME"].iter().find_map(|k| std::env::var(k).ok().filter(|v| !v.trim().is_empty())).unwrap_or_default()
}
/// The lock next to the design at `path`, if there is a readable one.
/// The lock next to the design at `path`, if there is a readable one. Anything at the lock's path
/// but a regular file (a symbolic link, a device, a FIFO) is no lock and is not opened, and no
/// more than a lock's size is ever read.
pub fn read(path: &Path) -> Option<Lock> {
if cfg!(target_arch = "wasm32") {
return None;
}
let file = lock_path(path);
if std::fs::metadata(&file).ok()?.len() > MAX_LOCK_BYTES {
let meta = std::fs::symlink_metadata(&file).ok().filter(std::fs::Metadata::is_file)?;
if meta.len() > MAX_LOCK_BYTES {
return None;
}
serde_json::from_slice(&std::fs::read(&file).ok()?).ok()
let opened = std::fs::File::open(&file).ok()?;
// The path may have been swapped between the two calls; the handle decides.
if !opened.metadata().ok()?.is_file() {
return None;
}
let mut bytes = Vec::new();
opened.take(MAX_LOCK_BYTES + 1).read_to_end(&mut bytes).ok()?;
if bytes.len() as u64 > MAX_LOCK_BYTES {
return None;
}
serde_json::from_slice(&bytes).ok()
}
/// Another app's fresh lock on the design at `path` at `now`: it is open elsewhere.
@@ -126,10 +140,12 @@ pub fn held_elsewhere(path: &Path, now: u64) -> Option<Lock> {
read(path).filter(|l| !l.is_mine() && !l.is_stale(now))
}
/// Write `lock` next to the design at `path` (replacing whatever lock is there).
/// Write `lock` next to the design at `path`, replacing whatever lock is there. A symbolic link
/// at the lock's path is refused and never followed, unlike a save, which follows a link to where
/// the design really lives: the file a link there points to is not this app's to replace.
pub fn write(path: &Path, lock: &Lock) -> std::io::Result<()> {
let bytes = serde_json::to_vec_pretty(lock).map_err(std::io::Error::other)?;
crate::safe_write::write_atomic(&lock_path(path), &bytes, false)
crate::safe_write::write_atomic_no_follow(&lock_path(path), &bytes)
}
/// Remove this process's lock on the design at `path`; another app's lock is left alone.
crates/io/src/safe_write.rs
+139 -6
View File
@@ -226,13 +226,17 @@ fn write_steps(path: &Path, bytes: &[u8], backup: bool, stop: Option<Step>) -> s
return Err(e);
}
sync_dir(path);
// Temporary files of saves that crashed part way (a minute old or more) go.
for t in stale_temps(path) {
if fs::metadata(&t).and_then(|m| m.modified()).ok().and_then(|m| m.elapsed().ok()).is_some_and(|age| age.as_secs() >= 60) {
remove_old_temps(path);
Ok(())
}
/// Temporary files of saves that crashed part way (a minute old or more) go.
fn remove_old_temps(path: &Path) {
for t in temps_beside(path) {
if fs::symlink_metadata(&t).and_then(|m| m.modified()).ok().and_then(|m| m.elapsed().ok()).is_some_and(|age| age.as_secs() >= 60) {
let _ = fs::remove_file(t);
}
}
Ok(())
}
/// Write `bytes` to `path` atomically; with `backup`, the file being replaced is kept as
@@ -249,6 +253,34 @@ pub fn write_atomic(path: &Path, bytes: &[u8], backup: bool) -> std::io::Result<
write_steps(&resolve_links(path)?, bytes, backup, None)
}
/// Write `bytes` to the file named `path` itself, atomically: a link at `path` is never followed,
/// and what it points to is never touched. Anything there that isn't a regular file is refused. For
/// the app's own small files beside a design, which never live anywhere else.
pub fn write_atomic_no_follow(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
if cfg!(target_arch = "wasm32") {
return crate::vfs::write(&path.to_string_lossy(), bytes);
}
let there = fs::symlink_metadata(path).ok();
if there.as_ref().is_some_and(|m| !m.is_file()) {
return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{}: not a regular file", path.display())));
}
let tmp = temp_beside(path, "new");
if let Err(e) = write_synced(&tmp, bytes) {
let _ = fs::remove_file(&tmp);
return Err(e);
}
keep_permissions(&there.map(|m| m.permissions()), &tmp);
// A rename replaces the name itself, so a link put there since the check above goes, and the
// file it pointed to stays as it was.
if let Err(e) = replace_synced(&tmp, path) {
let _ = fs::remove_file(&tmp);
return Err(e);
}
sync_dir(path);
remove_old_temps(path);
Ok(())
}
/// Publish a complete new file without replacing an existing destination, even if another
/// writer creates it after this call starts. Unsupported filesystems return an error.
pub fn create_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
@@ -275,8 +307,11 @@ pub fn create_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
/// Leftover temporary files of saves that never finished (a crash part way), next to `path`
/// (next to the file a link at `path` points to, where a save puts them).
pub fn stale_temps(path: &Path) -> Vec<PathBuf> {
let resolved = resolve_links(path).unwrap_or_else(|_| path.to_path_buf());
let path = resolved.as_path();
temps_beside(&resolve_links(path).unwrap_or_else(|_| path.to_path_buf()))
}
/// The leftover temporary files of the file named `path` itself.
fn temps_beside(path: &Path) -> Vec<PathBuf> {
let Some(name) = path.file_name().map(|n| n.to_string_lossy().to_string()) else { return Vec::new() };
let dir = path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new("."));
let prefix = format!(".{name}.");
@@ -407,6 +442,104 @@ mod tests {
let _ = std::fs::remove_dir_all(&d);
}
/// A link at the lock's path is neither written through nor read through, and closing the
/// design leaves it alone. A save still follows a link at the design's own path.
#[cfg(unix)]
#[test]
fn a_link_at_the_lock_path_is_never_followed() {
use crate::design_lock as lock;
use std::os::unix::fs::symlink;
let d = dir("design-lock-link");
let design = d.join("Bracket.solidcraft");
std::fs::write(&design, b"one").unwrap();
let lock_file = lock::lock_path(&design);
let now = lock::now_secs();
let mine = lock::Lock::mine(now);
let victim = d.join("victim.txt");
std::fs::write(&victim, b"keep me").unwrap();
symlink(&victim, &lock_file).unwrap();
assert!(lock::write(&design, &mine).is_err());
assert_eq!(std::fs::read(&victim).unwrap(), b"keep me");
assert!(std::fs::symlink_metadata(&lock_file).unwrap().file_type().is_symlink(), "the link is refused, not replaced");
std::fs::remove_file(&lock_file).unwrap();
// A link to a place with nothing there doesn't make the file either.
let missing = d.join("made-by-the-link.txt");
symlink(&missing, &lock_file).unwrap();
assert!(lock::write(&design, &mine).is_err());
assert!(!missing.exists());
std::fs::remove_file(&lock_file).unwrap();
// A link to a real lock is no lock: nothing is read through it, and closing leaves it.
let elsewhere = d.join("elsewhere.lock");
std::fs::write(&elsewhere, serde_json::to_vec(&mine).unwrap()).unwrap();
symlink(&elsewhere, &lock_file).unwrap();
assert!(lock::read(&design).is_none());
lock::remove(&design);
assert!(std::fs::symlink_metadata(&lock_file).is_ok() && elsewhere.exists());
std::fs::remove_file(&lock_file).unwrap();
// A hard link is a regular file: its name is replaced, the other name keeps its content.
let other_name = d.join("other-name.txt");
std::fs::write(&other_name, b"keep me too").unwrap();
std::fs::hard_link(&other_name, &lock_file).unwrap();
lock::write(&design, &mine).unwrap();
assert_eq!(std::fs::read(&other_name).unwrap(), b"keep me too");
assert!(lock::read(&design).is_some_and(|l| l.is_mine()));
std::fs::remove_file(&lock_file).unwrap();
// Nothing there, and no temporary file left: a plain lock is still written.
lock::write(&design, &mine).unwrap();
assert_eq!(lock::read(&design), Some(mine));
assert!(stale_temps(&lock_file).is_empty());
// The design's own link is still followed by a save.
let real = d.join("real.solidcraft");
std::fs::write(&real, b"old").unwrap();
let linked = d.join("linked.solidcraft");
symlink(&real, &linked).unwrap();
write_atomic(&linked, b"new", false).unwrap();
assert_eq!(std::fs::read(&real).unwrap(), b"new");
assert!(std::fs::symlink_metadata(&linked).unwrap().file_type().is_symlink());
let _ = std::fs::remove_dir_all(&d);
}
/// A lock path that isn't a regular file is ignored at once. Read on a thread, so a regression
/// fails here rather than holding up the test run.
#[cfg(unix)]
#[test]
fn a_lock_path_that_is_not_a_regular_file_is_ignored_at_once() {
use crate::design_lock as lock;
use std::os::unix::fs::symlink;
let d = dir("design-lock-special");
let design = d.join("Bracket.solidcraft");
let lock_file = lock::lock_path(&design);
let read_soon = |design: &Path| {
let (tx, rx) = std::sync::mpsc::channel();
let design = design.to_path_buf();
std::thread::spawn(move || {
let _ = tx.send(lock::read(&design));
});
rx.recv_timeout(std::time::Duration::from_secs(10)).expect("read returned")
};
let cases: [&dyn Fn(&Path); 3] = [&|p| symlink("/dev/zero", p).unwrap(), &|p| std::fs::create_dir(p).unwrap(), &|p| {
assert!(std::process::Command::new("mkfifo").arg(p).status().unwrap().success())
}];
for make in cases {
make(&lock_file);
assert!(read_soon(&design).is_none());
assert!(lock::held_elsewhere(&design, lock::now_secs()).is_none());
assert!(lock::write(&design, &lock::Lock::mine(lock::now_secs())).is_err());
assert!(std::fs::symlink_metadata(&lock_file).is_ok(), "left where it is");
let _ = std::fs::remove_file(&lock_file).or_else(|_| std::fs::remove_dir(&lock_file));
}
// A regular file too large to be a lock is not read either.
std::fs::write(&lock_file, vec![b' '; 4097]).unwrap();
assert!(read_soon(&design).is_none());
let _ = std::fs::remove_dir_all(&d);
}
#[test]
fn create_does_not_replace_an_existing_destination() {
let d = dir("create");