Message-ID: From: "sehrope (@sehrope)" To: "pgjdbc/pgjdbc" Date: Wed, 10 Jun 2026 15:19:10 +0000 Subject: [pgjdbc/pgjdbc] PR #4163: Fix NumberParser.getFastLong(...) handling of overlong values List-Id: X-GitHub-Additions: 33 X-GitHub-Author-Id: 1690926 X-GitHub-Author-Login: sehrope X-GitHub-Base: master X-GitHub-Changed-Files: 2 X-GitHub-Commits: 2 X-GitHub-Deletions: 14 X-GitHub-Head-Branch: fix-get-fast-long-overflow X-GitHub-Head-SHA: 32a8b88657f19eeb7cf641b433e72ff11b81d860 X-GitHub-Issue: 4163 X-GitHub-Merge-SHA: 32a8b88657f19eeb7cf641b433e72ff11b81d860 X-GitHub-Merged-By: sehrope X-GitHub-Repo: pgjdbc/pgjdbc X-GitHub-State: merged X-GitHub-Type: pull_request X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/4163 Content-Type: text/plain; charset=utf-8 Fixes handling of some overlong edge cases in `NumberParser.getFastLong(...)`. Second commits has some test cases for it. The test cases fail without the fix applied. The reworked code for the parser works by doing accumulating things as a negative since the absolute value is one bigger than for positive numbers. It's kind of odd to read at first, but makes sense when you realize that adding it all up as negatives and flipping the sign at the end is the same thing. Been analyzing the code base with some tooling and this the first of a couple PRs. They're all separate topics so posting them as such. The fix should probably be backpatched too. diff --git a/pgjdbc/src/main/java/org/postgresql/util/NumberParser.java b/pgjdbc/src/main/java/org/postgresql/util/NumberParser.java index a41e3fbac7..87a9f2fa7e 100644 --- a/pgjdbc/src/main/java/org/postgresql/util/NumberParser.java +++ b/pgjdbc/src/main/java/org/postgresql/util/NumberParser.java @@ -17,7 +17,7 @@ public Throwable fillInStackTrace() { } }; - private static final long MAX_LONG_DIV_TEN = Long.MAX_VALUE / 10; + private static final long MIN_LONG_DIV_TEN = Long.MIN_VALUE / 10; /** * Optimised byte[] to number parser. This code does not handle null values, so the caller must do @@ -37,6 +37,9 @@ public static long getFastLong(byte[] bytes, long minVal, long maxVal) throws Nu boolean neg = bytes[0] == '-'; + // Accumulate the value as negative since abs(MIN_VALUE) > abs(MAX_VALUE), so every valid + // input fits without overflow. Wrapped arithmetic on a positive accumulator would let + // overlong inputs pass the overflow guard and parse to silently wrong values. long val = 0; int start = neg ? 1 : 0; while (start < len) { @@ -60,24 +63,22 @@ public static long getFastLong(byte[] bytes, long minVal, long maxVal) throws Nu } } - if (val <= MAX_LONG_DIV_TEN) { - val *= 10; - val += b - '0'; - } else { + if (val < MIN_LONG_DIV_TEN) { throw FAST_NUMBER_FAILED; } + val *= 10; + int digit = b - '0'; + if (val < Long.MIN_VALUE + digit) { + throw FAST_NUMBER_FAILED; + } + val -= digit; } - if (val < 0) { - // It is possible to get overflow in two situations: - // 1. for MIN_VALUE, because abs(MIN_VALUE)=MAX_VALUE+1. In this situation thanks to - // complement arithmetic we got correct result and shouldn't do anything with it. - // 2. for incorrect string, representing a number greater than MAX_VALUE, for example - // "9223372036854775809", it this case we have to throw exception - if (!(neg && val == Long.MIN_VALUE)) { + if (!neg) { + if (val == Long.MIN_VALUE) { + // abs(MIN_VALUE) = MAX_VALUE + 1, so this magnitude is only valid when negative throw FAST_NUMBER_FAILED; } - } else if (neg) { val = -val; } diff --git a/pgjdbc/src/test/java/org/postgresql/util/NumberParserTest.java b/pgjdbc/src/test/java/org/postgresql/util/NumberParserTest.java index ebe2824582..e059ef24ab 100644 --- a/pgjdbc/src/test/java/org/postgresql/util/NumberParserTest.java +++ b/pgjdbc/src/test/java/org/postgresql/util/NumberParserTest.java @@ -40,7 +40,25 @@ void getFastLong_failOnIncorrectStrings() { assertGetLongFail("-234.12542."); assertGetLongFail("."); assertGetLongFail("-."); - assertGetLongFail(Long.toString(Long.MIN_VALUE).substring(1)); + } + + @Test + void getFastLong_failOnOverflowedValues() { + // values that wrap the long accumulator must fail instead of returning wrapped results + assertGetLongFail("9223372036854775808"); + assertGetLongFail("-9223372036854775809"); + assertGetLongFail("92233720368547758089"); + assertGetLongFail("-92233720368547758080"); + assertGetLongFail("92233720368547758081234"); + assertGetLongFail("-92233720368547758081234"); + assertGetLongFail("18446744073709551616"); + assertGetLongFail("99999999999999999999"); + } + + @Test + void getFastLong_parsesMinValueWithFraction() { + assertGetLongResult("-9223372036854775808.9", Long.MIN_VALUE); + assertGetLongResult("9223372036854775807.9", Long.MAX_VALUE); } private static void assertGetLongResult(String s, long expected) {