Added numeric value guards to ValueMetric.

[ Merge of http://go/wvgerrit/137951 ]

ValueMetric was not checking whether certain unsigned types will
overflow when casted to protobuf's signed int64 type.  This CL
adds a max value to prevent potential overflows.

The value metric has also been updated to an emum to represent its
possible three state, as opposed to using two boolean flags.  For basic
value metrics, this saves about 4 bytes of space.  This works out to
being ~100 bytes per crypto session, ~20 bytes per CDM session and ~20
bytes per engine metrics.

Bug: 204946540
Test: Metric unit tests
Change-Id: I688bece8f31cccaf2a25bd8f99d9b080b0efd0de

libwvdrmengine/cdm/metrics/include/value_metric.h

@@ -15,109 +15,103 @@
 
 namespace wvcdm {
 namespace metrics {
-// Internal namespace for helper methods.
+namespace internal {
-namespace impl {
 // Helper function for setting a value in the proto.
 template <typename T>
 void SetValue(drm_metrics::ValueMetric* value_proto, const T& value);
-}  // namespace impl
+}  // namespace internal
 
 // The ValueMetric class supports storing a single, overwritable value or an
 // error code. This class can be serialized to a drm_metrics::ValueMetric proto.
 // If an error or value was not provided, this metric will serialize to nullptr.
 //
 // Example Usage:
-//   Metric<string> cdm_version().Record("a.b.c.d");
+//   ValueMetric<string> cdm_version().Record("a.b.c.d");
 //   std::unique_ptr<drm_metrics::ValueMetric> mymetric(
 //       cdm_version.ToProto());
 //
 // Example Error Usage:
 //
-//   Metric<string> cdm_version().SetError(error_code);
+//   ValueMetric<string> cdm_version().SetError(error_code);
 //   std::unique_ptr<drm_metrics::ValueMetric> mymetric(
 //       cdm_version.ToProto());
 //
 template <typename T>
 class ValueMetric {
  public:
-  // Constructs a metric with the given metric name.
+  ValueMetric() {}
-  explicit ValueMetric()
-      : error_code_(0), has_error_(false), has_value_(false) {}
 
   // Record the value of the metric.
   void Record(const T& value) {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     value_ = value;
-    has_value_ = true;
+    state_ = kHasValue;
-    has_error_ = false;
   }
 
   // Set the error code if an error was encountered.
   void SetError(int error_code) {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     error_code_ = error_code;
-    has_value_ = false;
+    state_ = kHasError;
-    has_error_ = true;
   }
 
   bool HasValue() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
-    return has_value_;
+    return state_ == kHasValue;
   }
   const T& GetValue() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     return value_;
   }
 
   bool HasError() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
-    return has_error_;
+    return state_ == kHasError;
   }
   int GetError() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     return error_code_;
   }
 
   // Clears the indicators that the metric or error was set.
   void Clear() {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
-    has_value_ = false;
+    state_ = kNone;
-    has_error_ = false;
   }
 
   // Returns a new ValueMetric proto containing the metric value or the
   // error code. If neither the error or value are set, it returns nullptr.
   drm_metrics::ValueMetric* ToProto() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
-    if (has_error_) {
+    switch (state_) {
-      drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
+      case kHasValue: {
-      value_proto->set_error_code(error_code_);
+        drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
-      return value_proto;
+        internal::SetValue(value_proto, value_);
-    } else if (has_value_) {
+        return value_proto;
-      drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
+      }
-      impl::SetValue(value_proto, value_);
+      case kHasError: {
-      return value_proto;
+        drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
+        value_proto->set_error_code(error_code_);
+        return value_proto;
+      }
+      case kNone:
+      default:
+        return nullptr;
     }
-
-    return nullptr;
   }
 
  private:
-  T value_;
+  enum ValueState { kNone, kHasValue, kHasError };
-  int error_code_;
-  bool has_error_;
-  bool has_value_;
 
-  /*
+  T value_;
-   * This locks the internal state of the value metric to ensure safety
+  int error_code_ = 0;
-   * across multiple threads preventing the caller from worrying about
+  ValueState state_ = kNone;
-   * locking.
+
-   *
+  // This locks the internal state of the value metric to ensure safety
-   * This field must be declared mutable because the lock must be takeable even
+  // across multiple threads preventing the caller from worrying about
-   * in const methods.
+  // locking.
-   */
+  mutable std::mutex mutex_;
-  mutable std::mutex internal_lock_;
+};
-};  // class ValueMetric
 }  // namespace metrics
 }  // namespace wvcdm
 #endif  // WVCDM_METRICS_VALUE_METRIC_H_

