From 9b69a096256ffcfb49a3609dd2780d84790c756c Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 5 Oct 2023 03:15:50 +0100 Subject: [PATCH] Make File::create work on Windows hidden files Previously it failed on Windows if the file had the `FILE_ATTRIBUTE_HIDDEN` attribute set. This was inconsistent with `OpenOptions::new().write(true).truncate(true)` which can truncate an existing hidden file. --- library/std/src/fs/tests.rs | 27 +++++++++++++++- library/std/src/sys/windows/c/windows_sys.lst | 1 + library/std/src/sys/windows/c/windows_sys.rs | 10 ++++++ library/std/src/sys/windows/fs.rs | 31 ++++++++++++++++--- 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index f0c4be2327ce2..6eb083515a462 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -22,7 +22,7 @@ use crate::os::unix::fs::symlink as symlink_file; #[cfg(unix)] use crate::os::unix::fs::symlink as symlink_junction; #[cfg(windows)] -use crate::os::windows::fs::{symlink_dir, symlink_file}; +use crate::os::windows::fs::{symlink_dir, symlink_file, OpenOptionsExt}; #[cfg(windows)] use crate::sys::fs::symlink_junction; #[cfg(target_os = "macos")] @@ -1742,3 +1742,28 @@ fn windows_unix_socket_exists() { assert_eq!(socket_path.try_exists().unwrap(), true); assert_eq!(socket_path.metadata().is_ok(), true); } + +#[cfg(windows)] +#[test] +fn test_hidden_file_truncation() { + // Make sure that File::create works on an existing hidden file. See #115745. + let tmpdir = tmpdir(); + let path = tmpdir.join("hidden_file.txt"); + + // Create a hidden file. + const FILE_ATTRIBUTE_HIDDEN: u32 = 2; + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .attributes(FILE_ATTRIBUTE_HIDDEN) + .open(&path) + .unwrap(); + file.write("hidden world!".as_bytes()).unwrap(); + file.flush().unwrap(); + drop(file); + + // Create a new file by truncating the existing one. + let file = File::create(&path).unwrap(); + let metadata = file.metadata().unwrap(); + assert_eq!(metadata.len(), 0); +} diff --git a/library/std/src/sys/windows/c/windows_sys.lst b/library/std/src/sys/windows/c/windows_sys.lst index dad4a4223b2a3..31e5de4bce423 100644 --- a/library/std/src/sys/windows/c/windows_sys.lst +++ b/library/std/src/sys/windows/c/windows_sys.lst @@ -2224,6 +2224,7 @@ Windows.Win32.Storage.FileSystem.FILE_ACCESS_RIGHTS Windows.Win32.Storage.FileSystem.FILE_ADD_FILE Windows.Win32.Storage.FileSystem.FILE_ADD_SUBDIRECTORY Windows.Win32.Storage.FileSystem.FILE_ALL_ACCESS +Windows.Win32.Storage.FileSystem.FILE_ALLOCATION_INFO Windows.Win32.Storage.FileSystem.FILE_APPEND_DATA Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_ARCHIVE Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_COMPRESSED diff --git a/library/std/src/sys/windows/c/windows_sys.rs b/library/std/src/sys/windows/c/windows_sys.rs index 20b44966a39f5..a3400eb3ac6ab 100644 --- a/library/std/src/sys/windows/c/windows_sys.rs +++ b/library/std/src/sys/windows/c/windows_sys.rs @@ -3107,6 +3107,16 @@ impl ::core::clone::Clone for FILETIME { pub type FILE_ACCESS_RIGHTS = u32; pub const FILE_ADD_FILE: FILE_ACCESS_RIGHTS = 2u32; pub const FILE_ADD_SUBDIRECTORY: FILE_ACCESS_RIGHTS = 4u32; +#[repr(C)] +pub struct FILE_ALLOCATION_INFO { + pub AllocationSize: i64, +} +impl ::core::marker::Copy for FILE_ALLOCATION_INFO {} +impl ::core::clone::Clone for FILE_ALLOCATION_INFO { + fn clone(&self) -> Self { + *self + } +} pub const FILE_ALL_ACCESS: FILE_ACCESS_RIGHTS = 2032127u32; pub const FILE_APPEND_DATA: FILE_ACCESS_RIGHTS = 4u32; pub const FILE_ATTRIBUTE_ARCHIVE: FILE_FLAGS_AND_ATTRIBUTES = 32u32; diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index 6ded683aade1f..cf06a0c63cf3e 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -1,7 +1,7 @@ use crate::os::windows::prelude::*; use crate::borrow::Cow; -use crate::ffi::OsString; +use crate::ffi::{c_void, OsString}; use crate::fmt; use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem::{self, MaybeUninit}; @@ -273,7 +273,9 @@ impl OpenOptions { (false, false, false) => c::OPEN_EXISTING, (true, false, false) => c::OPEN_ALWAYS, (false, true, false) => c::TRUNCATE_EXISTING, - (true, true, false) => c::CREATE_ALWAYS, + // `CREATE_ALWAYS` has weird semantics so we emulate it using + // `OPEN_ALWAYS` and a manual truncation step. See #115745. + (true, true, false) => c::OPEN_ALWAYS, (_, _, true) => c::CREATE_NEW, }) } @@ -289,19 +291,40 @@ impl OpenOptions { impl File { pub fn open(path: &Path, opts: &OpenOptions) -> io::Result { let path = maybe_verbatim(path)?; + let creation = opts.get_creation_mode()?; let handle = unsafe { c::CreateFileW( path.as_ptr(), opts.get_access_mode()?, opts.share_mode, opts.security_attributes, - opts.get_creation_mode()?, + creation, opts.get_flags_and_attributes(), ptr::null_mut(), ) }; let handle = unsafe { HandleOrInvalid::from_raw_handle(handle) }; - if let Ok(handle) = handle.try_into() { + if let Ok(handle) = OwnedHandle::try_from(handle) { + // Manual truncation. See #115745. + if opts.truncate + && creation == c::OPEN_ALWAYS + && unsafe { c::GetLastError() } == c::ERROR_ALREADY_EXISTS + { + unsafe { + // Setting the allocation size to zero also sets the + // EOF position to zero. + let alloc = c::FILE_ALLOCATION_INFO { AllocationSize: 0 }; + let result = c::SetFileInformationByHandle( + handle.as_raw_handle(), + c::FileAllocationInfo, + ptr::addr_of!(alloc).cast::(), + mem::size_of::() as u32, + ); + if result == 0 { + return Err(io::Error::last_os_error()); + } + } + } Ok(File { handle: Handle::from_inner(handle) }) } else { Err(Error::last_os_error())