From 94aeda2a4672a925e199ada17dfd42416b2ba0ca Mon Sep 17 00:00:00 2001 From: Kirawi <67773714+kirawi@users.noreply.github.com> Date: Thu, 11 Apr 2024 21:49:16 -0400 Subject: [PATCH] Read symlink when writing files (#10339) Co-authored-by: Pascal Kuthe --- helix-term/tests/test/commands/write.rs | 75 +++++++++++++++++++++++++ helix-term/tests/test/helpers.rs | 12 ++++ helix-view/src/document.rs | 14 +++-- 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index c9280bbd49d5..350f34aab27e 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -529,6 +529,81 @@ async fn test_write_all_insert_final_newline_do_not_add_if_unmodified() -> anyho Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn test_symlink_write() -> anyhow::Result<()> { + #[cfg(unix)] + use std::os::unix::fs::symlink; + #[cfg(not(unix))] + use std::os::windows::fs::symlink_file as symlink; + + let dir = tempfile::tempdir()?; + + let mut file = tempfile::NamedTempFile::new_in(&dir)?; + let symlink_path = dir.path().join("linked"); + symlink(file.path(), &symlink_path)?; + + let mut app = helpers::AppBuilder::new() + .with_file(&symlink_path, None) + .build()?; + + test_key_sequence( + &mut app, + Some("ithe gostak distims the doshes:w"), + None, + false, + ) + .await?; + + reload_file(&mut file).unwrap(); + let mut file_content = String::new(); + file.as_file_mut().read_to_string(&mut file_content)?; + + assert_eq!( + LineFeedHandling::Native.apply("the gostak distims the doshes"), + file_content + ); + assert!(symlink_path.is_symlink()); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_symlink_write_fail() -> anyhow::Result<()> { + #[cfg(unix)] + use std::os::unix::fs::symlink; + #[cfg(not(unix))] + use std::os::windows::fs::symlink_file as symlink; + + let dir = tempfile::tempdir()?; + + let file = helpers::new_readonly_tempfile_in_dir(&dir)?; + let symlink_path = dir.path().join("linked"); + symlink(file.path(), &symlink_path)?; + + let mut app = helpers::AppBuilder::new() + .with_file(&symlink_path, None) + .build()?; + + test_key_sequence( + &mut app, + Some("ihello:wq"), + Some(&|app| { + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(Some(&path::normalize(&symlink_path)), doc.path()); + assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); + }), + false, + ) + .await?; + + assert!(symlink_path.is_symlink()); + + Ok(()) +} + async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index d7205b0c046a..70b3f4022c00 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -305,6 +305,18 @@ pub fn new_readonly_tempfile() -> anyhow::Result { Ok(file) } +/// Creates a new temporary file in the directory that is set to read only. Useful for +/// testing write failures. +pub fn new_readonly_tempfile_in_dir( + dir: impl AsRef, +) -> anyhow::Result { + let mut file = tempfile::NamedTempFile::new_in(dir)?; + let metadata = file.as_file().metadata()?; + let mut perms = metadata.permissions(); + perms.set_readonly(true); + file.as_file_mut().set_permissions(perms)?; + Ok(file) +} pub struct AppBuilder { args: Args, config: Config, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index f26ba8b97851..d44b4240c7f2 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -893,15 +893,18 @@ impl Document { } } } + let write_path = tokio::fs::read_link(&path) + .await + .unwrap_or_else(|_| path.clone()); - if readonly(&path) { + if readonly(&write_path) { bail!(std::io::Error::new( std::io::ErrorKind::PermissionDenied, "Path is read only" )); } let backup = if path.exists() { - let path_ = path.clone(); + let path_ = write_path.clone(); // hacks: we use tempfile to handle the complex task of creating // non clobbered temporary path for us we don't want // the whole automatically delete path on drop thing @@ -925,7 +928,7 @@ impl Document { }; let write_result: anyhow::Result<_> = async { - let mut dst = tokio::fs::File::create(&path).await?; + let mut dst = tokio::fs::File::create(&write_path).await?; to_writer(&mut dst, encoding_with_bom_info, &text).await?; Ok(()) } @@ -934,14 +937,13 @@ impl Document { if let Some(backup) = backup { if write_result.is_err() { // restore backup - let _ = tokio::fs::rename(&backup, &path) + let _ = tokio::fs::rename(&backup, &write_path) .await .map_err(|e| log::error!("Failed to restore backup on write failure: {e}")); } else { // copy metadata and delete backup - let path_ = path.clone(); let _ = tokio::task::spawn_blocking(move || { - let _ = copy_metadata(&backup, &path_) + let _ = copy_metadata(&backup, &write_path) .map_err(|e| log::error!("Failed to copy metadata on write: {e}")); let _ = std::fs::remove_file(backup) .map_err(|e| log::error!("Failed to remove backup file on write: {e}"));