This is an automated email from the ASF dual-hosted git repository.
tisonkun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datasketches-rust.git
The following commit(s) were added to refs/heads/main by this push:
new 8d9a36a test: streamline deserialization regressions (#228)
8d9a36a is described below
commit 8d9a36ac54f592f242ad208bfc85818fa3ac04c6
Author: tison <[email protected]>
AuthorDate: Wed Aug 26 17:14:22 2026 +0800
test: streamline deserialization regressions (#228)
---
datasketches/tests/cpc_test/deserialize.rs | 77 +-------------
datasketches/tests/serde_tests/frequencies.rs | 147 ++++++--------------------
datasketches/tests/serde_tests/req.rs | 4 +-
3 files changed, 38 insertions(+), 190 deletions(-)
diff --git a/datasketches/tests/cpc_test/deserialize.rs
b/datasketches/tests/cpc_test/deserialize.rs
index 3a77577..3bd7886 100644
--- a/datasketches/tests/cpc_test/deserialize.rs
+++ b/datasketches/tests/cpc_test/deserialize.rs
@@ -19,7 +19,6 @@
use datasketches::cpc::CpcSketch;
-/// Builds a valid serialized sketch that exercises a particular CPC flavor.
fn valid_bytes(lg_k: u8, n: u64) -> Vec<u8> {
let mut sketch = CpcSketch::new(lg_k);
for i in 0..n {
@@ -28,17 +27,6 @@ fn valid_bytes(lg_k: u8, n: u64) -> Vec<u8> {
sketch.serialize()
}
-/// Each non-empty entry lands in a different CPC flavor.
-const CASES: &[(u8, u64)] = &[
- (4, 0),
- (4, 3),
- (6, 20),
- (8, 200),
- (8, 2000),
- (10, 800),
- (10, 8000),
-];
-
#[test]
fn truncated_compressed_streams_return_errors() {
let bytes = valid_bytes(10, 8_000);
@@ -51,65 +39,8 @@ fn truncated_compressed_streams_return_errors() {
}
#[test]
-fn targeted_corruptions_return_err() {
- // A sliding-flavor sketch drives the pair/window decoders and the
offset/permutation logic.
- let base = valid_bytes(10, 8000);
-
- // Layout: [preamble_ints, serial_version, family, lg_k,
first_interesting_column, flags,
- // seed_hash(2), num_coupons(4), ...]. Corrupting the num_coupons
field makes the
- // decoders read past the compressed buffer / compute an
out-of-range window offset.
- let mut num_coupons_hi = base.clone();
- num_coupons_hi[11] = 0xff; // enormous num_coupons
- assert!(CpcSketch::deserialize(&num_coupons_hi).is_err());
-
- // Flipping the flags byte makes the declared flavor inconsistent with the
stored data.
- let mut bad_flags = base.clone();
- bad_flags[5] ^= 0xff;
- assert!(CpcSketch::deserialize(&bad_flags).is_err());
-
- // These payload edits previously reached panicking decoder and pair-table
paths.
- let mut bad_payload = base.clone();
- let last = bad_payload.len() - 1;
- bad_payload[last] = bad_payload[last].wrapping_add(1);
- bad_payload[last - 3] ^= 0xa5;
- let _ = CpcSketch::deserialize(&bad_payload);
-
- // A sparse sketch whose declared entry count exceeds its data words must
be rejected up front.
- let sparse = valid_bytes(8, 200);
- let mut inflated = sparse.clone();
- // num_coupons is a u32 at offset 8; inflate it far beyond the coupon
space.
- inflated[10] = 0xff;
- inflated[11] = 0xff;
- assert!(CpcSketch::deserialize(&inflated).is_err());
-}
-
-#[test]
-fn valid_sketches_round_trip_unchanged() {
- for &(lg_k, n) in CASES {
- let mut sketch = CpcSketch::new(lg_k);
- for i in 0..n {
- sketch.update(i);
- }
- let bytes = sketch.serialize();
-
- let restored = CpcSketch::deserialize(&bytes).unwrap_or_else(|e| {
- panic!("valid sketch (lg_k={lg_k}, n={n}) failed to deserialize:
{e}")
- });
-
- assert_eq!(
- sketch.estimate(),
- restored.estimate(),
- "estimate changed after round-trip (lg_k={lg_k}, n={n})"
- );
- assert_eq!(
- sketch.num_coupons(),
- restored.num_coupons(),
- "num_coupons changed after round-trip (lg_k={lg_k}, n={n})"
- );
- assert_eq!(
- bytes,
- restored.serialize(),
- "re-serialized bytes changed after round-trip (lg_k={lg_k}, n={n})"
- );
- }
+fn oversized_coupon_count_is_rejected() {
+ let mut bytes = valid_bytes(10, 8_000);
+ bytes[11] = u8::MAX;
+ assert!(CpcSketch::deserialize(&bytes).is_err());
}
diff --git a/datasketches/tests/serde_tests/frequencies.rs
b/datasketches/tests/serde_tests/frequencies.rs
index 75f69b6..1560bae 100644
--- a/datasketches/tests/serde_tests/frequencies.rs
+++ b/datasketches/tests/serde_tests/frequencies.rs
@@ -24,9 +24,7 @@ use datasketches::error::ErrorKind;
use datasketches::frequencies::FrequentItemValue;
use datasketches::frequencies::FrequentItemsSketch;
use googletest::assert_that;
-use googletest::prelude::anything;
use googletest::prelude::contains_substring;
-use googletest::prelude::err;
use googletest::prelude::gt;
use crate::serialization_test_data;
@@ -398,133 +396,52 @@ fn test_go_frequent_strings_utf8() {
assert_eq!(sketch.estimate(&"эюя".to_string()), 7);
}
-// Header field constants for the DataSketches frequent-items format.
-const FREQ_SERIAL_VERSION: u8 = 1;
-const FREQ_FAMILY_ID: u8 = 10;
-const FREQ_PREAMBLE_LONGS_EMPTY: u8 = 1;
-const FREQ_PREAMBLE_LONGS_NONEMPTY: u8 = 4;
-const FREQ_EMPTY_FLAG_MASK: u8 = 5;
-
-fn empty_header(lg_max: u8, lg_cur: u8) -> Vec<u8> {
- let mut bytes = SketchBytes::with_capacity(8);
- bytes.write_u8(FREQ_PREAMBLE_LONGS_EMPTY);
- bytes.write_u8(FREQ_SERIAL_VERSION);
- bytes.write_u8(FREQ_FAMILY_ID);
- bytes.write_u8(lg_max);
- bytes.write_u8(lg_cur);
- bytes.write_u8(FREQ_EMPTY_FLAG_MASK);
- bytes.write_u16_le(0);
- bytes.into_bytes()
-}
-
-// Regression: a corrupt header must never panic (or trigger an oversized
-// allocation) inside `with_lg_map_sizes`; deserialize must reject it cleanly.
-// Before the fix, `lg_max_map_size = 222` drove `1usize << lg_max` past the
-// width of `usize`, panicking with "attempt to shift left with overflow" in
-// debug builds.
-#[test]
-fn test_deserialize_rejects_out_of_range_lg_max_map_size() {
- let bytes = empty_header(222, 5);
- let result = FrequentItemsSketch::<i64>::deserialize(&bytes);
- assert_that!(result, err(anything()));
- assert_eq!(result.unwrap_err().kind(), ErrorKind::InvalidData);
-}
-
-#[test]
-fn test_deserialize_rejects_out_of_range_lg_max_map_size_nonempty() {
- let mut bytes = SketchBytes::with_capacity(16);
- bytes.write_u8(FREQ_PREAMBLE_LONGS_NONEMPTY);
- bytes.write_u8(FREQ_SERIAL_VERSION);
- bytes.write_u8(FREQ_FAMILY_ID);
- bytes.write_u8(200); // lg_max_map_size, out of range
- bytes.write_u8(3); // lg_cur_map_size
- bytes.write_u8(0); // flags (not empty)
- bytes.write_u16_le(0);
- bytes.write_u32_le(0); // active_items
- bytes.write_u32_le(0);
- bytes.write_u64_le(0); // stream_weight
- bytes.write_u64_le(0); // offset
- let result = FrequentItemsSketch::<i64>::deserialize(&bytes.into_bytes());
- assert_that!(result, err(anything()));
-}
-
-// `lg_cur_map_size` below the documented minimum is also corruption; the C++
-// reference `check_size` rejects it, so the Rust port must too.
-#[test]
-fn test_deserialize_rejects_undersized_lg_cur_map_size() {
- let bytes = empty_header(10, 1);
- let result = FrequentItemsSketch::<i64>::deserialize(&bytes);
- assert_that!(result, err(anything()));
-}
-
-// The largest supported configuration must remain cheap while the sketch is
-// empty and round-trip through the public constructor and serializer.
#[test]
-fn test_maximum_map_size_empty_round_trip() {
- let sketch = FrequentItemsSketch::<i64>::new(1usize << 30);
- let bytes = sketch.serialize();
- let restored = FrequentItemsSketch::<i64>::deserialize(&bytes).unwrap();
- assert!(restored.is_empty());
- assert_eq!(restored.lg_max_map_size(), 30);
- assert_eq!(restored.lg_cur_map_size(), 3);
-}
-
-// Builds a minimal non-empty (four-preamble-long) header. The caller supplies
-// `lg_max`, `lg_cur`, and `active_items`; no item payload is appended, so this
-// is only useful for exercising the header-consistency guards that run before
-// the payload is read.
-fn nonempty_header(lg_max: u8, lg_cur: u8, active_items: u32) -> Vec<u8> {
- let mut bytes = SketchBytes::with_capacity(32);
- bytes.write_u8(FREQ_PREAMBLE_LONGS_NONEMPTY);
- bytes.write_u8(FREQ_SERIAL_VERSION);
- bytes.write_u8(FREQ_FAMILY_ID);
- bytes.write_u8(lg_max);
- bytes.write_u8(lg_cur);
- bytes.write_u8(0); // flags (not empty)
- bytes.write_u16_le(0);
- bytes.write_u32_le(active_items);
- bytes.write_u32_le(0); // unused
- bytes.write_u64_le(0); // stream_weight
- bytes.write_u64_le(0); // offset
- bytes.into_bytes()
+fn test_deserialize_rejects_invalid_map_sizes() {
+ let empty = FrequentItemsSketch::<i64>::new(32).serialize();
+ let mut nonempty_sketch = FrequentItemsSketch::<i64>::new(32);
+ nonempty_sketch.update(1);
+ let nonempty = nonempty_sketch.serialize();
+
+ for (mut bytes, lg_max, lg_cur) in [(empty.clone(), 222, 3), (nonempty,
200, 3), (empty, 10, 1)]
+ {
+ bytes[3] = lg_max;
+ bytes[4] = lg_cur;
+ assert_eq!(
+ FrequentItemsSketch::<i64>::deserialize(&bytes)
+ .unwrap_err()
+ .kind(),
+ ErrorKind::InvalidData
+ );
+ }
}
-// Regression for tisonkun's report on #224: the accepted boundary
-// `(lg_max, lg_cur) = (30, 30)` on an *empty* header still drove
-// `ReversePurgeItemHashMap::new(1 << 30)` (~1e9 slots, multi-GB) before any
-// payload was read. An empty sketch holds nothing, so it must now build its
map
-// at the minimum size and deserialize cheaply instead of attempting the
-// allocation. If the fix regressed, this test would OOM/hang rather than fail.
#[test]
-fn test_deserialize_empty_header_does_not_over_allocate() {
- let bytes = empty_header(30, 30);
+fn test_deserialize_empty_header_does_not_allocate_declared_map() {
+ let mut bytes = FrequentItemsSketch::<i64>::new(1usize << 30).serialize();
+ bytes[4] = 30;
let restored = FrequentItemsSketch::<i64>::deserialize(&bytes).unwrap();
assert!(restored.is_empty());
- assert_eq!(restored.num_active_items(), 0);
assert_eq!(restored.lg_max_map_size(), 30);
assert_eq!(restored.lg_cur_map_size(), 3);
}
-// A non-empty header whose `lg_cur_map_size` is too small to hold the claimed
-// `active_items` under the load factor is corrupt: a map of `1 << 3` slots has
-// capacity 6, so it can never hold 100 active items. Reject it instead of
-// trusting the header.
#[test]
fn test_deserialize_rejects_lg_cur_inconsistent_with_num_active() {
- let bytes = nonempty_header(10, 3, 100);
- let result = FrequentItemsSketch::<i64>::deserialize(&bytes);
- assert_that!(result, err(anything()));
- assert_eq!(result.unwrap_err().kind(), ErrorKind::InvalidData);
+ let mut sketch = FrequentItemsSketch::<i64>::new(32);
+ sketch.update(1);
+ let mut bytes = sketch.serialize();
+ bytes[8..12].copy_from_slice(&100u32.to_le_bytes());
+ assert!(FrequentItemsSketch::<i64>::deserialize(&bytes).is_err());
}
-// A non-empty header claiming a huge `active_items` that the remaining bytes
-// cannot possibly contain must be rejected before any capacity is reserved
from
-// that count, so a corrupt header cannot drive a multi-GB `Vec` allocation.
#[test]
fn test_deserialize_rejects_num_active_exceeding_payload() {
- // 700_000_000 <= capacity implied by lg_cur = 30, so it clears the
capacity
- // guard, but there are no item bytes for it, so the length guard rejects
it.
- let bytes = nonempty_header(30, 30, 700_000_000);
- let result = FrequentItemsSketch::<i64>::deserialize(&bytes);
- assert_that!(result, err(anything()));
+ let mut sketch = FrequentItemsSketch::<i64>::new(32);
+ sketch.update(1);
+ let mut bytes = sketch.serialize();
+ bytes[3] = 30;
+ bytes[4] = 30;
+ bytes[8..12].copy_from_slice(&700_000_000u32.to_le_bytes());
+ assert!(FrequentItemsSketch::<i64>::deserialize(&bytes).is_err());
}
diff --git a/datasketches/tests/serde_tests/req.rs
b/datasketches/tests/serde_tests/req.rs
index d9d8acf..a5e48fa 100644
--- a/datasketches/tests/serde_tests/req.rs
+++ b/datasketches/tests/serde_tests/req.rs
@@ -53,9 +53,9 @@ where
#[test]
fn round_trip_f64_matrix() {
- for &k in &[4u16, 6, 10, 12, 1024] {
+ for &k in &[4u16, 12, 1024] {
for &ra in &[RankAccuracy::HighRank, RankAccuracy::LowRank] {
- for &n in &[0u64, 1, 4, 5, 100, 1_250, 2_562, 10_000, 100_000] {
+ for &n in &[0u64, 1, 4, 5, 100, 10_000] {
round_trip_one::<f64>(k, ra, n, |i| i as f64);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
