Skip to content

Commit

Permalink
Fix Integer Overflow in FuzzyList (#569)
Browse files Browse the repository at this point in the history
Turns out the code I ported for the `FuzzyList` essentially counts
consecutive ranges of characters that match in both strings. The way it
does this is by multiplying the score by 2 for each matching character.
So this quickly overflows "any" integer type. We could saturate, but
since the score is fairly rough anyway and the original implementation
is in JavaScript where floating point numbers are used anyway, we simply
do the same here and switch to f64. Sorting floating point numbers is of
course iffy, but we don't even have any `NaN` values here anyway, so we
can just use a simplified ordering.
  • Loading branch information
CryZe authored Sep 27, 2022
1 parent 9077724 commit f5b67d6
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 13 deletions.
9 changes: 4 additions & 5 deletions src/analysis/skill_curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,10 @@ impl SkillCurve {
.fold(offset, |sum, segment_time| sum + segment_time);

// Binary search the correct percentile
match sum.partial_cmp(&time_to_find) {
Some(Ordering::Equal) => return percentile,
Some(Ordering::Less) => perc_min = percentile,
Some(Ordering::Greater) => perc_max = percentile,
None => {}
match sum.cmp(&time_to_find) {
Ordering::Equal => return percentile,
Ordering::Less => perc_min = percentile,
Ordering::Greater => perc_max = percentile,
}
}

Expand Down
15 changes: 7 additions & 8 deletions src/run/editor/fuzzy_list.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::platform::prelude::*;
use crate::{platform::prelude::*, util::not_nan::NotNaN};
use alloc::collections::BinaryHeap;
use core::usize;

/// With a Fuzzy List, you can implement a fuzzy searching algorithm. The list
/// stores all the items that can be searched for. With the `search` method you
Expand Down Expand Up @@ -44,7 +43,7 @@ impl FuzzyList {
if heap.len() >= max {
heap.pop();
}
heap.push((usize::MAX - score, element));
heap.push((NotNaN(-score), element));
}
}
heap.into_sorted_vec()
Expand All @@ -54,24 +53,24 @@ impl FuzzyList {
}
}

fn match_against(pattern: &str, text: &str) -> Option<usize> {
let (mut current_score, mut total_score) = (0, 0);
fn match_against(pattern: &str, text: &str) -> Option<f64> {
let (mut current_score, mut total_score) = (0.0, 0.0);
let mut pattern_chars = pattern.chars();
let mut pattern_char = pattern_chars.next();

for c in text.chars() {
if pattern_char == Some(c) {
pattern_char = pattern_chars.next();
current_score = 1 + 2 * current_score;
current_score = 1.0 + 2.0 * current_score;
} else {
current_score = 0;
current_score = 0.0;
}
total_score += current_score;
}

if pattern_char.is_none() {
if pattern == text {
Some(usize::MAX)
Some(f64::INFINITY)
} else {
Some(total_score)
}
Expand Down
1 change: 1 addition & 0 deletions src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pub(crate) mod byte_parsing;
mod clear_vec;
pub(crate) mod not_nan;
pub mod ordered_map;
mod populate_string;
#[cfg(test)]
Expand Down
24 changes: 24 additions & 0 deletions src/util/not_nan.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
use core::cmp::Ordering;

#[repr(transparent)]
pub struct NotNaN(pub f64);

impl PartialEq for NotNaN {
fn eq(&self, other: &Self) -> bool {
self.cmp(other) == Ordering::Equal
}
}

impl Eq for NotNaN {}

impl PartialOrd for NotNaN {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl Ord for NotNaN {
fn cmp(&self, other: &Self) -> Ordering {
self.0.partial_cmp(&other.0).unwrap_or(Ordering::Equal)
}
}

0 comments on commit f5b67d6

Please sign in to comment.