fix(gax): prevent truncation/overflow when converting time values (#3095

) #1872 (comment) --------- Co-authored-by: Lawrence Qiu <[email protected]>
googleapis · Aug 29, 2024 · 699074e · 699074e
1 parent 92bdf4e
commit 699074e
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 4 deletions.
diff --git a/gax-java/gax/src/main/java/com/google/api/gax/util/TimeConversionUtils.java b/gax-java/gax/src/main/java/com/google/api/gax/util/TimeConversionUtils.java
@@ -35,27 +35,27 @@ public static java.time.Duration toJavaTimeDuration(org.threeten.bp.Duration sou
     if (source == null) {
       return null;
     }
-    return java.time.Duration.ofNanos(source.toNanos());
+    return java.time.Duration.ofSeconds(source.getSeconds(), source.getNano());
   }
 
   public static org.threeten.bp.Duration toThreetenDuration(java.time.Duration source) {
     if (source == null) {
       return null;
     }
-    return org.threeten.bp.Duration.ofNanos(source.toNanos());
+    return org.threeten.bp.Duration.ofSeconds(source.getSeconds(), source.getNano());
   }
 
   public static java.time.Instant toJavaTimeInstant(org.threeten.bp.Instant source) {
     if (source == null) {
       return null;
     }
-    return java.time.Instant.ofEpochMilli(source.toEpochMilli());
+    return java.time.Instant.ofEpochSecond(source.getEpochSecond(), source.getNano());
   }
 
   public static org.threeten.bp.Instant toThreetenInstant(java.time.Instant source) {
     if (source == null) {
       return null;
     }
-    return org.threeten.bp.Instant.ofEpochMilli(source.toEpochMilli());
+    return org.threeten.bp.Instant.ofEpochSecond(source.getEpochSecond(), source.getNano());
   }
 }
diff --git a/gax-java/gax/src/test/java/com/google/api/gax/util/TimeConversionUtilsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/util/TimeConversionUtilsTest.java
@@ -37,6 +37,12 @@
 
 public class TimeConversionUtilsTest {
 
+  // 0.999999999 seconds (1 second - 1 nano) - we need to subtract the nano or the Duration would
+  // overflow otherwise
+  final long MAX_DURATION_NANOS =
+      1 * 1000 /*=1 micro*/ * 1000 /*=1 milli*/ * 1000 /*=1 second*/ - 1;
+
+  // Arbitrary durations/instants to confirm conversion works as expected
   final org.threeten.bp.Duration ttDuration = org.threeten.bp.Duration.ofMillis(123);
   final org.threeten.bp.Instant ttInstant = org.threeten.bp.Instant.ofEpochMilli(123);
   final java.time.Duration jtDuration = java.time.Duration.ofMillis(345);
@@ -69,4 +75,44 @@ void testToThreetenTimeInstant_validInput_succeeds() {
         jtInstant.toEpochMilli(), TimeConversionUtils.toThreetenInstant(jtInstant).toEpochMilli());
     assertNull(TimeConversionUtils.toThreetenInstant(null));
   }
+
+  @Test
+  void testToThreeteenInstant_bigInput_doesNotOverflow() {
+    // defaults to MAX_SECONDS plus the max value of long for the nanos part
+    java.time.Instant jtInstant = java.time.Instant.MAX;
+    org.threeten.bp.Instant ttInstant = TimeConversionUtils.toThreetenInstant(jtInstant);
+    assertEquals(jtInstant.getEpochSecond(), ttInstant.getEpochSecond());
+    assertEquals(jtInstant.getNano(), ttInstant.getNano());
+  }
+
+  @Test
+  void testToJavaTimeInstant_bigInput_doesNotOverflow() {
+    // defaults to MAX_SECONDS plus the max value of long for the nanos part
+    org.threeten.bp.Instant ttInstant = org.threeten.bp.Instant.MAX;
+    java.time.Instant jtInstant = TimeConversionUtils.toJavaTimeInstant(ttInstant);
+    assertEquals(jtInstant.getEpochSecond(), ttInstant.getEpochSecond());
+    assertEquals(jtInstant.getNano(), ttInstant.getNano());
+  }
+
+  @Test
+  void testToThreeteenDuration_bigInput_doesNotOverflow() {
+    // we use the max long value for the seconds part and an arbitrary int for the nanos part, so we
+    // can confirm that both components are preserved
+    java.time.Duration jtDuration =
+        java.time.Duration.ofSeconds(Long.MAX_VALUE, MAX_DURATION_NANOS);
+    org.threeten.bp.Duration ttDuration = TimeConversionUtils.toThreetenDuration(jtDuration);
+    assertEquals(jtDuration.getSeconds(), ttDuration.getSeconds());
+    assertEquals(jtDuration.getNano(), ttDuration.getNano());
+  }
+
+  @Test
+  void testToJavaTimeDuration_bigInput_doesNotOverflow() {
+    // we use the max long value for the seconds part and an arbitrary int for the nanos part, so we
+    // can confirm that both components are preserved
+    org.threeten.bp.Duration ttDuration =
+        org.threeten.bp.Duration.ofSeconds(Long.MAX_VALUE, MAX_DURATION_NANOS);
+    java.time.Duration jtDuration = TimeConversionUtils.toJavaTimeDuration(ttDuration);
+    assertEquals(jtDuration.getSeconds(), ttDuration.getSeconds());
+    assertEquals(jtDuration.getNano(), ttDuration.getNano());
+  }
 }