Skip to content

[common] Range-check the digit guard in DateTimeUtils - #9649

Open
LuciferYang wants to merge 2 commits into
apache:masterfrom
LuciferYang:fix/datetime-parse-overflow
Open

[common] Range-check the digit guard in DateTimeUtils#9649
LuciferYang wants to merge 2 commits into
apache:masterfrom
LuciferYang:fix/datetime-parse-overflow

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Purpose

close #9648

DateTimeUtils.parseDate and parseTime return null for input they cannot parse, and they guard every Integer.parseInt with isInteger. That guard only checked the characters:

private static boolean isInteger(String s) {
    boolean isInt = s.length() > 0;
    for (int i = 0; i < s.length(); i++) {
        if (s.charAt(i) < '0' || s.charAt(i) > '9') {
            isInt = false;
            break;
        }
    }
    return isInt;
}

A component of eleven digits therefore passed it and Integer.parseInt threw NumberFormatException, escaping a method whose every other failure path returns null. parseDate("2147483648-01-01") and parseTime("2147483648:00:00") both do it.

The guard now also checks the range, which is enough because it sits in front of each of the eleven parseInt calls in those two methods. Nothing else changes: no signature, no local variable type, no arithmetic.

Callers see the difference as an exception type. Through the casts, NumberFormatException becomes the DateTimeException that BinaryStringUtils.toDate raises for any unparseable string, which is what a caller already gets for "99999-01-01" (rejected by isIllegalDate) or "not-a-date". The Hive PaimonTimeObjectInspector.convert passes the Integer through, so a TIME column holding such a value writes NULL rather than failing, matching what it already does for other invalid times.

Tests

DateTimeUtilsTest.testParseDateAndTimeOverflowReturnsNull covers a too-large year, month, day and hour, and the two boundary cases: 2147483648 is the smallest ten-digit value that does not fit an int, and 2147483647 does fit but is still not a valid year, so both have to come back null through different branches. Valid values are asserted alongside them.

Against the unfixed guard the test errors with NumberFormatException: For input string: "2147483648".

mvn -pl paimon-common -Dtest=DateTimeUtilsTest test on JDK 8: 12 tests, 0 failures. spotless:check and checkstyle:check on paimon-common are clean.

parseDate and parseTime return null for unparseable input and guard every
Integer.parseInt with isInteger, which only checked that the characters
were digits. An eleven-digit component passed it and parseInt threw
NumberFormatException out of a method whose other failure paths return
null.

Check the range in the guard, in front of all eleven parseInt calls.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overflow guard has a real caller through Hive's TIME conversion, and the 12 DateTimeUtils tests pass with the changed class. There is a narrow compatibility regression in the new length shortcut that should be addressed without changing accepted numeric values.

*/
private static boolean isInteger(String s) {
boolean isInt = s.length() > 0;
if (s.isEmpty() || s.length() > 10) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Range-check the value without rejecting leading zeros. This length limit rejects components whose numeric value still fits an int: parseDate("00000002024-01-01") and parseTime("00000000012:30:00") return 19723 and 45000000 on the base revision, but both return null with this change (confirmed in an isolated base/head probe). Through PaimonTimeObjectInspector.convert, previously accepted zero-padded TIME values are therefore silently replaced with NULL. Ignore leading zeros for the range check, or use an overflow-checked accumulator, and add regression coverage alongside the genuinely out-of-range inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] A date or time component too large for an int throws instead of parsing to null

2 participants