Add debug checks for valid duration and timestamp

PiperOrigin-RevId: 730982021

Author: jcking

common/legacy_value.cc

@@ -988,10 +988,10 @@ absl::Status ModernValue(google::protobuf::Arena* arena,
       return absl::OkStatus();
     }
     case CelValue::Type::kDuration:
-      result = DurationValue{legacy_value.DurationOrDie()};
+      result = UnsafeDurationValue(legacy_value.DurationOrDie());
       return absl::OkStatus();
     case CelValue::Type::kTimestamp:
-      result = TimestampValue{legacy_value.TimestampOrDie()};
+      result = UnsafeTimestampValue(legacy_value.TimestampOrDie());
       return absl::OkStatus();
     case CelValue::Type::kList:
       result = ListValue{common_internal::LegacyListValue{
@@ -1077,10 +1077,10 @@ absl::StatusOr<google::api::expr::runtime::CelValue> LegacyValue(
       return common_internal::LegacyTrivialStructValue(arena, modern_value);
     case ValueKind::kDuration:
       return CelValue::CreateUncheckedDuration(
-          Cast<DurationValue>(modern_value).NativeValue());
+          modern_value.GetDuration().NativeValue());
     case ValueKind::kTimestamp:
       return CelValue::CreateTimestamp(
-          Cast<TimestampValue>(modern_value).NativeValue());
+          modern_value.GetTimestamp().NativeValue());
     case ValueKind::kList:
       return common_internal::LegacyTrivialListValue(arena, modern_value);
     case ValueKind::kMap:
@@ -1133,9 +1133,9 @@ absl::StatusOr<Value> FromLegacyValue(google::protobuf::Arena* arena,
           reinterpret_cast<uintptr_t>(message_wrapper.legacy_type_info())};
     }
     case CelValue::Type::kDuration:
-      return DurationValue(legacy_value.DurationOrDie());
+      return UnsafeDurationValue(legacy_value.DurationOrDie());
     case CelValue::Type::kTimestamp:
-      return TimestampValue(legacy_value.TimestampOrDie());
+      return UnsafeTimestampValue(legacy_value.TimestampOrDie());
     case CelValue::Type::kList:
       return ListValue{common_internal::LegacyListValue{
           reinterpret_cast<uintptr_t>(legacy_value.ListOrDie())}};

common/values/duration_value.h

@@ -22,13 +22,16 @@
 #include <string>
 
 #include "absl/base/nullability.h"
+#include "absl/log/absl_check.h"
 #include "absl/status/status.h"
 #include "absl/strings/cord.h"
 #include "absl/strings/string_view.h"
 #include "absl/time/time.h"
+#include "absl/utility/utility.h"
 #include "common/type.h"
 #include "common/value_kind.h"
 #include "common/values/values.h"
+#include "internal/time.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/message.h"
@@ -39,14 +42,20 @@ class Value;
 class DurationValue;
 class TypeManager;
 
