Unnamed repository; edit this file 'description' to name the repository.
Use a temporary file for writes (#9236)
Co-authored-by: Pascal Kuthe <[email protected]>
| -rw-r--r-- | Cargo.lock | 4 | ||||
| -rw-r--r-- | helix-stdx/Cargo.toml | 7 | ||||
| -rw-r--r-- | helix-stdx/src/faccess.rs | 459 | ||||
| -rw-r--r-- | helix-stdx/src/lib.rs | 1 | ||||
| -rw-r--r-- | helix-term/tests/test/commands/write.rs | 50 | ||||
| -rw-r--r-- | helix-term/tests/test/helpers.rs | 16 | ||||
| -rw-r--r-- | helix-term/tests/test/splits.rs | 20 | ||||
| -rw-r--r-- | helix-view/Cargo.toml | 2 | ||||
| -rw-r--r-- | helix-view/src/document.rs | 87 |
9 files changed, 569 insertions, 77 deletions
@@ -1405,12 +1405,15 @@ version = "24.3.0" name = "helix-stdx" version = "24.3.0" dependencies = [ + "bitflags 2.5.0", "dunce", "etcetera", "regex-cursor", "ropey", + "rustix", "tempfile", "which", + "windows-sys 0.52.0", ] [[package]] @@ -1515,6 +1518,7 @@ dependencies = [ "serde", "serde_json", "slotmap", + "tempfile", "tokio", "tokio-stream", "toml", diff --git a/helix-stdx/Cargo.toml b/helix-stdx/Cargo.toml index ed23f4e4..f17e0885 100644 --- a/helix-stdx/Cargo.toml +++ b/helix-stdx/Cargo.toml @@ -17,6 +17,13 @@ etcetera = "0.8" ropey = { version = "1.6.1", default-features = false } which = "6.0" regex-cursor = "0.1.4" +bitflags = "2.4" + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.52", features = ["Win32_Security", "Win32_Security_Authorization", "Win32_System_Threading"] } + +[target.'cfg(unix)'.dependencies] +rustix = { version = "0.38", features = ["fs"] } [dev-dependencies] tempfile = "3.10" diff --git a/helix-stdx/src/faccess.rs b/helix-stdx/src/faccess.rs new file mode 100644 index 00000000..0270c1f8 --- /dev/null +++ b/helix-stdx/src/faccess.rs @@ -0,0 +1,459 @@ +//! From <https://github.com/Freaky/faccess> + +use std::io; +use std::path::Path; + +use bitflags::bitflags; + +// Licensed under MIT from faccess +bitflags! { + /// Access mode flags for `access` function to test for. + pub struct AccessMode: u8 { + /// Path exists + const EXISTS = 0b0001; + /// Path can likely be read + const READ = 0b0010; + /// Path can likely be written to + const WRITE = 0b0100; + /// Path can likely be executed + const EXECUTE = 0b1000; + } +} + +#[cfg(unix)] +mod imp { + use super::*; + + use rustix::fs::Access; + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> { + let mut imode = Access::empty(); + + if mode.contains(AccessMode::EXISTS) { + imode |= Access::EXISTS; + } + + if mode.contains(AccessMode::READ) { + imode |= Access::READ_OK; + } + + if mode.contains(AccessMode::WRITE) { + imode |= Access::WRITE_OK; + } + + if mode.contains(AccessMode::EXECUTE) { + imode |= Access::EXEC_OK; + } + + rustix::fs::access(p, imode)?; + Ok(()) + } + + fn chown(p: &Path, uid: Option<u32>, gid: Option<u32>) -> io::Result<()> { + let uid = uid.map(|n| unsafe { rustix::fs::Uid::from_raw(n) }); + let gid = gid.map(|n| unsafe { rustix::fs::Gid::from_raw(n) }); + rustix::fs::chown(p, uid, gid)?; + Ok(()) + } + + pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { + let from_meta = std::fs::metadata(from)?; + let to_meta = std::fs::metadata(to)?; + let from_gid = from_meta.gid(); + let to_gid = to_meta.gid(); + + let mut perms = from_meta.permissions(); + perms.set_mode(perms.mode() & 0o0777); + if from_gid != to_gid && chown(to, None, Some(from_gid)).is_err() { + let new_perms = (perms.mode() & 0o0707) | ((perms.mode() & 0o07) << 3); + perms.set_mode(new_perms); + } + + std::fs::set_permissions(to, perms)?; + + Ok(()) + } +} + +// Licensed under MIT from faccess except for `chown`, `copy_metadata` and `is_acl_inherited` +#[cfg(windows)] +mod imp { + + use windows_sys::Win32::Foundation::{CloseHandle, LocalFree, ERROR_SUCCESS, HANDLE, PSID}; + use windows_sys::Win32::Security::Authorization::{ + GetNamedSecurityInfoW, SetNamedSecurityInfoW, SE_FILE_OBJECT, + }; + use windows_sys::Win32::Security::{ + AccessCheck, AclSizeInformation, GetAce, GetAclInformation, GetSidIdentifierAuthority, + ImpersonateSelf, IsValidAcl, IsValidSid, MapGenericMask, RevertToSelf, + SecurityImpersonation, ACCESS_ALLOWED_CALLBACK_ACE, ACL, ACL_SIZE_INFORMATION, + DACL_SECURITY_INFORMATION, GENERIC_MAPPING, GROUP_SECURITY_INFORMATION, INHERITED_ACE, + LABEL_SECURITY_INFORMATION, OBJECT_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, + PRIVILEGE_SET, PROTECTED_DACL_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, + SID_IDENTIFIER_AUTHORITY, TOKEN_DUPLICATE, TOKEN_QUERY, + }; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_ACCESS_RIGHTS, FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, + FILE_GENERIC_WRITE, + }; + use windows_sys::Win32::System::Threading::{GetCurrentThread, OpenThreadToken}; + + use super::*; + + use std::ffi::c_void; + + use std::os::windows::{ffi::OsStrExt, fs::OpenOptionsExt}; + + struct SecurityDescriptor { + sd: PSECURITY_DESCRIPTOR, + owner: PSID, + group: PSID, + dacl: *mut ACL, + } + + impl Drop for SecurityDescriptor { + fn drop(&mut self) { + if !self.sd.is_null() { + unsafe { + LocalFree(self.sd); + } + } + } + } + + impl SecurityDescriptor { + fn for_path(p: &Path) -> io::Result<SecurityDescriptor> { + let path = std::fs::canonicalize(p)?; + let pathos = path.into_os_string(); + let mut pathw: Vec<u16> = Vec::with_capacity(pathos.len() + 1); + pathw.extend(pathos.encode_wide()); + pathw.push(0); + + let mut sd = std::ptr::null_mut(); + let mut owner = std::ptr::null_mut(); + let mut group = std::ptr::null_mut(); + let mut dacl = std::ptr::null_mut(); + + let err = unsafe { + GetNamedSecurityInfoW( + pathw.as_ptr(), + SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION + | GROUP_SECURITY_INFORMATION + | DACL_SECURITY_INFORMATION + | LABEL_SECURITY_INFORMATION, + &mut owner, + &mut group, + &mut dacl, + std::ptr::null_mut(), + &mut sd, + ) + }; + + if err == ERROR_SUCCESS { + Ok(SecurityDescriptor { + sd, + owner, + group, + dacl, + }) + } else { + Err(io::Error::last_os_error()) + } + } + + fn is_acl_inherited(&self) -> bool { + let mut acl_info: ACL_SIZE_INFORMATION = unsafe { ::core::mem::zeroed() }; + let acl_info_ptr: *mut c_void = &mut acl_info as *mut _ as *mut c_void; + let mut ace: ACCESS_ALLOWED_CALLBACK_ACE = unsafe { ::core::mem::zeroed() }; + + unsafe { + GetAclInformation( + self.dacl, + acl_info_ptr, + std::mem::size_of_val(&acl_info) as u32, + AclSizeInformation, + ) + }; + + for i in 0..acl_info.AceCount { + let mut ptr = &mut ace as *mut _ as *mut c_void; + unsafe { GetAce(self.dacl, i, &mut ptr) }; + if (ace.Header.AceFlags as u32 & INHERITED_ACE) != 0 { + return true; + } + } + + false + } + + fn descriptor(&self) -> &PSECURITY_DESCRIPTOR { + &self.sd + } + + fn owner(&self) -> &PSID { + &self.owner + } + } + + struct ThreadToken(HANDLE); + impl Drop for ThreadToken { + fn drop(&mut self) { + unsafe { + CloseHandle(self.0); + } + } + } + + impl ThreadToken { + fn new() -> io::Result<Self> { + unsafe { + if ImpersonateSelf(SecurityImpersonation) == 0 { + return Err(io::Error::last_os_error()); + } + + let token: *mut HANDLE = std::ptr::null_mut(); + let err = + OpenThreadToken(GetCurrentThread(), TOKEN_DUPLICATE | TOKEN_QUERY, 0, token); + + RevertToSelf(); + + if err == 0 { + return Err(io::Error::last_os_error()); + } + + Ok(Self(*token)) + } + } + + fn as_handle(&self) -> &HANDLE { + &self.0 + } + } + + // Based roughly on Tcl's NativeAccess() + // https://github.com/tcltk/tcl/blob/2ee77587e4dc2150deb06b48f69db948b4ab0584/win/tclWinFile.c + fn eaccess(p: &Path, mut mode: FILE_ACCESS_RIGHTS) -> io::Result<()> { + let md = p.metadata()?; + + if !md.is_dir() { + // Read Only is ignored for directories + if mode & FILE_GENERIC_WRITE == FILE_GENERIC_WRITE && md.permissions().readonly() { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "File is read only", + )); + } + + // If it doesn't have the correct extension it isn't executable + if mode & FILE_GENERIC_EXECUTE == FILE_GENERIC_EXECUTE { + if let Some(ext) = p.extension().and_then(|s| s.to_str()) { + match ext { + "exe" | "com" | "bat" | "cmd" => (), + _ => { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "File not executable", + )) + } + } + } + } + + return std::fs::OpenOptions::new() + .access_mode(mode) + .open(p) + .map(|_| ()); + } + + let sd = SecurityDescriptor::for_path(p)?; + + // Unmapped Samba users are assigned a top level authority of 22 + // ACL tests are likely to be misleading + const SAMBA_UNMAPPED: SID_IDENTIFIER_AUTHORITY = SID_IDENTIFIER_AUTHORITY { + Value: [0, 0, 0, 0, 0, 22], + }; + unsafe { + let owner = sd.owner(); + if IsValidSid(*owner) != 0 + && (*GetSidIdentifierAuthority(*owner)).Value == SAMBA_UNMAPPED.Value + { + return Ok(()); + } + } + + let token = ThreadToken::new()?; + + let mut privileges: PRIVILEGE_SET = unsafe { std::mem::zeroed() }; + let mut granted_access: u32 = 0; + let mut privileges_length = std::mem::size_of::<PRIVILEGE_SET>() as u32; + let mut result = 0; + + let mut mapping = GENERIC_MAPPING { + GenericRead: FILE_GENERIC_READ, + GenericWrite: FILE_GENERIC_WRITE, + GenericExecute: FILE_GENERIC_EXECUTE, + GenericAll: FILE_ALL_ACCESS, + }; + + unsafe { MapGenericMask(&mut mode, &mut mapping) }; + + if unsafe { + AccessCheck( + *sd.descriptor(), + *token.as_handle(), + mode, + &mut mapping, + &mut privileges, + &mut privileges_length, + &mut granted_access, + &mut result, + ) + } != 0 + { + if result == 0 { + Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "Permission Denied", + )) + } else { + Ok(()) + } + } else { + Err(io::Error::last_os_error()) + } + } + + pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> { + let mut imode = 0; + + if mode.contains(AccessMode::READ) { + imode |= FILE_GENERIC_READ; + } + + if mode.contains(AccessMode::WRITE) { + imode |= FILE_GENERIC_WRITE; + } + + if mode.contains(AccessMode::EXECUTE) { + imode |= FILE_GENERIC_EXECUTE; + } + + if imode == 0 { + if p.exists() { + Ok(()) + } else { + Err(io::Error::new(io::ErrorKind::NotFound, "Not Found")) + } + } else { + eaccess(p, imode) + } + } + + fn chown(p: &Path, sd: SecurityDescriptor) -> io::Result<()> { + let path = std::fs::canonicalize(p)?; + let pathos = path.as_os_str(); + let mut pathw = Vec::with_capacity(pathos.len() + 1); + pathw.extend(pathos.encode_wide()); + pathw.push(0); + + let mut owner = std::ptr::null_mut(); + let mut group = std::ptr::null_mut(); + let mut dacl = std::ptr::null(); + + let mut si = OBJECT_SECURITY_INFORMATION::default(); + if unsafe { IsValidSid(sd.owner) } != 0 { + si |= OWNER_SECURITY_INFORMATION; + owner = sd.owner; + } + + if unsafe { IsValidSid(sd.group) } != 0 { + si |= GROUP_SECURITY_INFORMATION; + group = sd.group; + } + + if unsafe { IsValidAcl(sd.dacl) } != 0 { + si |= DACL_SECURITY_INFORMATION; + if !sd.is_acl_inherited() { + si |= PROTECTED_DACL_SECURITY_INFORMATION; + } + dacl = sd.dacl as *const _; + } + + let err = unsafe { + SetNamedSecurityInfoW( + pathw.as_ptr(), + SE_FILE_OBJECT, + si, + owner, + group, + dacl, + std::ptr::null(), + ) + }; + + if err == ERROR_SUCCESS { + Ok(()) + } else { + Err(io::Error::last_os_error()) + } + } + + pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { + let sd = SecurityDescriptor::for_path(from)?; + chown(to, sd)?; + + let meta = std::fs::metadata(from)?; + let perms = meta.permissions(); + + std::fs::set_permissions(to, perms)?; + + Ok(()) + } +} + +// Licensed under MIT from faccess except for `copy_metadata` +#[cfg(not(any(unix, windows)))] +mod imp { + use super::*; + + pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> { + if mode.contains(AccessMode::WRITE) { + if std::fs::metadata(p)?.permissions().readonly() { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "Path is read only", + )); + } else { + return Ok(()); + } + } + + if p.exists() { + Ok(()) + } else { + Err(io::Error::new(io::ErrorKind::NotFound, "Path not found")) + } + } + + pub fn copy_metadata(from: &path, to: &Path) -> io::Result<()> { + let meta = std::fs::metadata(from)?; + let perms = meta.permissions(); + std::fs::set_permissions(to, perms)?; + + Ok(()) + } +} + +pub fn readonly(p: &Path) -> bool { + match imp::access(p, AccessMode::WRITE) { + Ok(_) => false, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => false, + Err(_) => true, + } +} + +pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { + imp::copy_metadata(from, to) +} diff --git a/helix-stdx/src/lib.rs b/helix-stdx/src/lib.rs index 68fe3ec3..19602c20 100644 --- a/helix-stdx/src/lib.rs +++ b/helix-stdx/src/lib.rs @@ -1,3 +1,4 @@ pub mod env; +pub mod faccess; pub mod path; pub mod rope; diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index 7259b4e5..c9280bbd 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -95,7 +95,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { .await?; helpers::assert_file_has_content( - file.as_file_mut(), + &mut file, &LineFeedHandling::Native.apply(&RANGE.end().to_string()), )?; @@ -117,9 +117,7 @@ async fn test_write() -> anyhow::Result<()> { ) .await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - + reload_file(&mut file).unwrap(); let mut file_content = String::new(); file.as_file_mut().read_to_string(&mut file_content)?; @@ -148,12 +146,9 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { test_key_sequence(&mut app, Some(":x<ret>"), None, false).await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - - file.rewind()?; + reload_file(&mut file).unwrap(); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!("extremely important content", file_content); @@ -175,11 +170,10 @@ async fn test_write_quit() -> anyhow::Result<()> { ) .await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; + reload_file(&mut file).unwrap(); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!( LineFeedHandling::Native.apply("the gostak distims the doshes"), @@ -205,11 +199,9 @@ async fn test_write_concurrent() -> anyhow::Result<()> { test_key_sequence(&mut app, Some(&command), None, false).await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - + reload_file(&mut file).unwrap(); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!( LineFeedHandling::Native.apply(&RANGE.end().to_string()), file_content @@ -279,7 +271,7 @@ async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { ) .await?; - helpers::assert_file_has_content(file.as_file_mut(), &LineFeedHandling::Native.apply("hello"))?; + helpers::assert_file_has_content(&mut file, &LineFeedHandling::Native.apply("hello"))?; Ok(()) } @@ -324,7 +316,7 @@ async fn test_write_auto_format_fails_still_writes() -> anyhow::Result<()> { test_key_sequences(&mut app, vec![(Some(":w<ret>"), None)], false).await?; // file still saves - helpers::assert_file_has_content(file.as_file_mut(), "let foo = 0;\n")?; + helpers::assert_file_has_content(&mut file, "let foo = 0;\n")?; Ok(()) } @@ -363,12 +355,12 @@ async fn test_write_new_path() -> anyhow::Result<()> { .await?; helpers::assert_file_has_content( - file1.as_file_mut(), + &mut file1, &LineFeedHandling::Native.apply("i can eat glass, it will not hurt me\n"), )?; helpers::assert_file_has_content( - file2.as_file_mut(), + &mut file2, &LineFeedHandling::Native.apply("i can eat glass, it will not hurt me\n"), )?; @@ -439,7 +431,7 @@ async fn test_write_insert_final_newline_added_if_missing() -> anyhow::Result<() test_key_sequence(&mut app, Some(":w<ret>"), None, false).await?; helpers::assert_file_has_content( - file.as_file_mut(), + &mut file, &LineFeedHandling::Native.apply("have you tried chamomile tea?\n"), )?; @@ -457,7 +449,7 @@ async fn test_write_insert_final_newline_unchanged_if_not_missing() -> anyhow::R test_key_sequence(&mut app, Some(":w<ret>"), None, false).await?; helpers::assert_file_has_content( - file.as_file_mut(), + &mut file, &LineFeedHandling::Native.apply("ten minutes, please\n"), )?; @@ -481,10 +473,8 @@ async fn test_write_insert_final_newline_unchanged_if_missing_and_false() -> any test_key_sequence(&mut app, Some(":w<ret>"), None, false).await?; - helpers::assert_file_has_content( - file.as_file_mut(), - "the quiet rain continued through the night", - )?; + reload_file(&mut file).unwrap(); + helpers::assert_file_has_content(&mut file, "the quiet rain continued through the night")?; Ok(()) } @@ -510,12 +500,12 @@ async fn test_write_all_insert_final_newline_add_if_missing_and_modified() -> an .await?; helpers::assert_file_has_content( - file1.as_file_mut(), + &mut file1, &LineFeedHandling::Native.apply("we don't serve time travelers here\n"), )?; helpers::assert_file_has_content( - file2.as_file_mut(), + &mut file2, &LineFeedHandling::Native.apply("a time traveler walks into a bar\n"), )?; @@ -534,7 +524,7 @@ async fn test_write_all_insert_final_newline_do_not_add_if_unmodified() -> anyho test_key_sequence(&mut app, Some(":wa<ret>"), None, false).await?; - helpers::assert_file_has_content(file.as_file_mut(), "i lost on Jeopardy!")?; + helpers::assert_file_has_content(&mut file, "i lost on Jeopardy!")?; Ok(()) } @@ -560,7 +550,7 @@ async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> { ) .await?; - file.rewind()?; + reload_file(&mut file).unwrap(); let mut new_file_content: Vec<u8> = Vec::new(); file.read_to_end(&mut new_file_content)?; diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 41eab427..d7205b0c 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -1,5 +1,4 @@ use std::{ - fs::File, io::{Read, Write}, mem::replace, path::PathBuf, @@ -390,9 +389,8 @@ pub async fn run_event_loop_until_idle(app: &mut Application) { 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()?; +pub fn assert_file_has_content(file: &mut NamedTempFile, content: &str) -> anyhow::Result<()> { + reload_file(file)?; let mut file_content = String::new(); file.read_to_string(&mut file_content)?; @@ -406,3 +404,13 @@ pub fn assert_status_not_error(editor: &Editor) { assert_ne!(&Severity::Error, sev); } } + +pub fn reload_file(file: &mut NamedTempFile) -> anyhow::Result<()> { + let path = file.path(); + let f = std::fs::OpenOptions::new() + .write(true) + .read(true) + .open(&path)?; + *file.as_file_mut() = f; + Ok(()) +} diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs index f19e3004..3ba5a504 100644 --- a/helix-term/tests/test/splits.rs +++ b/helix-term/tests/test/splits.rs @@ -62,18 +62,9 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { ) .await?; - helpers::assert_file_has_content( - file1.as_file_mut(), - &LineFeedHandling::Native.apply("hello1"), - )?; - helpers::assert_file_has_content( - file2.as_file_mut(), - &LineFeedHandling::Native.apply("hello2"), - )?; - helpers::assert_file_has_content( - file3.as_file_mut(), - &LineFeedHandling::Native.apply("hello3"), - )?; + helpers::assert_file_has_content(&mut file1, &LineFeedHandling::Native.apply("hello1"))?; + helpers::assert_file_has_content(&mut file2, &LineFeedHandling::Native.apply("hello2"))?; + helpers::assert_file_has_content(&mut file3, &LineFeedHandling::Native.apply("hello3"))?; Ok(()) } @@ -131,10 +122,7 @@ async fn test_split_write_quit_same_file() -> anyhow::Result<()> { ) .await?; - helpers::assert_file_has_content( - file.as_file_mut(), - &LineFeedHandling::Native.apply("hello\ngoodbye"), - )?; + helpers::assert_file_has_content(&mut file, &LineFeedHandling::Native.apply("hello\ngoodbye"))?; Ok(()) } diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index f12185c8..be0edc96 100644 --- a/helix-view/Cargo.toml +++ b/helix-view/Cargo.toml @@ -27,6 +27,8 @@ bitflags = "2.5" anyhow = "1" crossterm = { version = "0.27", optional = true } +tempfile = "3.9" + # Conversion traits once_cell = "1.19" url = "2.5.0" diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index da68a67f..0c3a75f1 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -10,6 +10,7 @@ use helix_core::encoding::Encoding; use helix_core::syntax::{Highlight, LanguageServerFeature}; use helix_core::text_annotations::{InlineAnnotation, Overlay}; use helix_lsp::util::lsp_pos_to_pos; +use helix_stdx::faccess::{copy_metadata, readonly}; use helix_vcs::{DiffHandle, DiffProviderRegistry}; use ::parking_lot::Mutex; @@ -870,7 +871,7 @@ impl Document { // We encode the file according to the `Document`'s encoding. let future = async move { - use tokio::{fs, fs::File}; + use tokio::fs; if let Some(parent) = path.parent() { // TODO: display a prompt asking the user if the directories should be created if !parent.exists() { @@ -893,8 +894,63 @@ impl Document { } } - let mut file = File::create(&path).await?; - to_writer(&mut file, encoding_with_bom_info, &text).await?; + if readonly(&path) { + bail!(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "Path is read only" + )); + } + let backup = if path.exists() { + let path_ = 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 + // since the path doesn't exist yet, we just want + // the path + tokio::task::spawn_blocking(move || -> Option<PathBuf> { + tempfile::Builder::new() + .prefix(path_.file_name()?) + .suffix(".bck") + .make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup)) + .ok()? + .into_temp_path() + .keep() + .ok() + }) + .await + .ok() + .flatten() + } else { + None + }; + + let write_result: anyhow::Result<_> = async { + let mut dst = tokio::fs::File::create(&path).await?; + to_writer(&mut dst, encoding_with_bom_info, &text).await?; + Ok(()) + } + .await; + + if let Some(backup) = backup { + if write_result.is_err() { + // restore backup + let _ = tokio::fs::rename(&backup, &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_) + .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}")); + }) + .await; + } + } + + write_result?; let event = DocumentSavedEvent { revision: current_rev, @@ -955,35 +1011,12 @@ impl Document { } } - #[cfg(unix)] // Detect if the file is readonly and change the readonly field if necessary (unix only) pub fn detect_readonly(&mut self) { - use rustix::fs::{access, Access}; // Allows setting the flag for files the user cannot modify, like root files self.readonly = match &self.path { None => false, - Some(p) => match access(p, Access::WRITE_OK) { - Ok(_) => false, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => false, - Err(_) => true, - }, - }; - } - - #[cfg(not(unix))] - // Detect if the file is readonly and change the readonly field if necessary (non-unix os) - pub fn detect_readonly(&mut self) { - // TODO Use the Windows' function `CreateFileW` to check if a file is readonly - // Discussion: https://github.com/helix-editor/helix/pull/7740#issuecomment-1656806459 - // Vim implementation: https://github.com/vim/vim/blob/4c0089d696b8d1d5dc40568f25ea5738fa5bbffb/src/os_win32.c#L7665 - // Windows binding: https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/Storage/FileSystem/fn.CreateFileW.html - self.readonly = match &self.path { - None => false, - Some(p) => match std::fs::metadata(p) { - Err(err) if err.kind() == std::io::ErrorKind::NotFound => false, - Err(_) => false, - Ok(metadata) => metadata.permissions().readonly(), - }, + Some(p) => readonly(p), }; } |