fix: reject non-finite ranks when importing - #1280
Open
pcruz1905 wants to merge 1 commit into
Open
Conversation
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Importing a data file whose rank is
inf,nan, or just larger than anf64can hold throws away the rest of the database:Three directories are gone and the command reports success.
Database::agescales every rank by0.9 * max_age / total. One non-finiterank makes
totalinfinite, so the factor is0.0: every other directory ismultiplied down to
0.0, drops below the1.0threshold and is removed,while the entry responsible survives, because
NaN < 1.0is false like everyother comparison against it.
It does not stop there. The NaN keeps the total NaN, so
total_age > max_ageis never true again and the database stops ageing permanently — 20 dirs at
rank 600 stay at a total of 12000 instead of being scaled back to 9000.
last_accessedis already validated, in thatparse::<u64>()rejectsanything unreasonable and reports it with a line number. Doing the same for
the rank keeps the bad line out and leaves the rest of the file alone:
agedrops non-finite ranks too, so a database that has already been hit bythis recovers on the next run rather than staying stuck.
This covers
z,fasd,z.luaandzsh-z, which share the parser.autojumppasses its rank through a sigmoid and was never affected.Testing
cargo test(26 tests),cargo +nightly fmt --checkandcargo clippy --all-targets -- -D warningsare clean.Added a parametrised parser test covering
inf,-inf,nanand1e400alongside the ranks that must keep working, including a path containing
|,and two
agetests: one that the finite entries survive a poisoned database,one that ageing still brings the total back under
max_age. Reverting eitherhalf of the fix on its own fails its tests and leaves the valid cases green.