Copilot commented on code in PR #226:
URL: https://github.com/apache/datasketches-rust/pull/226#discussion_r3858827197
##########
datasketches/src/cpc/sketch.rs:
##########
@@ -639,7 +639,54 @@ impl CpcSketch {
)));
}
- let uncompressed = compressed.uncompress(lg_k, num_coupons);
+ // The coupon space of a sketch has `k * 64` cells (`k` rows of 64
columns each), so a
+ // valid sketch can never report more coupons than that. Rejecting
larger values keeps the
+ // flavor arithmetic below from overflowing on corrupt input.
+ if (num_coupons as u64) > 64 * (1u64 << lg_k) {
+ return Err(Error::deserial(format!(
+ "num_coupons ({}) exceeds coupon space for lg_k = {}",
+ num_coupons, lg_k
+ )));
+ }
+
+ // A valid sketch stores a sliding window exactly for the pinned and
sliding flavors, and
+ // stores its coupons in the surprising-value table for the sparse and
hybrid flavors. The
+ // flavor is fully determined by `lg_k` and `num_coupons`, so the
flags must agree with it.
+ let flavor = determine_flavor(lg_k, num_coupons);
+ let window_expected = matches!(flavor, Flavor::Pinned |
Flavor::Sliding);
Review Comment:
`num_coupons == 0` implies `Flavor::Empty`, but the deserializer currently
allows `has_hip` / `has_table` / `has_window` flags to be set in that case
(because the flavor/flag consistency checks only constrain window for
pinned/sliding and table for sparse/hybrid). This accepts serialized states
that `serialize()` cannot produce and undermines the goal of rejecting
malformed inputs early.
##########
datasketches/src/cpc/compression.rs:
##########
@@ -403,116 +404,154 @@ impl CompressedState {
let mut next_true_pair = 0;
for i in 0..self.table_num_entries {
let row_col = pairs[i as usize];
- assert_ne!(row_col, u32::MAX);
+ if row_col == u32::MAX {
+ return Err(Error::deserial("CPC hybrid table contains an
invalid pair"));
+ }
let col = row_col & 63;
Review Comment:
In hybrid uncompression, `validate_pair_rows(&pairs, lg_k)?` runs before
this loop, which already rejects `u32::MAX` (row index is far out of range).
That makes the `row_col == u32::MAX` branch unreachable, and the more specific
error message here will never be returned. Either remove this check or move it
before row validation (and/or incorporate the check into `validate_pair_rows`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