+DurationValue UnsafeDurationValue(absl::Duration value);
+
 // `DurationValue` represents values of the primitive `duration` type.
 class DurationValue final : private common_internal::ValueMixin<DurationValue> {
  public:
   static constexpr ValueKind kKind = ValueKind::kDuration;
 
-  explicit DurationValue(absl::Duration value) noexcept : value_(value) {}
+  explicit DurationValue(absl::Duration value) noexcept
+      : DurationValue(absl::in_place, value) {
+    ABSL_DCHECK_OK(internal::ValidateDuration(value));
+  }
 
   DurationValue& operator=(absl::Duration value) noexcept {
+    ABSL_DCHECK_OK(internal::ValidateDuration(value));
     value_ = value;
     return *this;
   }
@@ -102,10 +111,17 @@ class DurationValue final : private common_internal::ValueMixin<DurationValue> {
 
  private:
   friend class common_internal::ValueMixin<DurationValue>;
+  friend DurationValue UnsafeDurationValue(absl::Duration value);
+
+  DurationValue(absl::in_place_t, absl::Duration value) : value_(value) {}
 
   absl::Duration value_ = absl::ZeroDuration();
 };
 
+inline DurationValue UnsafeDurationValue(absl::Duration value) {
+  return DurationValue(absl::in_place, value);
+}
+
 inline bool operator==(DurationValue lhs, DurationValue rhs) {
   return static_cast<absl::Duration>(lhs) == static_cast<absl::Duration>(rhs);
 }

common/values/timestamp_value.h

@@ -22,13 +22,16 @@
 #include <string>
 
 #include "absl/base/nullability.h"
+#include "absl/log/absl_check.h"
 #include "absl/status/status.h"
 #include "absl/strings/cord.h"
 #include "absl/strings/string_view.h"
 #include "absl/time/time.h"
+#include "absl/utility/utility.h"
 #include "common/type.h"
 #include "common/value_kind.h"
 #include "common/values/values.h"
+#include "internal/time.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/message.h"
@@ -39,15 +42,21 @@ class Value;
 class TimestampValue;
 class TypeManager;
 
+TimestampValue UnsafeTimestampValue(absl::Time value);
+
 // `TimestampValue` represents values of the primitive `timestamp` type.
 class TimestampValue final
     : private common_internal::ValueMixin<TimestampValue> {
  public:
   static constexpr ValueKind kKind = ValueKind::kTimestamp;
 
-  explicit TimestampValue(absl::Time value) noexcept : value_(value) {}
+  explicit TimestampValue(absl::Time value) noexcept
+      : TimestampValue(absl::in_place, value) {
+    ABSL_DCHECK_OK(internal::ValidateTimestamp(value));
+  }
 
   TimestampValue& operator=(absl::Time value) noexcept {
+    ABSL_DCHECK_OK(internal::ValidateTimestamp(value));
     value_ = value;
     return *this;
   }
@@ -101,10 +110,17 @@ class TimestampValue final
 
  private:
   friend class common_internal::ValueMixin<TimestampValue>;
+  friend TimestampValue UnsafeTimestampValue(absl::Time value);
+
+  TimestampValue(absl::in_place_t, absl::Time value) : value_(value) {}
 
   absl::Time value_ = absl::UnixEpoch();
 };
 
+inline TimestampValue UnsafeTimestampValue(absl::Time value) {
+  return TimestampValue(absl::in_place, value);
+}
+
 inline bool operator==(TimestampValue lhs, TimestampValue rhs) {
   return lhs.NativeValue() == rhs.NativeValue();
 }

internal/BUILD

@@ -308,6 +308,7 @@ cc_library(
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/time",
+        "@com_google_protobuf//:protobuf",
     ],
 )
 

internal/time.cc

@@ -18,8 +18,10 @@
 #include <string>
 
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_format.h"
+#include "absl/strings/string_view.h"
 #include "absl/time/time.h"
 #include "internal/status_macros.h"
 

internal/time.h

@@ -21,6 +21,7 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "absl/time/time.h"
+#include "google/protobuf/util/time_util.h"
 
 namespace cel::internal {
 
@@ -31,7 +32,8 @@ namespace cel::internal {
   // google.protobuf.Duration from protocol buffer messages, which this
   // implementation currently supports.
   // TODO: revisit
-  return absl::Seconds(315576000000) + absl::Nanoseconds(999999999);
+  return absl::Seconds(google::protobuf::util::TimeUtil::kDurationMaxSeconds) +
+         absl::Nanoseconds(google::protobuf::util::TimeUtil::kDurationMaxNanoseconds);
 }
 
     inline absl::Duration
@@ -41,18 +43,22 @@ namespace cel::internal {
   // google.protobuf.Duration from protocol buffer messages, which this
   // implementation currently supports.
   // TODO: revisit
-  return absl::Seconds(-315576000000) + absl::Nanoseconds(-999999999);
+  return absl::Seconds(google::protobuf::util::TimeUtil::kDurationMinSeconds) +
+         absl::Nanoseconds(google::protobuf::util::TimeUtil::kDurationMinNanoseconds);
 }
 
     inline absl::Time
     MaxTimestamp() {
-  return absl::UnixEpoch() + absl::Seconds(253402300799) +
-         absl::Nanoseconds(999999999);
+  return absl::UnixEpoch() +
+         absl::Seconds(google::protobuf::util::TimeUtil::kTimestampMaxSeconds) +
+         absl::Nanoseconds(google::protobuf::util::TimeUtil::kTimestampMaxNanoseconds);
 }
 
     inline absl::Time
     MinTimestamp() {
-  return absl::UnixEpoch() + absl::Seconds(-62135596800);
+  return absl::UnixEpoch() +
+         absl::Seconds(google::protobuf::util::TimeUtil::kTimestampMinSeconds) +
+         absl::Nanoseconds(google::protobuf::util::TimeUtil::kTimestampMinNanoseconds);
 }
 
 absl::Status ValidateDuration(absl::Duration duration);

runtime/internal/convert_constant.cc

@@ -60,10 +60,10 @@ struct ConvertVisitor {
     if (duration >= kDurationHigh || duration <= kDurationLow) {
       return ErrorValue(*DurationOverflowError());
     }
-    return DurationValue(duration);
+    return UnsafeDurationValue(duration);
   }
   absl::StatusOr<cel::Value> operator()(const absl::Time timestamp) {
-    return TimestampValue(timestamp);
+    return UnsafeTimestampValue(timestamp);
   }
 };
 

