From 39c0b00cf9e7bf23d90ef7863749590149572ed3 Mon Sep 17 00:00:00 2001 From: beetrees Date: Sat, 10 Sep 2022 11:29:01 +0100 Subject: [PATCH 1/2] Error instead of panicking when setting file times if the passed `SystemTime` doesn't fit into the required type --- library/std/src/lib.rs | 1 + library/std/src/sys/unix/fs.rs | 62 +++++++++++--------------------- library/std/src/sys/wasi/fs.rs | 19 ++++++---- library/std/src/sys/wasi/time.rs | 4 +-- 4 files changed, 37 insertions(+), 49 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 64b62fd3bba..97eed8a65c5 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -251,6 +251,7 @@ #![feature(doc_notable_trait)] #![feature(dropck_eyepatch)] #![feature(exhaustive_patterns)] +#![feature(if_let_guard)] #![feature(intra_doc_pointers)] #![feature(lang_items)] #![feature(let_chains)] diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index cc347e3586a..57c7bf6a28b 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -313,8 +313,11 @@ pub struct FilePermissions { mode: mode_t, } -#[derive(Copy, Clone)] -pub struct FileTimes([libc::timespec; 2]); +#[derive(Copy, Clone, Debug, Default)] +pub struct FileTimes { + accessed: Option, + modified: Option, +} #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] pub struct FileType { @@ -512,45 +515,11 @@ impl FilePermissions { impl FileTimes { pub fn set_accessed(&mut self, t: SystemTime) { - self.0[0] = t.t.to_timespec().expect("Invalid system time"); + self.accessed = Some(t); } pub fn set_modified(&mut self, t: SystemTime) { - self.0[1] = t.t.to_timespec().expect("Invalid system time"); - } -} - -struct TimespecDebugAdapter<'a>(&'a libc::timespec); - -impl fmt::Debug for TimespecDebugAdapter<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("timespec") - .field("tv_sec", &self.0.tv_sec) - .field("tv_nsec", &self.0.tv_nsec) - .finish() - } -} - -impl fmt::Debug for FileTimes { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("FileTimes") - .field("accessed", &TimespecDebugAdapter(&self.0[0])) - .field("modified", &TimespecDebugAdapter(&self.0[1])) - .finish() - } -} - -impl Default for FileTimes { - fn default() -> Self { - // Redox doesn't appear to support `UTIME_OMIT`, so we stub it out here, and always return - // an error in `set_times`. - // ESP-IDF and HorizonOS do not support `futimens` at all and the behavior for those OS is therefore - // the same as for Redox. - #[cfg(any(target_os = "redox", target_os = "espidf", target_os = "horizon"))] - let omit = libc::timespec { tv_sec: 0, tv_nsec: 0 }; - #[cfg(not(any(target_os = "redox", target_os = "espidf", target_os = "horizon")))] - let omit = libc::timespec { tv_sec: 0, tv_nsec: libc::UTIME_OMIT as _ }; - Self([omit; 2]) + self.modified = Some(t); } } @@ -1084,6 +1053,17 @@ impl File { } pub fn set_times(&self, times: FileTimes) -> io::Result<()> { + #[cfg(not(any(target_os = "redox", target_os = "espidf", target_os = "horizon")))] + let to_timespec = |time: Option| { + match time { + Some(time) if let Some(ts) = time.t.to_timespec() => Ok(ts), + Some(time) if time > crate::sys::time::UNIX_EPOCH => Err(io::const_io_error!(io::ErrorKind::InvalidInput, "timestamp is too large to set as a file time")), + Some(_) => Err(io::const_io_error!(io::ErrorKind::InvalidInput, "timestamp is too small to set as a file time")), + None => Ok(libc::timespec { tv_sec: 0, tv_nsec: libc::UTIME_OMIT as _ }), + } + }; + #[cfg(not(any(target_os = "redox", target_os = "espidf", target_os = "horizon")))] + let times = [to_timespec(times.accessed)?, to_timespec(times.modified)?]; cfg_if::cfg_if! { if #[cfg(any(target_os = "redox", target_os = "espidf", target_os = "horizon"))] { // Redox doesn't appear to support `UTIME_OMIT`. @@ -1099,7 +1079,7 @@ impl File { cvt(unsafe { weak!(fn futimens(c_int, *const libc::timespec) -> c_int); match futimens.get() { - Some(futimens) => futimens(self.as_raw_fd(), times.0.as_ptr()), + Some(futimens) => futimens(self.as_raw_fd(), times.as_ptr()), #[cfg(target_os = "macos")] None => { fn ts_to_tv(ts: &libc::timespec) -> libc::timeval { @@ -1108,7 +1088,7 @@ impl File { tv_usec: (ts.tv_nsec / 1000) as _ } } - let timevals = [ts_to_tv(×.0[0]), ts_to_tv(×.0[1])]; + let timevals = [ts_to_tv(×[0]), ts_to_tv(×[1])]; libc::futimes(self.as_raw_fd(), timevals.as_ptr()) } // futimes requires even newer Android. @@ -1121,7 +1101,7 @@ impl File { })?; Ok(()) } else { - cvt(unsafe { libc::futimens(self.as_raw_fd(), times.0.as_ptr()) })?; + cvt(unsafe { libc::futimens(self.as_raw_fd(), times.as_ptr()) })?; Ok(()) } } diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index 510cf36b1bf..953fbeb8395 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -65,8 +65,8 @@ pub struct FilePermissions { #[derive(Copy, Clone, Debug, Default)] pub struct FileTimes { - accessed: Option, - modified: Option, + accessed: Option, + modified: Option, } #[derive(PartialEq, Eq, Hash, Debug, Copy, Clone)] @@ -120,11 +120,11 @@ impl FilePermissions { impl FileTimes { pub fn set_accessed(&mut self, t: SystemTime) { - self.accessed = Some(t.to_wasi_timestamp_or_panic()); + self.accessed = Some(t); } pub fn set_modified(&mut self, t: SystemTime) { - self.modified = Some(t.to_wasi_timestamp_or_panic()); + self.modified = Some(t); } } @@ -476,9 +476,16 @@ impl File { } pub fn set_times(&self, times: FileTimes) -> io::Result<()> { + let to_timestamp = |time: Option| { + match time { + Some(time) if let Some(ts) = time.to_wasi_timestamp() => Ok(ts), + Some(_) => Err(io::const_io_error!(io::ErrorKind::InvalidInput, "timestamp is too large to set as a file time")), + None => Ok(0), + } + }; self.fd.filestat_set_times( - times.accessed.unwrap_or(0), - times.modified.unwrap_or(0), + to_timestamp(times.accessed)?, + to_timestamp(times.modified)?, times.accessed.map_or(0, |_| wasi::FSTFLAGS_ATIM) | times.modified.map_or(0, |_| wasi::FSTFLAGS_MTIM), ) diff --git a/library/std/src/sys/wasi/time.rs b/library/std/src/sys/wasi/time.rs index 3d326e49106..016b06efbdc 100644 --- a/library/std/src/sys/wasi/time.rs +++ b/library/std/src/sys/wasi/time.rs @@ -47,8 +47,8 @@ impl SystemTime { SystemTime(Duration::from_nanos(ts)) } - pub fn to_wasi_timestamp_or_panic(&self) -> wasi::Timestamp { - self.0.as_nanos().try_into().expect("time does not fit in WASI timestamp") + pub fn to_wasi_timestamp(&self) -> Option { + self.0.as_nanos().try_into().ok() } pub fn sub_time(&self, other: &SystemTime) -> Result { From c66860ab3ed7861458fca2fba4a7e5f512571537 Mon Sep 17 00:00:00 2001 From: beetrees Date: Sat, 10 Sep 2022 11:33:23 +0100 Subject: [PATCH 2/2] `SetFileTime` doesn't allow setting the file time to `0xFFFF_FFFF_FFFF_FFFF` --- library/std/src/sys/windows/fs.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index 155d0297e49..ade00750c95 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -572,6 +572,14 @@ impl File { "Cannot set file timestamp to 0", )); } + let is_max = + |t: c::FILETIME| t.dwLowDateTime == c::DWORD::MAX && t.dwHighDateTime == c::DWORD::MAX; + if times.accessed.map_or(false, is_max) || times.modified.map_or(false, is_max) { + return Err(io::const_io_error!( + io::ErrorKind::InvalidInput, + "Cannot set file timestamp to 0xFFFF_FFFF_FFFF_FFFF", + )); + } cvt(unsafe { c::SetFileTime(self.as_handle(), None, times.accessed.as_ref(), times.modified.as_ref()) })?;