fix(io): verify saves through edit leases
This commit is contained in:
commit
313349fd71
4 changed files with 342 additions and 25 deletions
|
|
@ -212,7 +212,7 @@ impl OpenCADStudio {
|
|||
let Some(path) = req["path"].as_str() else {
|
||||
return err("open: missing \"path\"");
|
||||
};
|
||||
let bytes = match std::fs::read(path) {
|
||||
let bytes = match self.read_drawing(std::path::Path::new(path)) {
|
||||
Ok(b) => b,
|
||||
Err(e) => return err(format!("open: {e}")),
|
||||
};
|
||||
|
|
@ -981,6 +981,42 @@ mod tests {
|
|||
let _ = std::fs::remove_file(&b);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saving_over_an_existing_drawing_succeeds() {
|
||||
for (label, pre_existing) in [("new path", false), ("existing drawing", true)] {
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"ocs_save_over_{}_{}.dxf",
|
||||
std::process::id(),
|
||||
pre_existing,
|
||||
));
|
||||
let _ = std::fs::remove_file(&path);
|
||||
if pre_existing {
|
||||
std::fs::write(&path, b"a previous drawing").unwrap();
|
||||
}
|
||||
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
app.automation_op(r#"{"op":"new"}"#);
|
||||
let p = path.to_string_lossy().replace('\\', "\\\\");
|
||||
let saved = app.automation_op(&format!(r#"{{"op":"save","path":"{p}"}}"#));
|
||||
assert_eq!(saved["ok"], true, "{label}: {}", saved["error"]);
|
||||
let saved_again = app.automation_op(r#"{"op":"save"}"#);
|
||||
assert_eq!(
|
||||
saved_again["ok"],
|
||||
true,
|
||||
"normal save: {}",
|
||||
saved_again["error"]
|
||||
);
|
||||
|
||||
drop(app);
|
||||
let sidecar = path.with_file_name(format!(
|
||||
".{}.ocs.lock",
|
||||
path.file_name().unwrap().to_string_lossy()
|
||||
));
|
||||
let _ = std::fs::remove_file(sidecar);
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_then_open_round_trips() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
|
|
|
|||
|
|
@ -711,6 +711,30 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Read through this session's lease when it covers `path`.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub(in crate::app) fn read_drawing(
|
||||
&self,
|
||||
path: &std::path::Path,
|
||||
) -> std::io::Result<Vec<u8>> {
|
||||
use std::io::{Read, Seek, SeekFrom};
|
||||
|
||||
let lease = self
|
||||
.tab_showing(path)
|
||||
.and_then(|i| self.tabs[i].edit_lease.as_ref());
|
||||
let leased = match lease {
|
||||
Some(lease) => lease.reader()?,
|
||||
None => None,
|
||||
};
|
||||
let Some(mut reader) = leased else {
|
||||
return std::fs::read(path);
|
||||
};
|
||||
reader.seek(SeekFrom::Start(0))?;
|
||||
let mut bytes = Vec::new();
|
||||
reader.read_to_end(&mut bytes)?;
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
/// Start the next drawing a second launch handed us, if any.
|
||||
///
|
||||
/// Must be called from EVERY path that clears `opening` — completion, error
|
||||
|
|
@ -851,7 +875,7 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
));
|
||||
}
|
||||
|
||||
let destination_lease = if path_changed {
|
||||
let mut destination_lease = if path_changed {
|
||||
match crate::io::edit_lock::EditLease::acquire(&path) {
|
||||
Ok(lease) => Some(lease),
|
||||
Err(crate::io::edit_lock::EditLeaseError::Locked(error)) => {
|
||||
|
|
@ -869,21 +893,17 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
};
|
||||
|
||||
let expected_fingerprint = if path_changed {
|
||||
match crate::io::edit_lock::FileFingerprint::capture(&path) {
|
||||
Ok(fingerprint) => Some(fingerprint),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => None,
|
||||
Err(error) => {
|
||||
return Err(crate::io::SaveFailure::other(format!(
|
||||
"could not verify {} before saving: {error}",
|
||||
path.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
None
|
||||
} else {
|
||||
self.tabs[i].disk_fingerprint.clone().or_else(|| {
|
||||
crate::io::edit_lock::FileFingerprint::capture(&path).ok()
|
||||
})
|
||||
self.tabs[i].disk_fingerprint.clone()
|
||||
};
|
||||
let lease = if path_changed {
|
||||
destination_lease.as_mut()
|
||||
} else {
|
||||
self.tabs[i].edit_lease.as_mut()
|
||||
};
|
||||
let (expected_fingerprint, verify_reader) =
|
||||
Self::native_save_verification(&path, lease, expected_fingerprint)?;
|
||||
|
||||
self.prepare_native_save(i);
|
||||
let version = self.tabs[i].scene.document.version;
|
||||
|
|
@ -894,6 +914,7 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
version,
|
||||
self.backup_on_save,
|
||||
expected_fingerprint,
|
||||
verify_reader,
|
||||
)?;
|
||||
|
||||
if set_current_path {
|
||||
|
|
@ -1380,6 +1401,57 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
self.sync_solid_models_to_acis(i);
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn native_save_verification(
|
||||
path: &std::path::Path,
|
||||
mut lease: Option<&mut crate::io::edit_lock::EditLease>,
|
||||
expected: Option<crate::io::edit_lock::FileFingerprint>,
|
||||
) -> Result<
|
||||
(
|
||||
Option<crate::io::edit_lock::FileFingerprint>,
|
||||
Option<std::fs::File>,
|
||||
),
|
||||
crate::io::SaveFailure,
|
||||
> {
|
||||
let expected = match expected {
|
||||
Some(expected) => Some(expected),
|
||||
None => {
|
||||
let captured = match lease.as_deref_mut() {
|
||||
Some(lease) => match lease.fingerprint() {
|
||||
Ok(Some(fingerprint)) => Ok(fingerprint),
|
||||
Ok(None) => crate::io::edit_lock::FileFingerprint::capture(path),
|
||||
Err(error) => Err(error),
|
||||
},
|
||||
None => crate::io::edit_lock::FileFingerprint::capture(path),
|
||||
};
|
||||
match captured {
|
||||
Ok(fingerprint) => Some(fingerprint),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => None,
|
||||
Err(error) => {
|
||||
return Err(crate::io::SaveFailure::other(format!(
|
||||
"could not verify {} before saving: {error}",
|
||||
path.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
let reader = if expected.is_some() {
|
||||
match lease.as_deref() {
|
||||
Some(lease) => lease.reader().map_err(|error| {
|
||||
crate::io::SaveFailure::other(format!(
|
||||
"could not verify {} before saving: {error}",
|
||||
path.display()
|
||||
))
|
||||
})?,
|
||||
None => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
Ok((expected, reader))
|
||||
}
|
||||
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
fn stamp_thumbnail(&mut self, i: usize, version: acadrust::DxfVersion) {
|
||||
let scene = &self.tabs[i].scene;
|
||||
|
|
@ -1532,16 +1604,58 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
let previous_autosave =
|
||||
(purpose != crate::app::SavePurpose::Autosave).then(|| self.autosave_target(i));
|
||||
let backup = purpose != crate::app::SavePurpose::Autosave && self.backup_on_save;
|
||||
let expected_fingerprint =
|
||||
if check_external_change && purpose != crate::app::SavePurpose::Autosave {
|
||||
if set_current_path {
|
||||
crate::io::edit_lock::FileFingerprint::capture(&path).ok()
|
||||
} else {
|
||||
self.tabs[i].disk_fingerprint.clone()
|
||||
}
|
||||
} else {
|
||||
let verification = if check_external_change
|
||||
&& purpose != crate::app::SavePurpose::Autosave
|
||||
{
|
||||
let expected = if set_current_path {
|
||||
None
|
||||
} else {
|
||||
self.tabs[i].disk_fingerprint.clone()
|
||||
};
|
||||
if self.pending_save_leases.contains_key(&tab_id) {
|
||||
Self::native_save_verification(
|
||||
&path,
|
||||
self.pending_save_leases.get_mut(&tab_id),
|
||||
expected,
|
||||
)
|
||||
} else if destination_is_current {
|
||||
Self::native_save_verification(
|
||||
&path,
|
||||
self.tabs[i].edit_lease.as_mut(),
|
||||
expected,
|
||||
)
|
||||
} else {
|
||||
Self::native_save_verification(&path, None, expected)
|
||||
}
|
||||
} else {
|
||||
Ok((None, None))
|
||||
};
|
||||
let (expected_fingerprint, verify_reader) = match verification {
|
||||
Ok(verification) => verification,
|
||||
Err(error) => {
|
||||
return Task::perform(
|
||||
async move {
|
||||
crate::app::SaveOutcome {
|
||||
job_id,
|
||||
tab_id,
|
||||
epoch,
|
||||
revision,
|
||||
camera_generation,
|
||||
path,
|
||||
version,
|
||||
previous_autosave,
|
||||
set_current_path,
|
||||
purpose,
|
||||
continuation,
|
||||
thumbnail_key,
|
||||
refreshed_preview: None,
|
||||
result: Err(error),
|
||||
}
|
||||
},
|
||||
Message::SaveFinished,
|
||||
);
|
||||
}
|
||||
};
|
||||
let worker_path = path.clone();
|
||||
|
||||
Task::perform(
|
||||
|
|
@ -1572,6 +1686,7 @@ pub(super) fn on_open_file(&mut self) -> Task<Message> {
|
|||
version,
|
||||
backup,
|
||||
expected_fingerprint,
|
||||
verify_reader,
|
||||
);
|
||||
(result, refreshed_preview)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -22,7 +22,11 @@ pub struct FileFingerprint {
|
|||
|
||||
impl FileFingerprint {
|
||||
pub fn capture(path: &Path) -> std::io::Result<Self> {
|
||||
let mut file = File::open(path)?;
|
||||
Self::capture_from(&mut File::open(path)?)
|
||||
}
|
||||
|
||||
/// Fingerprint an already-open drawing handle.
|
||||
pub fn capture_from(file: &mut File) -> std::io::Result<Self> {
|
||||
let metadata = file.metadata()?;
|
||||
let len = metadata.len();
|
||||
let modified_ns = metadata
|
||||
|
|
@ -66,6 +70,52 @@ impl FileFingerprint {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn path_matches_file(path: &Path, file: &File) -> std::io::Result<bool> {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
|
||||
let path_metadata = std::fs::metadata(path)?;
|
||||
let file_metadata = file.metadata()?;
|
||||
Ok(path_metadata.dev() == file_metadata.dev() && path_metadata.ino() == file_metadata.ino())
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub(crate) fn path_matches_file(path: &Path, file: &File) -> std::io::Result<bool> {
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
use windows_sys::Win32::Storage::FileSystem::{
|
||||
FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE,
|
||||
};
|
||||
|
||||
let path_file = OpenOptions::new()
|
||||
.access_mode(0)
|
||||
.share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE)
|
||||
.open(path)?;
|
||||
Ok(windows_file_identity(&path_file)? == windows_file_identity(file)?)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
fn windows_file_identity(file: &File) -> std::io::Result<(u32, u64)> {
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use windows_sys::Win32::Storage::FileSystem::{
|
||||
BY_HANDLE_FILE_INFORMATION, GetFileInformationByHandle,
|
||||
};
|
||||
|
||||
let mut info = BY_HANDLE_FILE_INFORMATION::default();
|
||||
if unsafe { GetFileInformationByHandle(file.as_raw_handle() as _, &mut info) } == 0 {
|
||||
return Err(std::io::Error::last_os_error());
|
||||
}
|
||||
let index = (u64::from(info.nFileIndexHigh) << 32) | u64::from(info.nFileIndexLow);
|
||||
Ok((info.dwVolumeSerialNumber, index))
|
||||
}
|
||||
|
||||
#[cfg(not(any(unix, target_os = "windows")))]
|
||||
pub(crate) fn path_matches_file(path: &Path, file: &File) -> std::io::Result<bool> {
|
||||
let mut path_file = File::open(path)?;
|
||||
let mut leased_file = file.try_clone()?;
|
||||
Ok(FileFingerprint::capture_from(&mut path_file)?
|
||||
== FileFingerprint::capture_from(&mut leased_file)?)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum EditLeaseError {
|
||||
Locked(String),
|
||||
|
|
@ -153,6 +203,19 @@ impl EditLease {
|
|||
self.platform_warning.as_deref()
|
||||
}
|
||||
|
||||
/// Fingerprint through the lease's drawing handle.
|
||||
pub fn fingerprint(&mut self) -> std::io::Result<Option<FileFingerprint>> {
|
||||
self.drawing
|
||||
.as_mut()
|
||||
.map(FileFingerprint::capture_from)
|
||||
.transpose()
|
||||
}
|
||||
|
||||
/// Clone the leased drawing handle for a worker.
|
||||
pub fn reader(&self) -> std::io::Result<Option<File>> {
|
||||
self.drawing.as_ref().map(File::try_clone).transpose()
|
||||
}
|
||||
|
||||
/// Atomic save replaces the path with a new file object. Move the platform
|
||||
/// lock to that new object; the sidecar remains locked throughout.
|
||||
pub fn refresh_drawing_lock(&mut self, path: &Path) -> Result<(), EditLeaseError> {
|
||||
|
|
@ -255,6 +318,66 @@ mod tests {
|
|||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_lease_fingerprints_the_drawing_it_has_locked() {
|
||||
let path = unique_path("leased.dwg");
|
||||
std::fs::write(&path, b"drawing bytes").unwrap();
|
||||
let unlocked = FileFingerprint::capture(&path).unwrap();
|
||||
|
||||
let mut lease = EditLease::acquire(&path).unwrap();
|
||||
let leased = lease
|
||||
.fingerprint()
|
||||
.expect("the lease handle is readable")
|
||||
.expect("the lease holds a drawing handle");
|
||||
assert_eq!(unlocked, leased);
|
||||
|
||||
drop(lease);
|
||||
let _ = std::fs::remove_file(sidecar_path(&path));
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_lease_lends_a_reader_for_the_drawing_it_has_locked() {
|
||||
let path = unique_path("leased_reader.dwg");
|
||||
std::fs::write(&path, b"drawing bytes").unwrap();
|
||||
|
||||
let lease = EditLease::acquire(&path).unwrap();
|
||||
let mut reader = lease
|
||||
.reader()
|
||||
.expect("the lease handle is cloneable")
|
||||
.expect("the lease holds a drawing handle");
|
||||
let mut bytes = Vec::new();
|
||||
reader.read_to_end(&mut bytes).unwrap();
|
||||
assert_eq!(b"drawing bytes".as_slice(), bytes.as_slice());
|
||||
|
||||
drop(reader);
|
||||
drop(lease);
|
||||
let _ = std::fs::remove_file(sidecar_path(&path));
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_replaced_path_no_longer_matches_the_lease() {
|
||||
let path = unique_path("identity.dwg");
|
||||
let replacement = unique_path("replacement.dwg");
|
||||
std::fs::write(&path, b"old").unwrap();
|
||||
std::fs::write(&replacement, b"new").unwrap();
|
||||
|
||||
let lease = EditLease::acquire(&path).unwrap();
|
||||
let reader = lease
|
||||
.reader()
|
||||
.unwrap()
|
||||
.expect("the lease holds a drawing handle");
|
||||
std::fs::remove_file(&path).unwrap();
|
||||
std::fs::rename(&replacement, &path).unwrap();
|
||||
assert!(!path_matches_file(&path, &reader).unwrap());
|
||||
|
||||
drop(reader);
|
||||
drop(lease);
|
||||
let _ = std::fs::remove_file(sidecar_path(&path));
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fingerprint_detects_same_size_content_change() {
|
||||
let path = unique_path("fingerprint.dwg");
|
||||
|
|
|
|||
|
|
@ -1437,6 +1437,41 @@ mod save_failure_tests {
|
|||
acadrust::DxfVersion::AC1032,
|
||||
false,
|
||||
Some(expected),
|
||||
None,
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert!(error.externally_modified);
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"new");
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[test]
|
||||
fn external_path_replacement_prevents_atomic_replace() {
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"ocs_external_replace_{}_{}.dwg",
|
||||
std::process::id(),
|
||||
std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap()
|
||||
.as_nanos()
|
||||
));
|
||||
let replacement = path.with_extension("replacement.dwg");
|
||||
std::fs::write(&path, b"old").unwrap();
|
||||
let expected = super::edit_lock::FileFingerprint::capture(&path).unwrap();
|
||||
let reader = std::fs::File::open(&path).unwrap();
|
||||
std::fs::write(&replacement, b"new").unwrap();
|
||||
std::fs::remove_file(&path).unwrap();
|
||||
std::fs::rename(&replacement, &path).unwrap();
|
||||
|
||||
let error = super::save_owned_as_version_atomic(
|
||||
acadrust::CadDocument::new(),
|
||||
&path,
|
||||
acadrust::DxfVersion::AC1032,
|
||||
false,
|
||||
Some(expected),
|
||||
Some(reader),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
|
|
@ -1509,12 +1544,20 @@ pub fn save_owned_as_version_atomic(
|
|||
version: acadrust::DxfVersion,
|
||||
backup: bool,
|
||||
expected_fingerprint: Option<edit_lock::FileFingerprint>,
|
||||
verify_reader: Option<std::fs::File>,
|
||||
) -> Result<(), SaveFailure> {
|
||||
save_owned_as_version_inner(doc, path, version, backup, 0.0, move |path| {
|
||||
let Some(expected) = expected_fingerprint else {
|
||||
return Ok(());
|
||||
};
|
||||
match edit_lock::FileFingerprint::capture(path) {
|
||||
let current = match verify_reader {
|
||||
Some(mut file) => match edit_lock::path_matches_file(path, &file) {
|
||||
Ok(true) => edit_lock::FileFingerprint::capture_from(&mut file),
|
||||
Ok(false) | Err(_) => return Err(SaveFailure::externally_modified(path)),
|
||||
},
|
||||
None => edit_lock::FileFingerprint::capture(path),
|
||||
};
|
||||
match current {
|
||||
Ok(current) if current == expected => Ok(()),
|
||||
_ => Err(SaveFailure::externally_modified(path)),
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue