Skip to content

Commit

Permalink
Ensure that temporary files written have the same metadata as the ori…
Browse files Browse the repository at this point in the history 
…ginal
  • Loading branch information
@kirawi
kirawi committed Nov 15, 2024
1 parent b53dafe commit c40a34c
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 36 deletions.
103 changes: 84 additions & 19 deletions helix-stdx/src/faccess.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! From <https://github.com/Freaky/faccess>

use std::fs::File;
use std::io;
use std::path::Path;

Expand All @@ -24,7 +25,10 @@ bitflags! {
mod imp {
use super::*;

use rustix::fs::Access;
use rustix::{
fd::AsFd,
fs::{Access, OpenOptionsExt},
};
use std::os::unix::fs::{MetadataExt, PermissionsExt};

pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> {
Expand Down Expand Up @@ -57,6 +61,13 @@ mod imp {
Ok(())
}

fn fchown(fd: impl AsFd, 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::fchown(fd, 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)?;
Expand All @@ -79,15 +90,34 @@ mod imp {
let metadata = p.metadata()?;
Ok(metadata.nlink())
}

pub fn create_copy_mode(from: &Path, to: &Path) -> io::Result<File> {
let from_meta = std::fs::metadata(from)?;
let mode = from_meta.permissions().mode();
let file = std::fs::OpenOptions::new()
.mode(mode)
.read(true)
.write(true)
.create_new(true)
.open(to)?;

// Change ownership
let from_meta = std::fs::metadata(from)?;
let uid = from_meta.uid();
let gid = from_meta.gid();
fchown(file.as_fd(), Some(uid), Some(gid))?;
Ok(file)
}
}

// 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};
use windows_sys::Win32::Foundation::{
CloseHandle, LocalFree, ERROR_SUCCESS, GENERIC_READ, GENERIC_WRITE, HANDLE,
};
use windows_sys::Win32::Security::Authorization::{
GetNamedSecurityInfoW, SetNamedSecurityInfoW, SE_FILE_OBJECT,
GetNamedSecurityInfoW, SetSecurityInfo, SE_FILE_OBJECT,
};
use windows_sys::Win32::Security::{
AccessCheck, AclSizeInformation, GetAce, GetAclInformation, GetSidIdentifierAuthority,
Expand All @@ -100,7 +130,8 @@ mod imp {
};
use windows_sys::Win32::Storage::FileSystem::{
GetFileInformationByHandle, BY_HANDLE_FILE_INFORMATION, FILE_ACCESS_RIGHTS,
FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, FILE_GENERIC_WRITE,
FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, FILE_GENERIC_WRITE, WRITE_DAC,
WRITE_OWNER,
};
use windows_sys::Win32::System::Threading::{GetCurrentThread, OpenThreadToken};

Expand All @@ -110,6 +141,7 @@ mod imp {

use std::os::windows::{ffi::OsStrExt, fs::OpenOptionsExt, io::AsRawHandle};

// Licensed under MIT from faccess
struct SecurityDescriptor {
sd: PSECURITY_DESCRIPTOR,
owner: PSID,
Expand All @@ -128,6 +160,7 @@ mod imp {
}

impl SecurityDescriptor {
// Licensed under MIT from faccess
fn for_path(p: &Path) -> io::Result<SecurityDescriptor> {
let path = std::fs::canonicalize(p)?;
let pathos = path.into_os_string();
Expand Down Expand Up @@ -202,6 +235,7 @@ mod imp {
}
}

// Licensed under MIT from faccess
struct ThreadToken(HANDLE);
impl Drop for ThreadToken {
fn drop(&mut self) {
Expand All @@ -211,6 +245,7 @@ mod imp {
}
}

// Licensed under MIT from faccess
impl ThreadToken {
fn new() -> io::Result<Self> {
unsafe {
Expand All @@ -237,6 +272,7 @@ mod imp {
}
}

// Licensed under MIT from faccess
// 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<()> {
Expand Down Expand Up @@ -330,6 +366,7 @@ mod imp {
}
}

// Licensed under MIT from faccess
pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> {
let mut imode = 0;

Expand All @@ -356,13 +393,8 @@ mod imp {
}
}

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);

// SAFETY: It is the caller's responsibility to close the handle
fn chown(handle: HANDLE, sd: SecurityDescriptor) -> io::Result<()> {
let mut owner = std::ptr::null_mut();
let mut group = std::ptr::null_mut();
let mut dacl = std::ptr::null();
Expand All @@ -387,8 +419,8 @@ mod imp {
}

let err = unsafe {
SetNamedSecurityInfoW(
pathw.as_ptr(),
SetSecurityInfo(
handle,
SE_FILE_OBJECT,
si,
owner,
Expand All @@ -407,7 +439,11 @@ mod imp {

pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> {
let sd = SecurityDescriptor::for_path(from)?;
chown(to, sd)?;
let to_file = std::fs::OpenOptions::new()
.read(true)
.access_mode(GENERIC_READ | GENERIC_WRITE | WRITE_OWNER | WRITE_DAC)
.open(to)?;
chown(to_file.as_raw_handle(), sd)?;

let meta = std::fs::metadata(from)?;
let perms = meta.permissions();
Expand All @@ -417,17 +453,42 @@ mod imp {
Ok(())
}

pub fn hardlink_count(p: &Path) -> std::io::Result<u64> {
let file = std::fs::File::open(p)?;
fn file_info(p: &Path) -> io::Result<BY_HANDLE_FILE_INFORMATION> {
let file = File::open(p)?;
let handle = file.as_raw_handle();
let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() };

if unsafe { GetFileInformationByHandle(handle, &mut info) } == 0 {
Err(std::io::Error::last_os_error())
Err(io::Error::last_os_error())
} else {
Ok(info.nNumberOfLinks as u64)
Ok(info)
}
}

pub fn hardlink_count(p: &Path) -> io::Result<u64> {
let n = file_info(p)?.nNumberOfLinks as u64;
Ok(n)
}

pub fn create_copy_mode(from: &Path, to: &Path) -> io::Result<File> {
let sd = SecurityDescriptor::for_path(from)?;

// read/write still need to be set to true or `create_new` returns an error
let to_file = std::fs::OpenOptions::new()
.read(true)
.write(true)
.access_mode(GENERIC_READ | GENERIC_WRITE | WRITE_OWNER | WRITE_DAC)
.create_new(true)
.open(to)?;

// Necessary because `security_attributes` is not exposed: https://github.com/rust-lang/libs-team/issues/314
chown(to_file.as_raw_handle(), sd)?;

let meta = std::fs::metadata(from)?;
let perms = meta.permissions();
std::fs::set_permissions(to, perms)?;
Ok(to_file)
}
}

// Licensed under MIT from faccess except for `copy_metadata`
Expand Down Expand Up @@ -478,3 +539,7 @@ pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> {
pub fn hardlink_count(p: &Path) -> io::Result<u64> {
imp::hardlink_count(p)
}

pub fn create_copy_mode(from: &Path, to: &Path) -> io::Result<File> {
imp::create_copy_mode(from, to)
}
42 changes: 25 additions & 17 deletions helix-view/src/document.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +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_stdx::faccess::{copy_metadata, create_copy_mode, readonly};
use helix_vcs::{DiffHandle, DiffProviderRegistry};
use thiserror;

Expand Down Expand Up @@ -936,7 +936,7 @@ impl Document {
}

// Assume it is a hardlink to prevent data loss if the metadata cant be read (e.g. on certain Windows configurations)
let is_hardlink = helix_stdx::faccess::hardlink_count(&write_path).unwrap_or(2) > 1;
let backup_copy = helix_stdx::faccess::hardlink_count(&write_path).unwrap_or(2) > 1;
let backup = if path.exists() {
let path_ = write_path.clone();
// hacks: we use tempfile to handle the complex task of creating
Expand All @@ -948,19 +948,16 @@ impl Document {
let mut builder = tempfile::Builder::new();
builder.prefix(path_.file_name()?).suffix(".bck");

let backup_path = if is_hardlink {
builder
.make_in(path_.parent()?, |backup| std::fs::copy(&path_, backup))
.ok()?
.into_temp_path()
let backup_path = if backup_copy {
builder.make_in(path_.parent()?, |backup| {
let _ = std::fs::copy(&path_, backup)?;
copy_metadata(&path_, backup)
})
} else {
builder
.make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup))
.ok()?
.into_temp_path()
builder.make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup))
};

backup_path.keep().ok()
backup_path.ok()?.into_temp_path().keep().ok()
})
.await
.ok()
Expand All @@ -970,7 +967,20 @@ impl Document {
};

let write_result: anyhow::Result<_> = async {
let mut dst = tokio::fs::File::create(&write_path).await?;
// Create the new file with the same permissions as the original file
let mut dst = if backup.is_some() && !backup_copy {
let from = backup.clone().unwrap();
let write_path = write_path.clone();

// This can fail if the file already exists, but since we moved the original file, it should be fine
let file = tokio::task::spawn_blocking(move || {
create_copy_mode(from.as_path(), write_path.as_path())
})
.await??;
fs::File::from_std(file)
} else {
tokio::fs::File::create(&write_path).await.unwrap()
};
to_writer(&mut dst, encoding_with_bom_info, &text).await?;
dst.sync_all().await?;
Ok(())
Expand All @@ -983,7 +993,7 @@ impl Document {
};

if let Some(backup) = backup {
if is_hardlink {
if backup_copy {
let mut delete = true;
if write_result.is_err() {
// Restore backup
Expand All @@ -1005,10 +1015,8 @@ impl Document {
.await
.map_err(|e| log::error!("Failed to restore backup on write failure: {e}"));
} else {
// copy metadata and delete backup
// delete backup
let _ = tokio::task::spawn_blocking(move || {
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}"));
})
Expand Down

0 comments on commit c40a34c

Please sign in to comment.