From 082a48aec348a67fdf04a9347af4d16d7dea0a45 Mon Sep 17 00:00:00 2001 From: youknowdot Date: Sat, 10 Oct 2026 04:21:50 -0700 Subject: [PATCH] Preserve signed filesystem timestamps in os.utime (#9047) Extract the Python 3.14-compatible signed timestamp conversion from #8954, preserving floor rounding, float-or-index argument rules and signed divmod remainders. Reuse the existing Windows file-time helper rather than the mega patch range restriction, and keep the 3.14 argument diagnostics. Add three concise parser regressions to the existing os snippet without native filesystem range assumptions. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/host_env/src/os.rs | 14 +++++ crates/host_env/src/posix.rs | 20 +++--- crates/host_env/src/posix_wasi.rs | 12 ++-- crates/vm/src/stdlib/os.rs | 101 +++++++++++++++++++++++++----- extra_tests/snippets/stdlib_os.py | 5 ++ 5 files changed, 122 insertions(+), 30 deletions(-) diff --git a/crates/host_env/src/os.rs b/crates/host_env/src/os.rs index 7d6879984..f6bd04d21 100644 --- a/crates/host_env/src/os.rs +++ b/crates/host_env/src/os.rs @@ -605,6 +605,20 @@ pub fn seek_fd( unsafe { suppress_iph!(libc::lseek(fd.as_raw(), position, how)) }.check_libc_neg() } +#[cfg(windows)] +pub type FileTimeSeconds = libc::time64_t; +#[cfg(any(unix, target_os = "wasi"))] +#[cfg_attr(target_env = "musl", allow(deprecated))] +pub type FileTimeSeconds = libc::time_t; + +/// Seconds and nanoseconds passed to filesystem timestamp operations. +#[cfg(any(unix, windows, target_os = "wasi"))] +#[derive(Clone, Copy)] +pub struct FileTime { + pub seconds: FileTimeSeconds, + pub nanoseconds: libc::c_long, +} + #[cfg(windows)] fn filetime_from_duration(duration: Duration) -> FILETIME { let intervals = ((duration.as_secs() as i64 + 11644473600) * 10_000_000) diff --git a/crates/host_env/src/posix.rs b/crates/host_env/src/posix.rs index a6ae039fd..ae7c0d3de 100644 --- a/crates/host_env/src/posix.rs +++ b/crates/host_env/src/posix.rs @@ -762,12 +762,12 @@ pub fn fchmod(fd: BorrowedFd<'_>, mode: u32) -> std::io::Result<()> { #[cfg(target_os = "redox")] pub fn utimes( path: &Path, - acc: core::time::Duration, - modif: core::time::Duration, + acc: crate::os::FileTime, + modif: crate::os::FileTime, ) -> std::io::Result<()> { - let tv = |d: core::time::Duration| libc::timeval { - tv_sec: d.as_secs() as _, - tv_usec: d.subsec_micros() as _, + let tv = |time: crate::os::FileTime| libc::timeval { + tv_sec: time.seconds, + tv_usec: (time.nanoseconds / 1_000) as _, }; nix::sys::stat::utimes(path, &tv(acc).into(), &tv(modif).into()).map_err(std::io::Error::from) } @@ -776,13 +776,13 @@ pub fn utimes( pub fn set_file_times_at( dir_fd: i32, path: &CStr, - access: core::time::Duration, - modified: core::time::Duration, + access: crate::os::FileTime, + modified: crate::os::FileTime, follow_symlinks: bool, ) -> std::io::Result<()> { - let ts = |d: core::time::Duration| libc::timespec { - tv_sec: d.as_secs() as _, - tv_nsec: d.subsec_nanos() as _, + let ts = |time: crate::os::FileTime| libc::timespec { + tv_sec: time.seconds, + tv_nsec: time.nanoseconds as _, }; let times = [ts(access), ts(modified)]; let ret = unsafe { diff --git a/crates/host_env/src/posix_wasi.rs b/crates/host_env/src/posix_wasi.rs index df09c08bc..7c055f084 100644 --- a/crates/host_env/src/posix_wasi.rs +++ b/crates/host_env/src/posix_wasi.rs @@ -1,5 +1,5 @@ use alloc::ffi::CString; -use core::{ffi::CStr, time::Duration}; +use core::ffi::CStr; use rustix::fd::AsFd; use std::{ffi::OsStr, io, path::Path}; @@ -14,13 +14,13 @@ pub fn stat_fd(fd: crate::crt_fd::Borrowed<'_>) -> io::Result io::Result<()> { - let ts = |d: Duration| libc::timespec { - tv_sec: d.as_secs() as _, - tv_nsec: d.subsec_nanos() as _, + let ts = |time: crate::os::FileTime| libc::timespec { + tv_sec: time.seconds, + tv_nsec: time.nanoseconds as _, }; let times = [ts(access), ts(modified)]; unsafe { diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 6f1dcca15..b873cb4c2 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -206,9 +206,9 @@ pub(super) mod _os { #[cfg(any(unix, windows))] use crate::utils::ToCString; use crate::{ - AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, + AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, builtins::{ - PyBytesRef, PyGenericAlias, PyIntRef, PyStrRef, PyTuple, PyTupleRef, PyTypeRef, + PyBytesRef, PyFloat, PyGenericAlias, PyIntRef, PyStrRef, PyTuple, PyTupleRef, PyTypeRef, }, class::PyClassDef, common::lock::{OnceCell, PyRwLock}, @@ -225,11 +225,12 @@ pub(super) mod _os { }, vm::VirtualMachine, }; + use core::hint::cold_path; #[cfg(not(windows))] use core::marker::PhantomData; - use core::{hint::cold_path, time::Duration}; use crossbeam_utils::atomic::AtomicCell; use rustpython_common::wtf8::Wtf8Buf; + use rustpython_host_env::os::{FileTime, FileTimeSeconds}; #[cfg(windows)] use rustpython_host_env::{nt as host_nt, windows::ToWideString}; @@ -1838,6 +1839,56 @@ pub(super) mod _os { follow_symlinks: FollowSymlinks, } + fn file_time_seconds(obj: &PyObject, vm: &VirtualMachine) -> PyResult { + obj.try_index(vm) + .and_then(|value| value.try_to_primitive(vm)) + .map_err(|err| { + if err.fast_isinstance(vm.ctx.exceptions.overflow_error) { + vm.new_overflow_error("timestamp out of range for platform time_t") + } else { + err + } + }) + } + + fn file_time(obj: &PyObject, vm: &VirtualMachine) -> PyResult { + if let Some(float) = obj.downcast_ref::() { + let value = float.to_f64(); + if value.is_nan() { + return Err(vm.new_value_error("Invalid value NaN (not a number)")); + } + let mut seconds = value.trunc(); + let mut nanoseconds = (value.fract() * 1_000_000_000.0).floor(); + if nanoseconds < 0.0 { + nanoseconds += 1_000_000_000.0; + seconds -= 1.0; + } + let minimum = FileTimeSeconds::MIN as f64; + // The signed maximum rounds up when represented by f64. + if !(minimum..-minimum).contains(&seconds) { + return Err(vm.new_overflow_error("timestamp out of range for platform time_t")); + } + Ok(FileTime { + seconds: seconds as FileTimeSeconds, + nanoseconds: nanoseconds as libc::c_long, + }) + } else { + let seconds = match file_time_seconds(obj, vm) { + Err(err) if err.fast_isinstance(vm.ctx.exceptions.type_error) => { + return Err(vm.new_type_error(format!( + "argument must be int or float, not {}", + obj.class().fully_qualified_name(vm)? + ))); + } + result => result?, + }; + Ok(FileTime { + seconds, + nanoseconds: 0, + }) + } + } + #[pyfunction] fn utime(args: UtimeArgs<'_>, vm: &VirtualMachine) -> PyResult<()> { let parse_tup = |tup: &Py| -> Option<(PyObjectRef, PyObjectRef)> { @@ -1852,13 +1903,13 @@ pub(super) mod _os { let (a, m) = parse_tup(&t).ok_or_else(|| { vm.new_type_error("utime: 'times' must be either a tuple of two ints or None") })?; - (a.try_into_value(vm)?, m.try_into_value(vm)?) + (file_time(&a, vm)?, file_time(&m, vm)?) } (None, Some(ns)) => { let (a, m) = parse_tup(&ns) .ok_or_else(|| vm.new_type_error("utime: 'ns' must be a tuple of two ints"))?; let ns_in_sec: PyObjectRef = vm.ctx.new_int(1_000_000_000).into(); - let ns_to_dur = |obj: PyObjectRef| { + let ns_to_time = |obj: PyObjectRef| { let divmod = vm._divmod(&obj, &ns_in_sec)?; let (div, rem) = divmod .downcast_ref::() @@ -1870,16 +1921,31 @@ pub(super) mod _os { divmod.class().name() )) })?; - let secs = div.try_index(vm)?.try_to_primitive(vm)?; - let ns = rem.try_index(vm)?.try_to_primitive(vm)?; - Ok(Duration::new(secs, ns)) + let seconds = file_time_seconds(&div, vm)?; + let rem = rem.try_index(vm)?; + let nanoseconds = rem.try_to_primitive::(vm).map_err(|err| { + if err.fast_isinstance(vm.ctx.exceptions.overflow_error) { + vm.new_overflow_error("Python int too large to convert to C long") + } else { + err + } + })?; + Ok(FileTime { + seconds, + nanoseconds, + }) }; - // TODO: do validation to make sure this doesn't.. underflow? - (ns_to_dur(a)?, ns_to_dur(m)?) + (ns_to_time(a)?, ns_to_time(m)?) } (None, None) => { let now = SystemTime::now(); let now = now.duration_since(SystemTime::UNIX_EPOCH).unwrap(); + let now = FileTime { + seconds: now.as_secs().try_into().map_err(|_| { + vm.new_overflow_error("timestamp out of range for platform time_t") + })?, + nanoseconds: now.subsec_nanos() as libc::c_long, + }; (now, now) } (Some(_), Some(_)) => { @@ -1893,8 +1959,8 @@ pub(super) mod _os { fn utime_impl( path: OsPath, - acc: Duration, - modif: Duration, + acc: FileTime, + modif: FileTime, dir_fd: DirFd<'_, { UTIME_DIR_FD as usize }>, _follow_symlinks: FollowSymlinks, vm: &VirtualMachine, @@ -1934,8 +2000,15 @@ pub(super) mod _os { )); } - crate::host_env::os::set_file_times(&path, acc, modif) - .map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) + let wide_path = path.to_wide_cstring(vm)?; + host_nt::set_file_times( + &wide_path, + acc.seconds, + acc.nanoseconds.into(), + modif.seconds, + modif.nanoseconds.into(), + ) + .map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) } } diff --git a/extra_tests/snippets/stdlib_os.py b/extra_tests/snippets/stdlib_os.py index 2c673e3de..5cb1c03b4 100644 --- a/extra_tests/snippets/stdlib_os.py +++ b/extra_tests/snippets/stdlib_os.py @@ -145,6 +145,11 @@ with TestWithTempDir() as tmpdir: assert os.write(fd, CONTENT2) == len(CONTENT2) os.close(fd) + # A valid negative access time must reach the invalid modification time. + for atime in (-1, -0.25): + assert_raises(TypeError, os.utime, fname, (atime, object())) + assert_raises(TypeError, os.utime, fname, ns=(-1, object())) + fd = os.open(fname, os.O_WRONLY | os.O_APPEND) assert os.write(fd, CONTENT3) == len(CONTENT3) os.close(fd)