From f386ff795d4833cce02d57de921999284aadded3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Delafargue?= Date: Wed, 8 Feb 2023 17:09:19 +0100 Subject: [PATCH] Check for external file modifications when writing (#5805) `:write` and other file-saving commands now check the file modification time before writing to protect against overwriting external changes. Co-authored-by: Gustavo Noronha Silva Co-authored-by: LeoniePhiline <22329650+LeoniePhiline@users.noreply.github.com> Co-authored-by: Pascal Kuthe --- helix-term/tests/test/commands.rs | 2 +- helix-term/tests/test/helpers.rs | 6 ++++++ helix-term/tests/test/write.rs | 36 +++++++++++++++++++++++++++++-- helix-view/src/document.rs | 25 ++++++++++++++++++++- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index da2e020ef..2ca9e3957 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -67,7 +67,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { const RANGE: RangeInclusive = 1..=1000; for i in RANGE { - let cmd = format!("%c{}:w", i); + let cmd = format!("%c{}:w!", i); command.push_str(&cmd); } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 8755b60f0..fb12ef12c 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -319,6 +319,12 @@ impl AppBuilder { } } +pub async fn run_event_loop_until_idle(app: &mut Application) { + let (_, rx) = tokio::sync::mpsc::unbounded_channel(); + let mut rx_stream = UnboundedReceiverStream::new(rx); + app.event_loop_until_idle(&mut rx_stream).await; +} + pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result<()> { file.flush()?; file.sync_all()?; diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index bbf14fc20..81459b2fe 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -1,5 +1,5 @@ use std::{ - io::{Read, Write}, + io::{Read, Seek, SeekFrom, Write}, ops::RangeInclusive, }; @@ -37,6 +37,38 @@ async fn test_write() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn test_overwrite_protection() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + + helpers::run_event_loop_until_idle(&mut app).await; + + file.as_file_mut() + .write_all(helpers::platform_line("extremely important content").as_bytes())?; + + file.as_file_mut().flush()?; + file.as_file_mut().sync_all()?; + + test_key_sequence(&mut app, Some(":x"), None, false).await?; + + file.as_file_mut().flush()?; + file.as_file_mut().sync_all()?; + + file.seek(SeekFrom::Start(0))?; + let mut file_content = String::new(); + file.as_file_mut().read_to_string(&mut file_content)?; + + assert_eq!( + helpers::platform_line("extremely important content"), + file_content + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn test_write_quit() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; @@ -76,7 +108,7 @@ async fn test_write_concurrent() -> anyhow::Result<()> { .build()?; for i in RANGE { - let cmd = format!("%c{}:w", i); + let cmd = format!("%c{}:w!", i); command.push_str(&cmd); } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 798b54006..d308d013f 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -19,6 +19,7 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; +use std::time::SystemTime; use helix_core::{ encoding, @@ -135,6 +136,10 @@ pub struct Document { pub savepoint: Option, + // Last time we wrote to the file. This will carry the time the file was last opened if there + // were no saves. + last_saved_time: SystemTime, + last_saved_revision: usize, version: i32, // should be usize? pub(crate) modified_since_accessed: bool, @@ -160,6 +165,7 @@ impl fmt::Debug for Document { .field("changes", &self.changes) .field("old_state", &self.old_state) // .field("history", &self.history) + .field("last_saved_time", &self.last_saved_time) .field("last_saved_revision", &self.last_saved_revision) .field("version", &self.version) .field("modified_since_accessed", &self.modified_since_accessed) @@ -382,6 +388,7 @@ impl Document { version: 0, history: Cell::new(History::default()), savepoint: None, + last_saved_time: SystemTime::now(), last_saved_revision: 0, modified_since_accessed: false, language_server: None, @@ -577,9 +584,11 @@ impl Document { let encoding = self.encoding; + let last_saved_time = self.last_saved_time; + // We encode the file according to the `Document`'s encoding. let future = async move { - use tokio::fs::File; + use tokio::{fs, fs::File}; if let Some(parent) = path.parent() { // TODO: display a prompt asking the user if the directories should be created if !parent.exists() { @@ -591,6 +600,17 @@ impl Document { } } + // Protect against overwriting changes made externally + if !force { + if let Ok(metadata) = fs::metadata(&path).await { + if let Ok(mtime) = metadata.modified() { + if last_saved_time < mtime { + bail!("file modified by an external process, use :w! to overwrite"); + } + } + } + } + let mut file = File::create(&path).await?; to_writer(&mut file, encoding, &text).await?; @@ -668,6 +688,8 @@ impl Document { self.append_changes_to_history(view); self.reset_modified(); + self.last_saved_time = SystemTime::now(); + self.detect_indent_and_line_ending(); match provider_registry.get_diff_base(&path) { @@ -1016,6 +1038,7 @@ impl Document { rev ); self.last_saved_revision = rev; + self.last_saved_time = SystemTime::now(); } /// Get the document's latest saved revision.