refactor: harden library usability before 0.5.0 - #253
Merged
Conversation
Returning owned items cloned every retained value while the iterator was documented as zero-allocation. Borrow the items from the sketch so callers can inspect heap-backed values without hidden cloning or allocation.
String lengths are read from untrusted sketch images. Check the declared length against the remaining payload before allocating so a four-byte length field cannot trigger an allocation much larger than the supplied input.
REQ estimation images store stream extrema separately from retained items. Reject images whose extrema do not bound those items so min_item and max_item cannot expose values contradicted by the sketch state.
The mutable query methods perform the same input assertions as TDigest, but their own API docs omitted those contracts. Document them at the call sites so users do not need to follow cross-links to discover when a query can panic.
REQ confidence bounds support only one, two, or three standard deviations, but a u8 accepted every value and differed from the rest of the crate. Reuse NumStdDev so invalid confidence levels are unrepresentable at the call site.
Update counts are unsigned and zero already returns early, so the following "not negative" assertions were always true. Remove the dead checks and their misleading signed-count error message.
REQ is new since the latest release, so its deserialization fix is part of the initial 0.5.0 contract rather than a user-visible change between releases. Keep the changelog focused on the feature's final behavior.
tisonkun
marked this pull request as ready for review
August 31, 2026 12:06
tisonkun
enabled auto-merge (squash)
August 31, 2026 12:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses a small set of release-facing issues found during a pre-0.5.0 audit:
NumStdDevtype for REQ rank confidence bounds so unsupported confidence levels are not representable;FrequentItemsSketch<String>allocates from untrusted input;REQ is still unreleased, so tightening its iterator and confidence-bound APIs here avoids carrying immediately regrettable interfaces into 0.5.0.
Audit boundary
The audit intentionally excluded changes without a concrete failure mode or an established contract. In particular:
Commit structure
Each concern is isolated in a reviewable commit whose message records the reason for the change:
refactor(req): borrow items from the public iteratorfix(frequencies): validate string lengths before allocationfix(req): validate serialized extrema against retained itemsdocs(tdigest): state mutable query panic contractsrefactor(req): type the confidence level parameterrefactor(frequencies): remove impossible count assertionsdocs(changelog): omit unreleased REQ hardeningValidation
cargo x prepare-testdatacargo x checkcargo x testcargo x lintcargo package --list -p datasketchescargo publish --dry-run --locked -p datasketchesThe publish dry-run packaged 99 files (1.9 MiB, 274.9 KiB compressed) and verified the package with Rust/Cargo 1.86.0.