Message-ID: From: "sehrope (@sehrope)" To: "pgjdbc/pgjdbc" Date: Wed, 17 Jun 2026 11:09:24 +0000 Subject: [pgjdbc/pgjdbc] PR #4194: Fix PGInterval.setSeconds to reject out of range and NaN values List-Id: X-GitHub-Additions: 54 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: 7 X-GitHub-Head-Branch: fix-pg-internal-set-seconds-silent-wrap X-GitHub-Head-SHA: dc007f7334d655604851ed6e47590451a51dd0b2 X-GitHub-Issue: 4194 X-GitHub-Merge-SHA: dc007f7334d655604851ed6e47590451a51dd0b2 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/4194 Content-Type: text/plain; charset=utf-8 `PGInterval.setSeconds` validated its argument against the long range in microseconds, but wholeSeconds is an int and the cast after division truncated unchecked. Values beyond the int range of whole seconds silently wrapped, e.g. `new PGInterval(0, 0, 0, 0, 0, 3e9)` produced a negative interval with no exception. Whole seconds is now computed as a long and checked against the int range before being stored, so out of range values throw IllegalArgumentException instead of wrapping. NaN is rejected explicitly (it previously passed the comparisons and Math.round mapped it to a zero interval). Infinite values are caught by the range check since `Math.round` saturates at the long bounds. Two slight behavioral changes are: 1. `isNull` is cleared only after validation succeeds, so a rejected value no longer mutates the object. 2. The `IllegalStateException` error message text is updated a bit I don't consider either to be a breaking change though. Second commits adds some tests for the for the boundary, out of range, NaN, and infinite cases (which fail without the fix). diff --git a/pgjdbc/src/main/java/org/postgresql/util/PGInterval.java b/pgjdbc/src/main/java/org/postgresql/util/PGInterval.java index 076c6be3f6..4e54ea26bc 100644 --- a/pgjdbc/src/main/java/org/postgresql/util/PGInterval.java +++ b/pgjdbc/src/main/java/org/postgresql/util/PGInterval.java @@ -427,14 +427,18 @@ public int getMicroSeconds() { * @param seconds seconds to set */ public void setSeconds(double seconds) { - isNull = false; - - double micros = seconds * MICROS_IN_SECOND; - if (micros > Long.MAX_VALUE || micros < Long.MIN_VALUE) { - throw new IllegalArgumentException("Number of seconds should be within Long.MIN_VALUE/1000000...Long.MAX_VALUE/1000000"); + if (Double.isNaN(seconds)) { + throw new IllegalArgumentException("Number of seconds must not be NaN"); } - long totalMicros = Math.round(micros); - wholeSeconds = (int) (totalMicros / MICROS_IN_SECOND); + // Math.round saturates at Long.MAX_VALUE / Long.MIN_VALUE so the division below + // cannot overflow even for infinite or extremely large arguments + long totalMicros = Math.round(seconds * MICROS_IN_SECOND); + long newWholeSeconds = totalMicros / MICROS_IN_SECOND; + if (newWholeSeconds < Integer.MIN_VALUE || newWholeSeconds > Integer.MAX_VALUE) { + throw new IllegalArgumentException("Number of whole seconds should be within Integer.MIN_VALUE...Integer.MAX_VALUE"); + } + isNull = false; + wholeSeconds = (int) newWholeSeconds; microSeconds = (int) (totalMicros % MICROS_IN_SECOND); } diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/IntervalTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/IntervalTest.java index 790c2f8242..d07b9f5720 100644 --- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/IntervalTest.java +++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/IntervalTest.java @@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.postgresql.test.TestUtil; @@ -173,6 +174,48 @@ void addRounding() { assertEquals(origTime, cal.getTime().getTime()); } + @Test + void setSecondsAtWholeSecondsBoundary() { + PGInterval pgi = new PGInterval(); + + pgi.setSeconds((double) Integer.MAX_VALUE); + assertEquals(Integer.MAX_VALUE, pgi.getWholeSeconds()); + assertEquals(0, pgi.getMicroSeconds()); + + pgi.setSeconds((double) Integer.MIN_VALUE); + assertEquals(Integer.MIN_VALUE, pgi.getWholeSeconds()); + assertEquals(0, pgi.getMicroSeconds()); + + pgi.setSeconds(Integer.MIN_VALUE - .456); + assertEquals(Integer.MIN_VALUE, pgi.getWholeSeconds()); + assertEquals(-456000, pgi.getMicroSeconds()); + + pgi.setSeconds(Integer.MAX_VALUE + .456); + assertEquals(Integer.MAX_VALUE, pgi.getWholeSeconds()); + assertEquals(456000, pgi.getMicroSeconds()); + } + + @Test + void setSecondsRejectsValuesBeyondWholeSecondsRange() { + PGInterval pgi = new PGInterval(); + + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(Integer.MAX_VALUE + 1.0)); + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(Integer.MIN_VALUE - 1.0)); + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(3_000_000_000.0)); + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(-3_000_000_000.0)); + assertThrows(IllegalArgumentException.class, + () -> new PGInterval(0, 0, 0, 0, 0, 3_000_000_000.0)); + } + + @Test + void setSecondsRejectsNonFiniteValues() { + PGInterval pgi = new PGInterval(); + + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(Double.NaN)); + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(Double.POSITIVE_INFINITY)); + assertThrows(IllegalArgumentException.class, () -> pgi.setSeconds(Double.NEGATIVE_INFINITY)); + } + @Test void offlineTests() throws Exception { PGInterval pgi = new PGInterval(2004, 4, 20, 15, 57, 12.1);