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.
This commit is contained in:
pcruz1905 2026-08-11 12:56:16 +01:00
parent ee594e6906
commit c677bbccf7
No known key found for this signature in database
2 changed files with 77 additions and 1 deletions

View File

@ -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::<Rank>();
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::<Rank>();
assert!(total <= 10000.0, "database did not age: total is {total}");
}
}

View File

@ -50,7 +50,14 @@ impl<R: BufRead> Iter<R> {
let last_accessed = last_accessed.parse::<u64>().map_err(|_| err())?;
let rank = split.next().ok_or_else(err)?;
let rank = rank.parse::<f64>().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::<f64>() {
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<PathBuf> {
}
}
}
#[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());
}
}