explicitely cast to uint64_t and prevent overflow

Karsten1987 · Karsten1987 · commit 89b6b6f2a6bf · 2017-08-07T11:09:24.000-07:00
diff --git a/rclcpp/include/rclcpp/time.hpp b/rclcpp/include/rclcpp/time.hpp
@@ -84,7 +84,7 @@ class Time
   Time(int32_t seconds, uint32_t nanoseconds)
   : rcl_time_(get_empty_time_point())
   {
-    uint64_t ns = RCL_S_TO_NS(seconds);
+    uint64_t ns = RCL_S_TO_NS(static_cast<uint64_t>(seconds));
     ns += nanoseconds;
     rcl_time_.nanoseconds = ns;
   }
@@ -99,7 +99,11 @@ class Time
   Time(const builtin_interfaces::msg::Time & time_msg)  // NOLINT
   : rcl_time_(get_empty_time_point())
   {
-    rcl_time_.nanoseconds = RCL_S_TO_NS(time_msg.sec);
+    if (time_msg.sec < 0) {
+      throw std::runtime_error("can't convert a negative time msg to rclcpp::Time");
+    }
+
+    rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<uint64_t>(time_msg.sec));
     rcl_time_.nanoseconds += time_msg.nanosec;
   }
 
@@ -114,8 +118,12 @@ class Time
   void
   operator=(const builtin_interfaces::msg::Time & time_msg)
   {
+    if (time_msg.sec < 0) {
+      throw std::runtime_error("can't convert a negative time msg to rclcpp::Time");
+    }
+
     auto time_point = get_empty_time_point();
-    time_point.nanoseconds = RCL_S_TO_NS(time_msg.sec);
+    time_point.nanoseconds = RCL_S_TO_NS(static_cast<uint64_t>(time_msg.sec));
     time_point.nanoseconds += time_msg.nanosec;
 
     this->rcl_time_ = time_point;
@@ -164,6 +172,11 @@ class Time
       throw std::runtime_error("can't add times with different time sources");
     }
 
+    auto ns = rcl_time_.nanoseconds + rhs.rcl_time_.nanoseconds;
+    if (ns < rcl_time_.nanoseconds) {
+      throw std::runtime_error("addition leads to uint64_t overflow");
+    }
+
     return Time(rcl_time_.nanoseconds + rhs.rcl_time_.nanoseconds);
   }
 
@@ -174,6 +187,11 @@ class Time
       throw std::runtime_error("can't add times with different time sources");
     }
 
+    auto ns = rcl_time_.nanoseconds - rhs.rcl_time_.nanoseconds;
+    if (ns > rcl_time_.nanoseconds) {
+      throw std::runtime_error("subtraction leads to uint64_t underflow");
+    }
+
     return Time(rcl_time_.nanoseconds - rhs.rcl_time_.nanoseconds);
   }
 
diff --git a/rclcpp/test/test_time.cpp b/rclcpp/test/test_time.cpp
@@ -14,6 +14,7 @@
 
 #include <gtest/gtest.h>
 
+#include <limits>
 #include <string>
 
 #include "rcl/error_handling.h"
@@ -30,7 +31,7 @@ class TestTime : public ::testing::Test
   }
 };
 
-TEST(TestTime, rate_basics) {
+TEST(TestTime, time_sources) {
   using builtin_interfaces::msg::Time;
   // TODO(Karsten1987): Fix this test once ROS_TIME is implemented
   EXPECT_ANY_THROW(rclcpp::Time::now<RCL_ROS_TIME>());
@@ -61,8 +62,20 @@ TEST(TestTime, convertions) {
   msg.nanosec = 67890;
 
   rclcpp::Time time = msg;
-  EXPECT_EQ(RCL_S_TO_NS(msg.sec) + static_cast<uint64_t>(msg.nanosec), time.nanoseconds());
+  EXPECT_EQ(
+    RCL_S_TO_NS(static_cast<uint64_t>(msg.sec)) + static_cast<uint64_t>(msg.nanosec),
+    time.nanoseconds());
   EXPECT_EQ(msg.sec, RCL_NS_TO_S(time.nanoseconds()));
+
+  builtin_interfaces::msg::Time negative_time_msg;
+  negative_time_msg.sec = -1;
+  negative_time_msg.nanosec = 1;
+  try {
+    rclcpp::Time negative_time = negative_time_msg;
+    FAIL();
+  } catch (const std::exception &) {
+    SUCCEED();
+  }
 }
 
 TEST(TestTime, operators) {
@@ -83,3 +96,11 @@ TEST(TestTime, operators) {
   EXPECT_EQ(sub.nanoseconds(), young.nanoseconds() - old.nanoseconds());
   EXPECT_EQ(sub, young - old);
 }
+
+TEST(TestTime, overflows) {
+  rclcpp::Time max(std::numeric_limits<uint64_t>::max());
+  rclcpp::Time one(1);
+
+  EXPECT_ANY_THROW(max + one);
+  EXPECT_ANY_THROW(one - max);
+}
