fix: check weight is zero

Author: proost

tdigest/include/tdigest_impl.hpp

@@ -23,6 +23,7 @@
 #include <algorithm>
 #include <cmath>
 #include <sstream>
+#include <type_traits>
 
 #include "common_defs.hpp"
 #include "memory_operations.hpp"
@@ -43,6 +44,14 @@ inline void check_not_infinite(T value, const char* name) {
   }
 }
 
+template<typename T>
+inline void check_non_zero(T value, const char* name) {
+  static_assert(std::is_arithmetic<T>::value, "T must be an arithmetic type");
+  if (value == 0) {
+    throw std::invalid_argument(std::string(name) + " must not be zero");
+  }
+}
+
 template<typename T, typename A>
 tdigest<T, A>::tdigest(uint16_t k, const A& allocator):
 tdigest(false, k, std::numeric_limits<T>::infinity(), -std::numeric_limits<T>::infinity(), vector_centroid(allocator), 0, vector_t(allocator))
@@ -437,6 +446,8 @@ tdigest<T, A> tdigest<T, A>::deserialize(std::istream& is, const A& allocator) {
   for (const auto& c: centroids) {
     check_not_nan(c.get_mean(), "centroid mean");
     check_not_infinite(c.get_mean(), "centroid mean");
+    check_non_zero(c.get_weight(), "centroid weight");
+
     weight += c.get_weight();
   }
   for (const auto& value: buffer) {
@@ -508,6 +519,8 @@ tdigest<T, A> tdigest<T, A>::deserialize(const void* bytes, size_t size, const A
   for (const auto& c: centroids) {
     check_not_nan(c.get_mean(), "centroid mean");
     check_not_infinite(c.get_mean(), "centroid mean");
+    check_non_zero(c.get_weight(), "centroid weight");
+
     weight += c.get_weight();
   }
   for (const auto& value: buffer) {
@@ -542,6 +555,8 @@ tdigest<T, A> tdigest<T, A>::deserialize_compat(std::istream& is, const A& alloc
       const auto weight_double = read_big_endian<double>(is);
       check_not_nan(weight_double, "centroid weight");
       check_not_infinite(weight_double, "centroid weight");
+      check_non_zero(weight_double, "centroid weight");
+
       const auto mean = read_big_endian<double>(is);
       check_not_nan(mean, "centroid mean");
       check_not_infinite(mean, "centroid mean");

tdigest/test/tdigest_test.cpp

@@ -33,6 +33,7 @@ constexpr size_t counts_size = 8;
 constexpr size_t min_offset = header_size + counts_size;
 constexpr size_t max_offset = min_offset + sizeof(double);
 constexpr size_t first_centroid_mean_offset = min_offset + sizeof(double) * 2;
+constexpr size_t first_centroid_weight_offset = first_centroid_mean_offset + sizeof(double);
 constexpr size_t first_buffered_value_offset = first_centroid_mean_offset;
 constexpr size_t single_value_offset = header_size;
 
@@ -582,4 +583,22 @@ TEST_CASE("deserialize bytes rejects infinity buffered value", "[tdigest]") {
   REQUIRE_THROWS_AS(tdigest_double::deserialize(bytes.data(), bytes.size()), std::invalid_argument);
 }
 
+TEST_CASE("deserialize bytes rejects zero centroid weight", "[tdigest]") {
+  tdigest_double td(100);
+  for (int i = 0; i < 10; ++i) td.update(i);
+  auto bytes = td.serialize();
+  write_bytes(bytes, first_centroid_weight_offset, static_cast<uint64_t>(0));
+  REQUIRE_THROWS_AS(tdigest_double::deserialize(bytes.data(), bytes.size()), std::invalid_argument);
+}
+
+TEST_CASE("deserialize stream rejects zero centroid weight", "[tdigest]") {
+  tdigest_double td(100);
+  for (int i = 0; i < 10; ++i) td.update(i);
+  auto bytes = td.serialize();
+  std::string data(reinterpret_cast<const char*>(bytes.data()), bytes.size());
+  write_bytes(data, first_centroid_weight_offset, static_cast<uint64_t>(0));
+  std::istringstream is(data, std::ios::binary);
+  REQUIRE_THROWS_AS(tdigest_double::deserialize(is), std::invalid_argument);
+}
+
 } /* namespace datasketches */