libwvdrmengine/cdm/metrics/src/value_metric.cpp

@@ -6,89 +6,111 @@
 // ValueMetric class.
 #include "value_metric.h"
 
+#include <limits>
+
 #include "OEMCryptoCENC.h"
 #include "metrics_collections.h"
 #include "wv_cdm_types.h"
 
 namespace wvcdm {
 namespace metrics {
-namespace impl {
+namespace internal {
+namespace {
+// Max value of protobuf's int64 type.
+constexpr uint64_t kMaxValueCmp =
+    static_cast<uint64_t>(std::numeric_limits<int64_t>::max());
+constexpr int64_t kMaxValue = std::numeric_limits<int64_t>::max();
+
+template <typename T>
+constexpr bool WillOverflow(T value) {
+  return static_cast<uint64_t>(value) > kMaxValueCmp;
+}
+
+// Prevents an overflow of very large numbers.  If the value attempted to
+// be assigned is greater than the maximum value supported by protobuf's
+// int64 type, then the value is capped at the max.
+template <typename T>
+constexpr int64_t SafeAssign(T value) {
+  return WillOverflow(value) ? kMaxValue : static_cast<int64_t>(value);
+}
+}  // namespace
+
 template <>
 void SetValue<int>(drm_metrics::ValueMetric* value_proto, const int& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<long>(drm_metrics::ValueMetric* value_proto, const long& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<long long>(drm_metrics::ValueMetric* value_proto,
                          const long long& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned int>(drm_metrics::ValueMetric* value_proto,
                             const unsigned int& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned short>(drm_metrics::ValueMetric* value_proto,
                               const unsigned short& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned long>(drm_metrics::ValueMetric* value_proto,
                              const unsigned long& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned long long>(drm_metrics::ValueMetric* value_proto,
                                   const unsigned long long& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<bool>(drm_metrics::ValueMetric* value_proto, const bool& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(value ? 1 : 0);
 }
 
 template <>
 void SetValue<OEMCrypto_HDCP_Capability>(
     drm_metrics::ValueMetric* value_proto,
     const OEMCrypto_HDCP_Capability& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<OEMCrypto_ProvisioningMethod>(
     drm_metrics::ValueMetric* value_proto,
     const OEMCrypto_ProvisioningMethod& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<OEMCryptoInitializationMode>(
     drm_metrics::ValueMetric* value_proto,
     const OEMCryptoInitializationMode& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<CdmSecurityLevel>(drm_metrics::ValueMetric* value_proto,
                                 const CdmSecurityLevel& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<CdmUsageSupportType>(drm_metrics::ValueMetric* value_proto,
                                    const CdmUsageSupportType& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
@@ -102,6 +124,6 @@ void SetValue<std::string>(drm_metrics::ValueMetric* value_proto,
                            const std::string& value) {
   value_proto->set_string_value(value);
 }
-}  // namespace impl
+}  // namespace internal
 }  // namespace metrics
 }  // namespace wvcdm

libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp

@@ -57,5 +57,32 @@ TEST(ValueMetricTest, SetError) {
   ASSERT_EQ(7, metric_proto->error_code());
   ASSERT_FALSE(metric_proto->has_int_value());
 }
+
+TEST(ValueMetricTest, ValueOverflow) {
+  constexpr int64_t kMaxValue = std::numeric_limits<int64_t>::max();
+  constexpr uint64_t kMaxU64 = std::numeric_limits<uint64_t>::max();
+  constexpr size_t kMaxSizeT = std::numeric_limits<size_t>::max();
+
+  ValueMetric<uint64_t> metric_u64;
+  metric_u64.Record(kMaxU64);
+  // Runtime value should not be capped.
+  ASSERT_EQ(kMaxU64, metric_u64.GetValue());
+
+  std::unique_ptr<drm_metrics::ValueMetric> metric_proto(metric_u64.ToProto());
+  // Proto value should be capped.
+  ASSERT_EQ(kMaxValue, metric_proto->int_value());
+  ASSERT_FALSE(metric_proto->has_error_code());
+
+  ValueMetric<uint64_t> metric_z;
+  metric_z.Record(kMaxSizeT);
+  ASSERT_EQ(kMaxSizeT, metric_z.GetValue());
+  metric_proto.reset(metric_z.ToProto());
+  if (sizeof(int64_t) <= sizeof(size_t) &&
+      static_cast<size_t>(kMaxValue) <= kMaxSizeT) {
+    ASSERT_EQ(kMaxValue, metric_proto->int_value());
+  } else {
+    ASSERT_GT(kMaxValue, metric_proto->int_value());
+  }
+}
 }  // namespace metrics
 }  // namespace wvcdm