From 8617354feeeed41753e6cda14477943ad284019b Mon Sep 17 00:00:00 2001 From: Kevin Wilfong Date: Tue, 4 Mar 2025 16:06:20 -0800 Subject: [PATCH] Fix silent overflow in DateTimeEncoding.pack --- .../presto/common/type/DateTimeEncoding.java | 6 ++ .../operator/scalar/DateTimeFunctions.java | 28 ++++++++++ .../scalar/TestDateTimeFunctionsBase.java | 55 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/presto-common/src/main/java/com/facebook/presto/common/type/DateTimeEncoding.java b/presto-common/src/main/java/com/facebook/presto/common/type/DateTimeEncoding.java index ba7d6141eed3c..e09550b23c0f3 100644 --- a/presto-common/src/main/java/com/facebook/presto/common/type/DateTimeEncoding.java +++ b/presto-common/src/main/java/com/facebook/presto/common/type/DateTimeEncoding.java @@ -15,6 +15,7 @@ import static com.facebook.presto.common.type.TimeZoneKey.getTimeZoneKey; import static com.facebook.presto.common.type.TimeZoneKey.getTimeZoneKeyForOffset; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; public final class DateTimeEncoding @@ -25,9 +26,14 @@ private DateTimeEncoding() private static final int TIME_ZONE_MASK = 0xFFF; private static final int MILLIS_SHIFT = 12; + private static final long MAX_MILLIS = 0x7FFFFFFFFFFFFL; + private static final long MIN_MILLIS = (MAX_MILLIS + 1) * -1; private static long pack(long millisUtc, short timeZoneKey) { + if (millisUtc > MAX_MILLIS || millisUtc < MIN_MILLIS) { + throw new ArithmeticException(format("TimestampWithTimeZone overflow: %s ms", millisUtc)); + } return (millisUtc << MILLIS_SHIFT) | (timeZoneKey & TIME_ZONE_MASK); } diff --git a/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java b/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java index bf8d8e74afedb..6760fa7b37936 100644 --- a/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java +++ b/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java @@ -50,6 +50,7 @@ import static com.facebook.presto.operator.scalar.QuarterOfYearDateTimeField.QUARTER_OF_YEAR; import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; +import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; import static com.facebook.presto.spi.function.SqlFunctionVisibility.HIDDEN; import static com.facebook.presto.type.DateTimeOperators.modulo24Hour; import static com.facebook.presto.util.DateTimeZoneIndex.extractZoneOffsetMinutes; @@ -128,6 +129,9 @@ public static long currentTime(SqlFunctionProperties properties) catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @Description("current time without time zone") @@ -164,6 +168,9 @@ public static long currentTimestamp(SqlFunctionProperties properties) catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @Description("current timestamp without time zone") @@ -215,6 +222,9 @@ public static long fromUnixTime(@SqlType(StandardTypes.DOUBLE) double unixTime, catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @ScalarFunction("from_unixtime") @@ -231,6 +241,9 @@ public static long fromUnixTime(@SqlType(StandardTypes.DOUBLE) double unixTime, catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @ScalarFunction("to_unixtime") @@ -308,6 +321,9 @@ public static long fromISO8601Timestamp(SqlFunctionProperties properties, @SqlTy catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @ScalarFunction("from_iso8601_date") @@ -352,6 +368,9 @@ public static long timestampAtTimeZone(@SqlType(StandardTypes.TIMESTAMP_WITH_TIM catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @ScalarFunction(value = "at_timezone", visibility = HIDDEN) @@ -369,6 +388,9 @@ public static long timestampAtTimeZone(@SqlType(StandardTypes.TIMESTAMP_WITH_TIM catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } @Description("truncate to the specified precision in the session timezone") @@ -642,6 +664,9 @@ public static long parseDatetime(SqlFunctionProperties properties, @SqlType("var catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } private static DateTime parseDateTimeHelper(DateTimeFormatter formatter, String datetimeString) @@ -1451,6 +1476,9 @@ private static long timeAtTimeZone(SqlFunctionProperties properties, long timeWi catch (IllegalArgumentException e) { throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage(), e); } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, e.getMessage(), e); + } } // HACK WARNING! diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java index 53186af9f835e..9fdd81ad2d351 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java @@ -248,6 +248,24 @@ public void testFromUnixTimeWithTimeZone() assertFunction(format("from_unixtime(7200, '%s')", zoneId), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); } + @Test + public void testFromUnixTimeWithTimeZoneOverflow() + { + String zoneId = "Asia/Shanghai"; + + // Test the largest possible valid value. + DateTime expected = new DateTime(73326, 9, 12, 4, 14, 45, DateTimeZone.forID(zoneId)); + assertFunction(format("from_unixtime(2251799813685, '%s')", zoneId), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); + + // Test the smallest possible valid value. + expected = new DateTime(-69387, 4, 22, 11, 50, 58, DateTimeZone.forID(zoneId)); + assertFunction(format("from_unixtime(-2251799813685, '%s')", zoneId), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); + + // Test the values just outside the range of valid values. + assertNumericOverflow(format("from_unixtime(2251799813686, '%s')", zoneId), "TimestampWithTimeZone overflow: 2251799813686000 ms"); + assertNumericOverflow(format("from_unixtime(-2251799813686, '%s')", zoneId), "TimestampWithTimeZone overflow: -2251799813686000 ms"); + } + @Test public void testToUnixTime() { @@ -271,6 +289,24 @@ public void testFromISO8601() assertFunction("from_iso8601_date('" + DATE_ISO8601_STRING + "')", DateType.DATE, toDate(DATE)); } + @Test + public void testFromISO8601Overflow() + { + String zoneId = "Z"; + + // Test the largest possible valid value. + DateTime expected = new DateTime(73326, 9, 11, 20, 14, 45, 247, DateTimeZone.forID(zoneId)); + assertFunction(format("from_iso8601_timestamp('73326-09-11T20:14:45.247%s')", zoneId), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); + + // Test the smallest possible valid value. + expected = new DateTime(-69387, 12, 31, 23, 59, 59, 999, DateTimeZone.forID(zoneId)); + assertFunction(format("from_iso8601_timestamp('-69387-12-31T23:59:59.999%s')", zoneId), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); + + // Test the values just outside the range of valid values. + assertNumericOverflow(format("from_iso8601_timestamp('73326-09-11T20:14:45.248%s')", zoneId), "TimestampWithTimeZone overflow: 2251799813685248 ms"); + assertNumericOverflow(format("from_iso8601_timestamp('-69388-01-01T00:00:00.000%s')", zoneId), "TimestampWithTimeZone overflow: -2251841040000000 ms"); + } + @Test public void testToIso8601() { @@ -764,6 +800,25 @@ public void testParseDatetime() toTimestampWithTimeZone(new DateTime(1960, 1, 22, 3, 4, 0, 0, DateTimeZone.forOffsetHours(5)))); } + @Test + public void testParseDatetimeOverflow() + { + String zoneId = "Z"; + String pattern = "yyyy/MM/dd HH:mm:ss.SSS Z"; + + // Test the largest possible valid value. + DateTime expected = new DateTime(73326, 9, 11, 20, 14, 45, 247, DateTimeZone.forID(zoneId)); + assertFunction(format("parse_datetime('73326/09/11 20:14:45.247 %s', '%s')", zoneId, pattern), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); + + // Test the smallest possible valid value. + expected = new DateTime(-69387, 12, 31, 23, 59, 59, 999, DateTimeZone.forID(zoneId)); + assertFunction(format("parse_datetime('-69387/12/31 23:59:59.999 %s', '%s')", zoneId, pattern), TIMESTAMP_WITH_TIME_ZONE, toTimestampWithTimeZone(expected)); + + // Test the values just outside the range of valid values. + assertNumericOverflow(format("parse_datetime('73326/09/11 20:14:45.248 %s', '%s')", zoneId, pattern), "TimestampWithTimeZone overflow: 2251799813685248 ms"); + assertNumericOverflow(format("parse_datetime('-69388/01/01 00:00:00.000 %s', '%s')", zoneId, pattern), "TimestampWithTimeZone overflow: -2251841040000000 ms"); + } + @Test public void testFormatDatetime() {