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 <gustavo@noronha.dev.br> Co-authored-by: LeoniePhiline <22329650+LeoniePhiline@users.noreply.github.com> Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
This commit is contained in:
parent
00ecc556a8
commit
f386ff795d
4 changed files with 65 additions and 4 deletions
|
@ -67,7 +67,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> {
|
|||
const RANGE: RangeInclusive<i32> = 1..=1000;
|
||||
|
||||
for i in RANGE {
|
||||
let cmd = format!("%c{}<esc>:w<ret>", i);
|
||||
let cmd = format!("%c{}<esc>:w!<ret>", i);
|
||||
command.push_str(&cmd);
|
||||
}
|
||||
|
||||
|
|
|
@ -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()?;
|
||||
|
|
|
@ -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<ret>"), 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{}<esc>:w<ret>", i);
|
||||
let cmd = format!("%c{}<esc>:w!<ret>", i);
|
||||
command.push_str(&cmd);
|
||||
}
|
||||
|
||||
|
|
|
@ -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<Transaction>,
|
||||
|
||||
// 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.
|
||||
|
|
Loading…
Reference in a new issue