From c677bbccf705c20265ccf945682828b04b1d4b82 Mon Sep 17 00:00:00 2001 From: pcruz1905 Date: Tue, 11 Aug 2026 12:56:16 +0100 Subject: [PATCH] fix: reject non-finite ranks when importing Importing a z, fasd, z.lua or zsh-z data file whose rank is `inf`, `nan`, or simply larger than f64 can hold silently wipes the rest of the database. Ageing scales every rank by `0.9 * max_age / total`. One non-finite rank makes the total infinite, so the factor is zero: every other directory is multiplied down to 0.0, falls under the 1.0 threshold and is dropped, while the offending entry survives as NaN because none of the comparisons against it hold. Importing four directories where one has an `inf` rank leaves a single NaN entry behind, and the command still exits 0. The NaN then keeps the total NaN, `total_age > max_age` is never true again, and the database stops ageing for good. The last_accessed field is already validated by virtue of parsing as u64, so do the same for the rank and reject it with the usual line-numbered error. Ageing also drops non-finite ranks now, so a database that was already hit by this recovers instead of staying stuck. --- src/db/mod.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/import/z.rs | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/db/mod.rs b/src/db/mod.rs index 1856fda..206a2c9 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -121,6 +121,14 @@ impl Database { pub fn age(&mut self, max_age: Rank) { let mut dirty = false; self.with_dirs_mut(|dirs| { + // A non-finite rank would make the total non-finite too, which + // either scales every other rank to zero and drops it, or leaves + // the total NaN so that no comparison below ever holds and the + // database stops ageing entirely. Drop those entries instead. + let len_before = dirs.len(); + dirs.retain(|dir| dir.rank.is_finite()); + dirty |= dirs.len() != len_before; + let total_age = dirs.iter().map(|dir| dir.rank).sum::(); if total_age > max_age { let factor = 0.9 * max_age / total_age; @@ -285,4 +293,41 @@ mod tests { db.save().unwrap(); } } + + #[test] + fn age_drops_non_finite_ranks() { + let data_dir = tempfile::tempdir().unwrap(); + let now = 946684800; + + let mut db = Database::open_dir(data_dir.path()).unwrap(); + db.add_unchecked("/foo", 1.0, now); + db.add_unchecked("/bar", f64::INFINITY, now); + db.add_unchecked("/baz", f64::NAN, now); + db.age(10000.0); + + // The finite entry survives, and it is not scaled to zero by a total + // that a non-finite rank has poisoned. + let dirs = db.dirs(); + assert_eq!(dirs.len(), 1); + assert_eq!(dirs[0].path, "/foo"); + assert!((dirs[0].rank - 1.0).abs() < 0.01); + } + + #[test] + fn age_keeps_ageing_after_non_finite_rank() { + let data_dir = tempfile::tempdir().unwrap(); + let now = 946684800; + + let mut db = Database::open_dir(data_dir.path()).unwrap(); + db.add_unchecked("/poison", f64::NAN, now); + for i in 0..20 { + db.add_unchecked(format!("/dir{i}"), 600.0, now); + } + db.age(10000.0); + + // Without dropping the NaN the total stays NaN, no comparison holds, + // and the database never ages below max_age again. + let total = db.dirs().iter().map(|dir| dir.rank).sum::(); + assert!(total <= 10000.0, "database did not age: total is {total}"); + } } diff --git a/src/import/z.rs b/src/import/z.rs index 511980c..f6b6cb7 100644 --- a/src/import/z.rs +++ b/src/import/z.rs @@ -50,7 +50,14 @@ impl Iter { let last_accessed = last_accessed.parse::().map_err(|_| err())?; let rank = split.next().ok_or_else(err)?; - let rank = rank.parse::().map_err(|_| err())?; + // `parse` accepts `inf` / `nan`, and anything past f64's range parses + // as infinity. Aging multiplies every rank by a factor derived from + // their total, so letting one through turns the entire database into + // zeroes and NaNs. + let rank = match rank.parse::() { + Ok(rank) if rank.is_finite() => rank, + _ => return Err(err()), + }; let path = split.next().ok_or_else(err)?; @@ -101,3 +108,27 @@ fn data_path() -> Result { } } } + +#[cfg(test)] +mod tests { + use rstest::rstest; + + use super::*; + + #[rstest] + #[case("/foo|3|100", true)] + #[case("/foo|0|100", true)] + #[case("/foo|1.5|100", true)] + // A path may contain `|`. + #[case("/f|oo|3|100", true)] + // Non-finite ranks poison the total that ageing divides by. + #[case("/foo|inf|100", false)] + #[case("/foo|-inf|100", false)] + #[case("/foo|nan|100", false)] + // Beyond f64's range, which `parse` rounds to infinity. + #[case("/foo|1e400|100", false)] + fn parse_line_rank(#[case] line: &str, #[case] is_ok: bool) { + let iter = Iter::new(std::io::empty(), PathBuf::new()); + assert_eq!(is_ok, iter.parse_line(line.as_bytes()).is_ok()); + } +}