tisonkun opened a new issue, #218: URL: https://github.com/apache/datasketches-rust/issues/218
## Description REQ deserialization currently accepts compactor states that cannot be produced by a valid sketch. Those states can later panic in safe public APIs or violate documented return-value contracts. Two concrete examples on current `main`: 1. A single-level image with `num_sections = 0` deserializes successfully. A subsequent `ReqSketch::merge` panics at `1u64 << (self.num_sections - 1)` because the subtraction underflows. 2. A single-level image whose level-0 compactor declares `lg_weight = 63` and contains one item deserializes successfully. Its inclusive rank for that item is approximately `9.22e18`, even though `ReqSketch::rank` promises a result in `[0.0, 1.0]`. Relevant code: - [`Compactor::deserialize` validates only broad `section_size_raw` and `lg_weight` bounds](https://github.com/apache/datasketches-rust/blob/bcb83e629ae86eb8619bb7eb0a372a172c94b79c/datasketches/src/req/compactor.rs#L355-L416) - [`ensure_enough_sections` assumes `num_sections` is nonzero](https://github.com/apache/datasketches-rust/blob/bcb83e629ae86eb8619bb7eb0a372a172c94b79c/datasketches/src/req/compactor.rs#L287-L300) - [`ReqSketch::deserialize` accepts the reconstructed compactors without validating cross-field invariants](https://github.com/apache/datasketches-rust/blob/bcb83e629ae86eb8619bb7eb0a372a172c94b79c/datasketches/src/req/sketch.rs#L661-L714) ## Expected fix Validate wire-controlled compactor state before constructing a `ReqSketch`. At minimum: - Reject `num_sections = 0` and validate it against states the REQ section schedule can produce. - Require each compactor's `lg_weight` to equal its enclosing level index. - Use checked arithmetic to calculate the retained weighted count and require it to equal the serialized stream length `n`. - Return `ErrorKind::InvalidData` for malformed serialized state instead of `InvalidArgument`. - Ensure malformed input is rejected by `deserialize` rather than causing a later public operation to panic or return a rank outside `[0.0, 1.0]`. Please add focused malformed-image regression tests to `datasketches/tests/serde_tests/req.rs`, including the two cases above. The existing `single_level_image` helper can be reused. Introduced with the REQ implementation in #204. -- 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]
