From 0fb8160f9c8799b92214b1a0e930457d1e9e5466 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Mon, 12 Aug 2024 11:17:02 -0400 Subject: [PATCH 1/4] fix(gax): prevent truncation/overflow when converting time values --- .../api/gax/util/TimeConversionUtils.java | 8 ++-- .../api/gax/util/TimeConversionUtilsTest.java | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+), 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 index d19fa39fc2..5ee39874b7 100644 --- 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 index e288a06bae..b160ab8079 100644 --- 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 @@ -69,4 +69,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, 123); + 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, 123); + java.time.Duration jtDuration = TimeConversionUtils.toJavaTimeDuration(ttDuration); + assertEquals(jtDuration.getSeconds(), ttDuration.getSeconds()); + assertEquals(jtDuration.getNano(), ttDuration.getNano()); + } } From 329c46743b1aa05b5a22ed8e58b9f9f93f50fe3b Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Tue, 13 Aug 2024 10:39:11 -0400 Subject: [PATCH 2/4] Update gax-java/gax/src/test/java/com/google/api/gax/util/TimeConversionUtilsTest.java Co-authored-by: Lawrence Qiu --- .../java/com/google/api/gax/util/TimeConversionUtilsTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index b160ab8079..6f0c51d582 100644 --- 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 @@ -91,8 +91,7 @@ void testToJavaTimeInstant_bigInput_doesNotOverflow() { @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 + // can confirm that both components are preserved java.time.Duration jtDuration = java.time.Duration.ofSeconds(Long.MAX_VALUE, 123); org.threeten.bp.Duration ttDuration = TimeConversionUtils.toThreetenDuration(jtDuration); assertEquals(jtDuration.getSeconds(), ttDuration.getSeconds()); From 5014c4b55666917c96ac13a95dfea15ee126ed6c Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Tue, 13 Aug 2024 10:39:19 -0400 Subject: [PATCH 3/4] Update gax-java/gax/src/test/java/com/google/api/gax/util/TimeConversionUtilsTest.java Co-authored-by: Lawrence Qiu --- .../java/com/google/api/gax/util/TimeConversionUtilsTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index 6f0c51d582..796aa4fa33 100644 --- 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 @@ -101,8 +101,7 @@ void testToThreeteenDuration_bigInput_doesNotOverflow() { @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 + // can confirm that both components are preserved org.threeten.bp.Duration ttDuration = org.threeten.bp.Duration.ofSeconds(Long.MAX_VALUE, 123); java.time.Duration jtDuration = TimeConversionUtils.toJavaTimeDuration(ttDuration); assertEquals(jtDuration.getSeconds(), ttDuration.getSeconds()); From 86d58553fe3f74a07b605da70ea1bd8416e8394e Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Thu, 15 Aug 2024 12:50:55 -0400 Subject: [PATCH 4/4] add max possible nano quantity to max possible seconds quantity in duration test --- .../google/api/gax/util/TimeConversionUtilsTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 index b160ab8079..a22682f9dc 100644 --- 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,11 @@ 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 * 1000 * 1000 - 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); @@ -93,7 +98,8 @@ 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, 123); + 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()); @@ -104,7 +110,8 @@ 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, 123); + 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());