Skip to content

Fix silent overflow in DateTimeEncoding.pack #24674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amitkdutta merged 1 commit into from
Mar 6, 2025
Merged

Fix silent overflow in DateTimeEncoding.pack #24674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/DateTimeEncoding.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}

Expand Down
28 changes: 28 additions & 0 deletions presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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!
Expand Down
55 changes: 55 additions & 0 deletions presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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()
{
Expand Down Expand Up @@ -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()
{
Expand Down