runtime/standard/time_functions.cc

@@ -400,15 +400,15 @@ absl::Status RegisterUncheckedTimeArithmeticFunctions(
                                                               false),
       BinaryFunctionAdapter<Value, absl::Time, absl::Duration>::WrapFunction(
           [](absl::Time t1, absl::Duration d2) -> Value {
-            return TimestampValue(t1 + d2);
+            return UnsafeTimestampValue(t1 + d2);
           })));
 
   CEL_RETURN_IF_ERROR(registry.Register(
       BinaryFunctionAdapter<Value, absl::Duration,
                             absl::Time>::CreateDescriptor(builtin::kAdd, false),
       BinaryFunctionAdapter<Value, absl::Duration, absl::Time>::WrapFunction(
           [](absl::Duration d2, absl::Time t1) -> Value {
-            return TimestampValue(t1 + d2);
+            return UnsafeTimestampValue(t1 + d2);
           })));
 
   CEL_RETURN_IF_ERROR(registry.Register(
@@ -417,7 +417,7 @@ absl::Status RegisterUncheckedTimeArithmeticFunctions(
                                                               false),
       BinaryFunctionAdapter<Value, absl::Duration, absl::Duration>::
           WrapFunction([](absl::Duration d1, absl::Duration d2) -> Value {
-            return DurationValue(d1 + d2);
+            return UnsafeDurationValue(d1 + d2);
           })));
 
   CEL_RETURN_IF_ERROR(registry.Register(
@@ -427,7 +427,7 @@ absl::Status RegisterUncheckedTimeArithmeticFunctions(
       BinaryFunctionAdapter<Value, absl::Time, absl::Duration>::WrapFunction(
 
           [](absl::Time t1, absl::Duration d2) -> Value {
-            return TimestampValue(t1 - d2);
+            return UnsafeTimestampValue(t1 - d2);
           })));
 
   CEL_RETURN_IF_ERROR(registry.Register(
@@ -436,15 +436,15 @@ absl::Status RegisterUncheckedTimeArithmeticFunctions(
       BinaryFunctionAdapter<Value, absl::Time, absl::Time>::WrapFunction(
 
           [](absl::Time t1, absl::Time t2) -> Value {
-            return DurationValue(t1 - t2);
+            return UnsafeDurationValue(t1 - t2);
           })));
 
   CEL_RETURN_IF_ERROR(registry.Register(
       BinaryFunctionAdapter<Value, absl::Duration, absl::Duration>::
           CreateDescriptor(builtin::kSubtract, false),
       BinaryFunctionAdapter<Value, absl::Duration, absl::Duration>::
           WrapFunction([](absl::Duration d1, absl::Duration d2) -> Value {
-            return DurationValue(d1 - d2);
+            return UnsafeDurationValue(d1 - d2);
           })));
 
   return absl::OkStatus();

runtime/standard/type_conversion_functions.cc

@@ -39,9 +39,7 @@ namespace {
 using ::cel::internal::EncodeDurationToJson;
 using ::cel::internal::EncodeTimestampToJson;
 using ::cel::internal::MaxTimestamp;
-
-// Time representing `9999-12-31T23:59:59.999999999Z`.
-const absl::Time kMaxTime = MaxTimestamp();
+using ::cel::internal::MinTimestamp;
 
 absl::Status RegisterBoolConversionFunctions(FunctionRegistry& registry,
                                              const RuntimeOptions&) {
@@ -356,11 +354,11 @@ absl::Status RegisterTimeConversionFunctions(FunctionRegistry& registry,
                   "String to Timestamp conversion failed"));
             }
             if (enable_timestamp_duration_overflow_errors) {
-              if (ts < absl::UniversalEpoch() || ts > kMaxTime) {
+              if (ts < MinTimestamp() || ts > MaxTimestamp()) {
                 return ErrorValue(absl::OutOfRangeError("timestamp overflow"));
               }
             }
-            return TimestampValue(ts);
+            return UnsafeTimestampValue(ts);
           },
           registry);
 }