From 0acc709fc6b7f406d1f33c0be4b1a9a53738b816 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Sat, 6 Aug 2022 22:17:10 +0200 Subject: [PATCH] And add Windows readdir support --- Cargo.toml | 2 +- README.md | 2 +- src/lib.rs | 19 ++++++- src/unix.rs | 2 +- src/win.rs | 157 ++++++++++++++++++++++++++++++++++++++++++++++------ 5 files changed, 158 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 07d098b..c3a7d15 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,12 +14,12 @@ version = "0.0.1" [dependencies] cfg-if = "1.0.0" +cvt = "0.1.1" [dev-dependencies] tempfile = "3.3.0" [target.'cfg(not(windows))'.dependencies] -cvt = "0.1.1" libc = "0.2.121" # Saves nontrivial unsafe and platform specific code (Darwin vs other Unixes, # MAX_PATH and more) : consider it weak and something we can remove if expedient diff --git a/README.md b/README.md index 8f08ee0..2150823 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ filesystem code, since otherwise the state of the filesystem path that operations are executed against can change silently, leading to TOC-TOU race conditions. For Unix these calls are readily available in the libc crate, but for Windows some more plumbing is needed. This crate provides a unified -Rust-y interface to these calls. +Rust-y and safe interface to these calls. ## MSRV policy diff --git a/src/lib.rs b/src/lib.rs index 9980da3..933f676 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,7 +24,7 @@ cfg_if::cfg_if! { if #[cfg(windows)] { mod win; - use win::OpenOptionsImpl; + use win::{OpenOptionsImpl, ReadDirImpl, DirEntryImpl}; } else { mod unix; @@ -184,6 +184,11 @@ impl OpenOptions { /// This will honour the options set for creation/append etc, but will only /// operate relative to d. To open a file with an absolute path, use the /// stdlib fs::OpenOptions. + /// + /// Note: On Windows this uses low level APIs that do not perform path + /// separator translation: if passing a path containing a separator, it must + /// be a platform native one. e.g. `foo\\bar` on Windows, vs `foo/bar` on + /// most other OS's. pub fn open_at>(&self, d: &mut File, p: P) -> Result { self._impl.open_at(d, OpenOptions::ensure_root(p.as_ref())?) } @@ -232,8 +237,16 @@ impl Iterator for ReadDir<'_> { /// The returned type for each entry found by [`read_dir`]. /// /// Each entry represents a single entry inside the directory. Platforms that -/// provide rich metadata expose this through the methods on DirEntry. -#[derive(Debug, PartialEq)] +/// provide rich metadata may in future expose this through methods or extension +/// traits on DirEntry. +/// +/// For now however, only the [`name()`] is exposed. This does not imply any +/// additional IO for most workloads: metadata returned from a directory listing +/// is inherently racy: presuming that what was a dir, or symlink etc when the +/// directory was listed, will still be the same when opened is fallible. +/// Instead, use open_at to open the contents, and then process based on the +/// type of content found. +#[derive(Debug)] pub struct DirEntry { _impl: DirEntryImpl, } diff --git a/src/unix.rs b/src/unix.rs index 0c7b63a..f2ea1a0 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -274,7 +274,7 @@ impl Iterator for ReadDirImpl<'_> { } } -#[derive(Debug, PartialEq)] +#[derive(Debug)] pub(crate) struct DirEntryImpl { name: OsString, } diff --git a/src/win.rs b/src/win.rs index 09110a1..4ac8519 100644 --- a/src/win.rs +++ b/src/win.rs @@ -1,33 +1,40 @@ mod sugar; use std::{ - ffi::c_void, + ffi::{c_void, OsStr, OsString}, fmt, fs::File, io::Result, mem::{size_of, zeroed, MaybeUninit}, - os::windows::prelude::{AsRawHandle, FromRawHandle, OsStrExt}, + os::windows::prelude::{AsRawHandle, FromRawHandle, OsStrExt, OsStringExt}, path::Path, ptr::null_mut, + slice, }; use ntapi::ntioapi::{ - FILE_CREATE, FILE_CREATED, FILE_DIRECTORY_FILE, FILE_DOES_NOT_EXIST, FILE_EXISTS, - FILE_NON_DIRECTORY_FILE, FILE_OPENED, FILE_OPEN_IF, FILE_OVERWRITE_IF, FILE_OVERWRITTEN, - FILE_SUPERSEDED, FILE_SYNCHRONOUS_IO_NONALERT, + FILE_CREATE, FILE_CREATED, FILE_DIRECTORY_FILE, FILE_DOES_NOT_EXIST, FILE_EXISTS, FILE_OPEN, + FILE_OPENED, FILE_OPEN_IF, FILE_OVERWRITE_IF, FILE_OVERWRITTEN, FILE_SUPERSEDED, + FILE_SYNCHRONOUS_IO_NONALERT, }; use winapi::{ ctypes, shared::{ - minwindef::ULONG, - ntdef::{NULL, OBJECT_ATTRIBUTES, OBJ_CASE_INSENSITIVE, PLARGE_INTEGER, PVOID}, - winerror::ERROR_INVALID_PARAMETER, + minwindef::{LPVOID, ULONG}, + ntdef::{HANDLE, NULL, OBJECT_ATTRIBUTES, OBJ_CASE_INSENSITIVE, PLARGE_INTEGER, PVOID}, + winerror::{ERROR_INVALID_PARAMETER, ERROR_NO_MORE_FILES}, }, - um::winnt::{ - DELETE, FILE_ATTRIBUTE_NORMAL, FILE_GENERIC_WRITE, FILE_LIST_DIRECTORY, FILE_SHARE_DELETE, - FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_TRAVERSE, FILE_WRITE_DATA, GENERIC_READ, - GENERIC_WRITE, PSECURITY_QUALITY_OF_SERVICE, SECURITY_CONTEXT_TRACKING_MODE, - SECURITY_DESCRIPTOR, SECURITY_QUALITY_OF_SERVICE, SYNCHRONIZE, + um::{ + fileapi::FILE_ID_BOTH_DIR_INFO, + minwinbase::{FileIdBothDirectoryInfo, FileIdBothDirectoryRestartInfo}, + winbase::GetFileInformationByHandleEx, + winnt::{ + DELETE, FILE_ATTRIBUTE_NORMAL, FILE_GENERIC_WRITE, FILE_LIST_DIRECTORY, + FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_TRAVERSE, FILE_WRITE_DATA, + GENERIC_READ, GENERIC_WRITE, PSECURITY_QUALITY_OF_SERVICE, + SECURITY_CONTEXT_TRACKING_MODE, SECURITY_DESCRIPTOR, SECURITY_QUALITY_OF_SERVICE, + SYNCHRONIZE, + }, }, }; @@ -226,8 +233,10 @@ impl OpenOptionsImpl { // create options needs to be controlled through OpenOptions too. // FILE_SYNCHRONOUS_IO_NONALERT is set by CreateFile with the options // Rust itself uses - this lets the OS position tracker work. It also - // requires SYNCHRONIZE on the access mode. - let create_options = CreateOptions(FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT); + // requires SYNCHRONIZE on the access mode. We should permit users to + // expect particular types, but until we make that explicit, we need to + // open any kind of file when requested # FILE_NON_DIRECTORY_FILE | + let create_options = CreateOptions(FILE_SYNCHRONOUS_IO_NONALERT); self.do_create_file(f, path, desired_access, create_disposition, create_options) } @@ -244,9 +253,11 @@ impl OpenOptionsImpl { // its poor ergonomics otherwise. Ok(FileDisposition(FILE_CREATE)) } else { - Err(std::io::Error::from_raw_os_error( - ERROR_INVALID_PARAMETER as i32, - )) + // just open the existing file. + Ok(FileDisposition(FILE_OPEN)) + // Err(std::io::Error::from_raw_os_error( + // ERROR_INVALID_PARAMETER as i32, + // )) } } @@ -519,6 +530,116 @@ impl OpenOptionsExt for OpenOptions { } } +#[derive(Debug)] +pub(crate) struct ReadDirImpl<'a> { + /// FILE_ID_BOTH_DIR_INFO is a variable-length struct, otherwise this would + /// be a vec of that. None indicates end of iterator from the OS. + buffer: Option>, + d: &'a mut File, + // byte offset in buffer to next entry to yield + offset: usize, +} + +impl<'a> ReadDirImpl<'a> { + pub fn new(d: &mut File) -> Result { + let mut result = ReadDirImpl { + // Start with a page, can always grow it statically or dynamically if + // needed. + buffer: Some(vec![0_u8; 4096]), + d, + offset: 0, + }; + // TODO: can this ever fail as FindFirstFile does? + result.fill_buffer(FileIdBothDirectoryRestartInfo)?; + Ok(result) + } + + fn fill_buffer(&mut self, class: ULONG) -> Result { + let buffer = self.buffer.as_mut().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::Other, + "Attempt to fill buffer after end of dir", + ) + })?; + // Implement + // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-findnextfilea + // without ever doing path resolution... the docs for + // GetFileInformationByHandleEx do not mention how to detect end of dir, + // but FindNextFile does: + // + // ``` + //If the function fails because no more matching files can be found, + //the GetLastError function returns ERROR_NO_MORE_FILES. + // ``` + let result = cvt::cvt(unsafe { + GetFileInformationByHandleEx( + self.d.as_raw_handle() as HANDLE, + class, + buffer.as_mut_ptr() as LPVOID, + buffer.len() as u32, + ) + }); + match result { + Ok(_) => Ok(false), + Err(e) if e.raw_os_error() == Some(ERROR_NO_MORE_FILES as i32) => Ok(true), + Err(e) => Err(e), + } + } +} + +impl Iterator for ReadDirImpl<'_> { + type Item = Result; + + fn next(&mut self) -> Option { + // if the buffer is empty, fill it; if the buffer is None, exit early. + if self.offset >= self.buffer.as_ref()?.len() { + match self.fill_buffer(FileIdBothDirectoryInfo) { + Ok(false) => { + self.offset = 0; + } + Ok(true) => { + self.buffer = None; + return None; + } + Err(e) => return Some(Err(e)), + } + } + // offset is now valid. Dereference into a struct. + let struct_mem = &self.buffer.as_ref()?[self.offset..]; + let info = unsafe { &*struct_mem.as_ptr().cast::() }; + self.offset = if info.NextEntryOffset == 0 { + self.buffer.as_ref()?.len() + } else { + info.NextEntryOffset as usize + self.offset + }; + + let name = OsString::from_wide(unsafe { + slice::from_raw_parts( + info.FileName.as_ptr(), + info.FileNameLength as usize / size_of::(), + ) + }); + Some(Ok(DirEntryImpl { name })) + // + // + // Read Attributes, Delete, Synchronize + // Disposition: Open + // Options: Synchronous IO Non-Alert, Open Reparse Point + // + } +} + +#[derive(Debug)] +pub(crate) struct DirEntryImpl { + name: OsString, +} + +impl DirEntryImpl { + pub fn name(&self) -> &OsStr { + &self.name + } +} + #[cfg(test)] mod tests { use std::{fs::rename, io::Result};