From 84442334e042b23972dd47572dc559f92d764ede Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 28 Dec 2024 21:03:55 -0500 Subject: [PATCH 1/3] fix(windows): stat - close file handle on error --- ext/fs/std_fs.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/ext/fs/std_fs.rs b/ext/fs/std_fs.rs index 86ad2131601bab..fec39b1f2f36cd 100644 --- a/ext/fs/std_fs.rs +++ b/ext/fs/std_fs.rs @@ -61,7 +61,7 @@ impl FileSystem for RealFs { umask(Mode::from_bits_truncate(mask as mode_t)) } else { // If no mask provided, we query the current. Requires two syscalls. - let prev = umask(Mode::from_bits_truncate(0o777)); + let prev = umask(Mode::from_bits_truncate(0)); let _ = umask(prev); prev }; @@ -802,6 +802,17 @@ fn stat_extra( use winapi::um::winnt::FILE_SHARE_READ; use winapi::um::winnt::FILE_SHARE_WRITE; + struct WinHandle(winapi::shared::ntdef::HANDLE); + + impl Drop for WinHandle { + fn drop(&mut self) { + // SAFETY: winapi call + unsafe { + CloseHandle(self.0); + } + } + } + unsafe fn get_dev( handle: winapi::shared::ntdef::HANDLE, ) -> std::io::Result { @@ -883,11 +894,12 @@ fn stat_extra( if file_handle == INVALID_HANDLE_VALUE { return Err(std::io::Error::last_os_error().into()); } + let file_handle = WinHandle(file_handle); - let result = get_dev(file_handle); + let result = get_dev(file_handle.0); fsstat.dev = result?; - if let Ok(file_info) = query_file_information(file_handle) { + if let Ok(file_info) = query_file_information(file_handle.0) { fsstat.ctime = Some(windows_time_to_unix_time_msec( &file_info.BasicInformation.ChangeTime, ) as u64); @@ -924,7 +936,6 @@ fn stat_extra( } } - CloseHandle(file_handle); Ok(()) } } From e9f36e9c483d66f56f70f3d5ef1e8e293b5d85b2 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 28 Dec 2024 21:04:50 -0500 Subject: [PATCH 2/3] combine statement --- ext/fs/std_fs.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/fs/std_fs.rs b/ext/fs/std_fs.rs index fec39b1f2f36cd..821f95430930b6 100644 --- a/ext/fs/std_fs.rs +++ b/ext/fs/std_fs.rs @@ -896,8 +896,7 @@ fn stat_extra( } let file_handle = WinHandle(file_handle); - let result = get_dev(file_handle.0); - fsstat.dev = result?; + fsstat.dev = get_dev(file_handle.0)?; if let Ok(file_info) = query_file_information(file_handle.0) { fsstat.ctime = Some(windows_time_to_unix_time_msec( From aef54836005b68e2bd6a3c01f04f58a48a033e80 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 28 Dec 2024 21:55:35 -0500 Subject: [PATCH 3/3] only open file once --- ext/fs/std_fs.rs | 74 ++++++++++++++---------------------------------- 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/ext/fs/std_fs.rs b/ext/fs/std_fs.rs index 821f95430930b6..0badc7e2adfd35 100644 --- a/ext/fs/std_fs.rs +++ b/ext/fs/std_fs.rs @@ -757,11 +757,16 @@ fn stat(path: &Path) -> FsResult { #[cfg(windows)] fn stat(path: &Path) -> FsResult { - let metadata = fs::metadata(path)?; - let mut fsstat = FsStat::from_std(metadata); + use std::os::windows::fs::OpenOptionsExt; use winapi::um::winbase::FILE_FLAG_BACKUP_SEMANTICS; - let path = path.canonicalize()?; - stat_extra(&mut fsstat, &path, FILE_FLAG_BACKUP_SEMANTICS)?; + + let mut opts = fs::OpenOptions::new(); + opts.access_mode(0); // no read or write + opts.custom_flags(FILE_FLAG_BACKUP_SEMANTICS); + let file = opts.open(path)?; + let metadata = file.metadata()?; + let mut fsstat = FsStat::from_std(metadata); + stat_extra(&file, &mut fsstat)?; Ok(fsstat) } @@ -773,45 +778,24 @@ fn lstat(path: &Path) -> FsResult { #[cfg(windows)] fn lstat(path: &Path) -> FsResult { + use std::os::windows::fs::OpenOptionsExt; + use winapi::um::winbase::FILE_FLAG_BACKUP_SEMANTICS; use winapi::um::winbase::FILE_FLAG_OPEN_REPARSE_POINT; - let metadata = fs::symlink_metadata(path)?; + let mut opts = fs::OpenOptions::new(); + opts.access_mode(0); // no read or write + opts.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT); + let file = opts.open(path)?; + let metadata = file.metadata()?; let mut fsstat = FsStat::from_std(metadata); - stat_extra( - &mut fsstat, - path, - FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, - )?; + stat_extra(&file, &mut fsstat)?; Ok(fsstat) } #[cfg(windows)] -fn stat_extra( - fsstat: &mut FsStat, - path: &Path, - file_flags: winapi::shared::minwindef::DWORD, -) -> FsResult<()> { - use std::os::windows::prelude::OsStrExt; - - use winapi::um::fileapi::CreateFileW; - use winapi::um::fileapi::OPEN_EXISTING; - use winapi::um::handleapi::CloseHandle; - use winapi::um::handleapi::INVALID_HANDLE_VALUE; - use winapi::um::winnt::FILE_SHARE_DELETE; - use winapi::um::winnt::FILE_SHARE_READ; - use winapi::um::winnt::FILE_SHARE_WRITE; - - struct WinHandle(winapi::shared::ntdef::HANDLE); - - impl Drop for WinHandle { - fn drop(&mut self) { - // SAFETY: winapi call - unsafe { - CloseHandle(self.0); - } - } - } +fn stat_extra(file: &std::fs::File, fsstat: &mut FsStat) -> FsResult<()> { + use std::os::windows::io::AsRawHandle; unsafe fn get_dev( handle: winapi::shared::ntdef::HANDLE, @@ -880,25 +864,11 @@ fn stat_extra( // SAFETY: winapi calls unsafe { - let mut path: Vec<_> = path.as_os_str().encode_wide().collect(); - path.push(0); - let file_handle = CreateFileW( - path.as_ptr(), - 0, - FILE_SHARE_READ | FILE_SHARE_DELETE | FILE_SHARE_WRITE, - std::ptr::null_mut(), - OPEN_EXISTING, - file_flags, - std::ptr::null_mut(), - ); - if file_handle == INVALID_HANDLE_VALUE { - return Err(std::io::Error::last_os_error().into()); - } - let file_handle = WinHandle(file_handle); + let file_handle = file.as_raw_handle(); - fsstat.dev = get_dev(file_handle.0)?; + fsstat.dev = get_dev(file_handle)?; - if let Ok(file_info) = query_file_information(file_handle.0) { + if let Ok(file_info) = query_file_information(file_handle) { fsstat.ctime = Some(windows_time_to_unix_time_msec( &file_info.BasicInformation.ChangeTime, ) as u64);