From cd9471997103872ef90793ad3aa3b1512879492e Mon Sep 17 00:00:00 2001 From: AryanRG Date: Sun, 18 Oct 2026 12:39:46 +0530 Subject: [PATCH] [DBCP-600] Honor positive sub-second validation timeouts Preserve positive sub-second validation bounds in both JDBC validation paths and cover them with regression tests. --- src/changes/changes.xml | 1 + .../commons/dbcp2/PoolableConnection.java | 17 +++++++++-------- .../commons/dbcp2/TestBasicDataSource.java | 11 ++++++++++- .../commons/dbcp2/TestPoolableConnection.java | 19 +++++++++++++++++++ 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d14221bbf..54428b0d5 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -64,6 +64,7 @@ The type attribute can be add,update,fix,remove. + Apply a positive sub-second validation timeout as one JDBC second instead of disabling the timeout. Handle SQLException on connection initialization with SQL statements (#540). diff --git a/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java b/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java index 0b4e1b6cb..25ecba009 100644 --- a/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java +++ b/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java @@ -404,21 +404,22 @@ public void setLastUsed() { * @param sql * The validation SQL query. * @param timeoutDuration - * The validation timeout in seconds. + * The validation timeout, applied in whole seconds: zero or negative means no timeout, and a positive + * sub-second value is rounded up to one second. * @throws SQLException * Thrown when validation fails or an SQLException occurs during validation * @since 2.10.0 */ - public void validate(final String sql, Duration timeoutDuration) throws SQLException { + public void validate(final String sql, final Duration timeoutDuration) throws SQLException { if (fastFailValidation && fatalSqlExceptionThrown.get()) { throw new SQLException(Utils.getMessage("poolableConnection.validate.fastFail")); } + // JDBC timeouts are whole seconds where 0 means no timeout, so a positive sub-second timeout becomes 1 second. + final int timeoutSeconds = timeoutDuration.isNegative() || timeoutDuration.isZero() ? 0 + : (int) Math.max(1, Math.min(Integer.MAX_VALUE, timeoutDuration.getSeconds())); if (sql == null || sql.isEmpty()) { - if (timeoutDuration.isNegative()) { - timeoutDuration = Duration.ZERO; - } - if (!isValid(timeoutDuration)) { + if (!isValid(Duration.ofSeconds(timeoutSeconds))) { throw new SQLException("isValid() returned false"); } return; @@ -431,8 +432,8 @@ public void validate(final String sql, Duration timeoutDuration) throws SQLExcep validationPreparedStatement = getInnermostDelegateInternal().prepareStatement(sql); } - if (timeoutDuration.compareTo(Duration.ZERO) > 0) { - validationPreparedStatement.setQueryTimeout((int) timeoutDuration.getSeconds()); + if (timeoutSeconds > 0) { + validationPreparedStatement.setQueryTimeout(timeoutSeconds); } try (ResultSet rs = validationPreparedStatement.executeQuery()) { diff --git a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java index e181e2de9..0e7c49da9 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java +++ b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java @@ -1131,12 +1131,21 @@ void testValidationQueryTimeoutNegative() throws Exception { void testValidationQueryTimeoutSucceed() throws Exception { ds.setTestOnBorrow(true); ds.setTestOnReturn(true); - ds.setValidationQueryTimeout(Duration.ofMillis(100)); // Works for TesterStatement + ds.setValidationQueryTimeout(Duration.ofSeconds(5)); // Works for TesterStatement try (final Connection con = ds.getConnection()) { // close right away. } } + @Test + void testValidationQueryTimeoutSubSecondFail() { + ds.setTestOnBorrow(true); + // A positive sub-second timeout must not be truncated to 0 (no timeout); it becomes 1 second, too fast for TesterStatement. + ds.setValidationQueryTimeout(Duration.ofMillis(100)); + final SQLException e = assertThrows(SQLException.class, ds::getConnection); + assertTrue(e.toString().contains("timeout")); + } + @Test void testValidationQueryTimeoutZero() throws Exception { ds.setTestOnBorrow(true); diff --git a/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java b/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java index 183d2cdcb..6add327a8 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java +++ b/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java @@ -21,9 +21,14 @@ 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 static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.sql.Connection; import java.sql.SQLException; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -213,4 +218,18 @@ void testPoolableConnectionLeak() throws Exception { assertEquals(0, pool.getNumActive(), "The pool should have no active connections"); } + + /** + * Without a validation query, a positive sub-second timeout must reach {@link Connection#isValid(int)} as 1 second, + * not 0 (no timeout). + */ + @Test + void testValidateWithoutQuerySubSecondTimeout() throws Exception { + final Connection nativeConnection = mock(Connection.class); + when(nativeConnection.isValid(1)).thenReturn(true); + final PoolableConnection conn = new PoolableConnection(nativeConnection, null, null); + conn.validate(null, Duration.ofMillis(100)); + verify(nativeConnection).isValid(1); + verify(nativeConnection, never()).isValid(0); + } }