From f49bca16dba4a90107178268d7e7a8c437139067 Mon Sep 17 00:00:00 2001 From: tison Date: Tue, 11 Aug 2026 09:36:00 +0800 Subject: [PATCH 1/2] fix(cpc): classify malformed image fields as invalid data --- CHANGELOG.md | 1 + datasketches/src/cpc/sketch.rs | 7 ++----- datasketches/src/cpc/wrapper.rs | 7 ++----- datasketches/tests/cpc_test/wrapper.rs | 23 +++++++++++++++++++++++ 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce8552a6..0b705ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ All significant changes to this project will be documented in this file. ### Bug fixes * `FrequentItemsSketch::serialize` now writes the full 8-byte preamble for an empty sketch, matching the Java and C++ encoding. Empty sketches previously serialized to 6 bytes, which `FrequentItemsSketch::deserialize` rejected with an insufficient-data error. +* `CpcSketch` and `CpcWrapper` now classify out-of-range fields in serialized images as `InvalidData` rather than `InvalidArgument`. ## v0.3.0 (2026-05-18) diff --git a/datasketches/src/cpc/sketch.rs b/datasketches/src/cpc/sketch.rs index 071d35fb..c90b875d 100644 --- a/datasketches/src/cpc/sketch.rs +++ b/datasketches/src/cpc/sketch.rs @@ -630,13 +630,10 @@ impl CpcSketch { ErrorKind::InvalidData, )?; if !(MIN_LG_K..=MAX_LG_K).contains(&lg_k) { - return Err(Error::invalid_argument(format!( - "lg_k out of range; got {}", - lg_k - ))); + return Err(Error::deserial(format!("lg_k out of range; got {}", lg_k))); } if first_interesting_column > 63 { - return Err(Error::invalid_argument(format!( + return Err(Error::deserial(format!( "first_interesting_column out of range; got {}", first_interesting_column ))); diff --git a/datasketches/src/cpc/wrapper.rs b/datasketches/src/cpc/wrapper.rs index 6fb1ad91..51f9f9dc 100644 --- a/datasketches/src/cpc/wrapper.rs +++ b/datasketches/src/cpc/wrapper.rs @@ -62,13 +62,10 @@ impl CpcWrapper { .read_u8() .map_err(insufficient_data("first_interesting_column"))?; if !(MIN_LG_K..=MAX_LG_K).contains(&lg_k) { - return Err(Error::invalid_argument(format!( - "lg_k out of range; got {}", - lg_k - ))); + return Err(Error::deserial(format!("lg_k out of range; got {}", lg_k))); } if first_interesting_column > 63 { - return Err(Error::invalid_argument(format!( + return Err(Error::deserial(format!( "first_interesting_column out of range; got {}", first_interesting_column ))); diff --git a/datasketches/tests/cpc_test/wrapper.rs b/datasketches/tests/cpc_test/wrapper.rs index 6355ed02..c791f1b8 100644 --- a/datasketches/tests/cpc_test/wrapper.rs +++ b/datasketches/tests/cpc_test/wrapper.rs @@ -19,6 +19,7 @@ use datasketches::common::NumStdDev; use datasketches::cpc::CpcSketch; use datasketches::cpc::CpcUnion; use datasketches::cpc::CpcWrapper; +use datasketches::error::ErrorKind; use googletest::assert_that; use googletest::prelude::contains_substring; use googletest::prelude::eq; @@ -90,3 +91,25 @@ fn test_is_compressed() { contains_substring("only compressed sketches are supported") ); } + +#[test] +fn test_invalid_image_fields_are_invalid_data() { + let original = CpcSketch::new(10).serialize(); + + for (index, value, field) in [ + (3, 3, "lg_k"), + (3, 27, "lg_k"), + (4, 64, "first_interesting_column"), + ] { + let mut bytes = original.clone(); + bytes[index] = value; + + let sketch_err = CpcSketch::deserialize(&bytes).unwrap_err(); + assert_that!(sketch_err.kind(), eq(ErrorKind::InvalidData)); + assert_that!(sketch_err.message(), contains_substring(field)); + + let wrapper_err = CpcWrapper::new(&bytes).unwrap_err(); + assert_that!(wrapper_err.kind(), eq(ErrorKind::InvalidData)); + assert_that!(wrapper_err.message(), contains_substring(field)); + } +} From 2386e694c7d4ed8b177d1e65b21585bd039c75f4 Mon Sep 17 00:00:00 2001 From: tison Date: Tue, 11 Aug 2026 09:36:10 +0800 Subject: [PATCH 2/2] refactor: clarify internal intersection result naming --- datasketches/src/thetafamily/common/intersection.rs | 4 ++-- datasketches/src/thetafamily/common/jaccard_similarity.rs | 2 +- datasketches/src/thetafamily/theta/intersection.rs | 2 +- datasketches/src/thetafamily/tuple/intersection.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/datasketches/src/thetafamily/common/intersection.rs b/datasketches/src/thetafamily/common/intersection.rs index e7a61720..6d0ccd88 100644 --- a/datasketches/src/thetafamily/common/intersection.rs +++ b/datasketches/src/thetafamily/common/intersection.rs @@ -243,8 +243,8 @@ where self.table.estimated_size() } - /// Return the current intersection state as compact-sketch parts. - pub fn result(&self, ordered: bool) -> CompactSketchParts + /// Returns the current intersection state as compact-sketch parts. + pub fn to_compact_parts(&self, ordered: bool) -> CompactSketchParts where E: Clone, { diff --git a/datasketches/src/thetafamily/common/jaccard_similarity.rs b/datasketches/src/thetafamily/common/jaccard_similarity.rs index 94a3d38c..45903849 100644 --- a/datasketches/src/thetafamily/common/jaccard_similarity.rs +++ b/datasketches/src/thetafamily/common/jaccard_similarity.rs @@ -194,7 +194,7 @@ where let mut intersection = IntersectionState::new(seed, NoopMergePolicy); intersection.update(KeyEntries(sketch_a))?; intersection.update(KeyEntries(sketch_b))?; - let intersection = intersection.result(false); + let intersection = intersection.to_compact_parts(false); let intersection_count = intersection .entries .iter() diff --git a/datasketches/src/thetafamily/theta/intersection.rs b/datasketches/src/thetafamily/theta/intersection.rs index e3bfdeeb..3f2f0f85 100644 --- a/datasketches/src/thetafamily/theta/intersection.rs +++ b/datasketches/src/thetafamily/theta/intersection.rs @@ -83,7 +83,7 @@ impl ThetaIntersection { self.state.has_result(), "ThetaIntersection::to_sketch() called before first update()" ); - let parts = self.state.result(ordered); + let parts = self.state.to_compact_parts(ordered); CompactThetaSketch::from_parts( parts .entries diff --git a/datasketches/src/thetafamily/tuple/intersection.rs b/datasketches/src/thetafamily/tuple/intersection.rs index 23e9fa76..273d7957 100644 --- a/datasketches/src/thetafamily/tuple/intersection.rs +++ b/datasketches/src/thetafamily/tuple/intersection.rs @@ -153,7 +153,7 @@ where self.state.has_result(), "TupleIntersection::to_sketch() called before first update()" ); - let parts = self.state.result(ordered); + let parts = self.state.to_compact_parts(ordered); CompactTupleSketch::from_parts( parts.entries, parts.theta,