pelikan-io / rustcommon

Common Rust library
Apache License 2.0
26 stars 13 forks source link

Array lookup before bounds check in `percentiles` #25

Closed willmurnane closed 1 year ago

willmurnane commented 1 year ago

The code which calculates a collection of percentile points from the histogram reads a value from the array immediately before checking that the value is in-bounds. This causes an index out of bounds exception every time I call percentiles with a small number of samples.

Expected behavior

Check the bounds before reading from the array, not after.

Actual behavior

Calling percentiles consistently causes an index out of bounds panic.

Steps to reproduce the behavior

fn main() {
    let splits = [25.0, 75.0];
    let h = histogram::Histogram::builder().build().unwrap();
    h.increment(1, 1).unwrap();
    h.increment(10000000, 1).unwrap();
    let answer = h.percentiles(&splits).unwrap();
    println!("Successfully retrieved the percentiles!");
    for p in answer {
        println!("{} {}",
            p.bucket().low(),
            p.bucket().count());
    }
}

thread 'main' panicked at 'index out of bounds: the len is 2 but the index is 2', ...

brayniac commented 1 year ago

Thanks for opening this @willmurnane

I've prepared a fix in #28 and will update once the fixed version is available on crates.io

brayniac commented 1 year ago

histogram 0.7.2 has been published to crates.io and fixes the reported issue.