From e43d893ea3828a5164a4f4b7535c5a1a9b5e7f13 Mon Sep 17 00:00:00 2001 From: David Horner Date: Thu, 25 Jan 2024 16:41:22 -0500 Subject: [PATCH] fix panic caused by `ls \\.\pipe` (#10558) wrap chrono in panic hooks to handle panic'ing unwraps on Jan 1, 1601 00:00 UTC and other reasons unknown. An overflow if time_u64 is smaller than EPOCH_AS_FILETIME has been wrapped. Further discussion https://github.com/nushell/nushell/issues/10464 There are two issues that are associated with Chrono. I did not test. It may not relate, but it could. thread 'main' panicked at 'SystemTimeToFileTime failed with: The parameter is incorrect. https://github.com/nushell/nushell/issues/6574 https://github.com/nushell/nushell/issues/9470 # Description I'm not a fan of this code that was pulled from chrono. negative seconds and nano seconds? ```rust // Adapted from https://github.com/chronotope/chrono/blob/v0.4.19/src/datetime.rs#L755-L767. let (sec, nsec, was_success) = match t.duration_since(UNIX_EPOCH) { Ok(dur) => { (dur.as_secs() as i64, dur.subsec_nanos(),true) }, Err(e) => { // unlikely but should be handled let dur = e.duration(); let (sec, nsec) = (dur.as_secs() as i64, dur.subsec_nanos()); if nsec == 0 { (-sec, 0,false) } else { (-sec - 1, 1_000_000_000 - nsec,false) } } }; ``` There's more on the #10464 ticket; # User-Facing Changes Use ls and it will not crash when listing windows pipes ls \\.\pipe. # Tests + Formatting - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) DONE - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style that command yields: ```rust error: casting raw pointers to the same type and constness is unnecessary (`*mut u16` -> `*mut u16`) --> crates\nu-system\src\windows.rs:972:13 | 972 | name.as_mut_ptr() as *mut u16, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `name.as_mut_ptr()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast = note: `-D clippy::unnecessary-cast` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_cast)]` error: casting raw pointers to the same type and constness is unnecessary (`*mut u16` -> `*mut u16`) --> crates\nu-system\src\windows.rs:974:13 | 974 | domainname.as_mut_ptr() as *mut u16, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `domainname.as_mut_ptr()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast error: could not compile `nu-system` (lib) due to 2 previous errors ``` TBD - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library --- crates/nu-command/src/filesystem/ls.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 89dca8d267..7732533ea3 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -630,6 +630,9 @@ fn try_convert_to_local_date_time(t: SystemTime) -> Option> { } }; + if sec < 0 { + return None; + } match Utc.timestamp_opt(sec, nsec) { LocalResult::Single(t) => Some(t.with_timezone(&Local)), _ => None, @@ -678,11 +681,11 @@ mod windows_helper { // Sometimes this happens when the file name is not allowed on Windows (ex: ends with a '.') // For now, we just log it and give up on returning metadata columns // TODO: find another way to get this data (like cmd.exe, pwsh, and MINGW bash can) - eprintln!( - "Failed to read metadata for '{}'. It may have an illegal filename", - filename.to_string_lossy() - ); - log::error!("{e}"); + // eprintln!( + // "Failed to read metadata for '{}'. It may have an illegal filename", + // filename.to_string_lossy() + // ); + log::error!("ls: {e}"); return Value::record(record, span); } }; @@ -756,10 +759,12 @@ mod windows_helper { const HUNDREDS_OF_NANOSECONDS: u64 = 10000000; let time_u64 = ((ft.dwHighDateTime as u64) << 32) | (ft.dwLowDateTime as u64); - let rel_to_linux_epoch = time_u64 - EPOCH_AS_FILETIME; - let seconds_since_unix_epoch = rel_to_linux_epoch / HUNDREDS_OF_NANOSECONDS; - - seconds_since_unix_epoch as i64 + if time_u64 > 0 { + let rel_to_linux_epoch = time_u64.saturating_sub(EPOCH_AS_FILETIME); + let seconds_since_unix_epoch = rel_to_linux_epoch / HUNDREDS_OF_NANOSECONDS; + return seconds_since_unix_epoch as i64; + } + 0 } // wrapper around the FindFirstFileW Win32 API