From 6546d03d84d5d165ae741e5ae6e4ec679eaf28b2 Mon Sep 17 00:00:00 2001 From: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:01:30 +0800 Subject: [PATCH 1/2] [DBCP-522] Invalidate auto-commit cache before driver calls A driver can change auto-commit before a setter or follow-up getter throws an Error. Clear the previous cached state before delegation so a later read does not return a stale value. Preserve the SQLException handler and the successful-path read of the actual delegate state. Add controlled-error regressions over real H2 memory connections and normal, SQLException and cache-disabled controls. Generated-by: DeepSeek Generated-by: Codex --- src/changes/changes.xml | 1 + .../commons/dbcp2/DelegatingConnection.java | 1 + .../dbcp2/TestDelegatingConnection.java | 107 ++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d14221bbf..d40db94c8 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -64,6 +64,7 @@ The type attribute can be add,update,fix,remove. + Invalidate the cached auto-commit state before delegating setAutoCommit to the underlying connection, so a failed state update does not leave a stale cached value. Handle SQLException on connection initialization with SQL statements (#540). diff --git a/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java b/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java index af1a6128c..b128957a3 100644 --- a/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java +++ b/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java @@ -839,6 +839,7 @@ public void rollback(final Savepoint savepoint) throws SQLException { @Override public void setAutoCommit(final boolean autoCommit) throws SQLException { checkOpen(); + cachedAutoCommit = null; try { connection.setAutoCommit(autoCommit); if (cacheState) { diff --git a/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java b/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java index f6e310819..25598a8e4 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java +++ b/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -343,6 +344,112 @@ void testRollbackSavepoint() throws Exception { } } + @Test + void testSetAutoCommitErrorAfterPhysicalChange() throws SQLException { + try (Connection physical = DriverManager.getConnection("jdbc:h2:mem:dcp522_autocommit_error")) { + physical.setAutoCommit(true); + final AssertionError failure = new AssertionError("setAutoCommit failed"); + final Connection faulty = new DelegatingConnection(physical) { + @Override + public void setAutoCommit(final boolean autoCommit) throws SQLException { + super.setAutoCommit(autoCommit); + throw failure; + } + }; + try (DelegatingConnection outer = new DelegatingConnection<>(faulty)) { + assertTrue(outer.getAutoCommit()); + final AssertionError thrown = assertThrows(AssertionError.class, () -> outer.setAutoCommit(false)); + assertSame(failure, thrown); + assertFalse(physical.getAutoCommit()); + assertFalse(outer.getAutoCommit()); + } + } + } + + @Test + void testSetAutoCommitErrorFromDelegateGetter() throws SQLException { + try (Connection physical = DriverManager.getConnection("jdbc:h2:mem:dcp522_autocommit_getter_error")) { + physical.setAutoCommit(true); + final AssertionError failure = new AssertionError("getAutoCommit failed"); + final Connection faulty = new DelegatingConnection(physical) { + private boolean failedOnce; + + @Override + public boolean getAutoCommit() throws SQLException { + if (!failedOnce && !physical.getAutoCommit()) { + failedOnce = true; + throw failure; + } + return physical.getAutoCommit(); + } + }; + try (DelegatingConnection outer = new DelegatingConnection<>(faulty)) { + assertTrue(outer.getAutoCommit()); + final AssertionError thrown = assertThrows(AssertionError.class, () -> outer.setAutoCommit(false)); + assertSame(failure, thrown); + assertFalse(physical.getAutoCommit()); + assertFalse(outer.getAutoCommit()); + } + } + } + + @Test + void testSetAutoCommitErrorWithCacheStateDisabled() throws SQLException { + try (Connection physical = DriverManager.getConnection("jdbc:h2:mem:dcp522_autocommit_no_cache")) { + physical.setAutoCommit(true); + final AssertionError failure = new AssertionError("setAutoCommit failed"); + final DelegatingConnection faulty = new DelegatingConnection(physical) { + @Override + public void setAutoCommit(final boolean autoCommit) throws SQLException { + super.setAutoCommit(autoCommit); + throw failure; + } + }; + faulty.setCacheState(false); + try (DelegatingConnection> outer = new DelegatingConnection<>(faulty)) { + outer.setCacheState(false); + assertTrue(outer.getAutoCommit()); + final AssertionError thrown = assertThrows(AssertionError.class, () -> outer.setAutoCommit(false)); + assertSame(failure, thrown); + assertFalse(physical.getAutoCommit()); + assertFalse(outer.getAutoCommit()); + } + } + } + + @Test + void testSetAutoCommitSqlExceptionAfterPhysicalChange() throws SQLException { + try (Connection physical = DriverManager.getConnection("jdbc:h2:mem:dcp522_autocommit_sql_exception")) { + physical.setAutoCommit(true); + final SQLException failure = new SQLException("setAutoCommit failed"); + final Connection faulty = new DelegatingConnection(physical) { + @Override + public void setAutoCommit(final boolean autoCommit) throws SQLException { + super.setAutoCommit(autoCommit); + throw failure; + } + }; + try (DelegatingConnection outer = new DelegatingConnection<>(faulty)) { + assertTrue(outer.getAutoCommit()); + final SQLException thrown = assertThrows(SQLException.class, () -> outer.setAutoCommit(false)); + assertSame(failure, thrown); + assertFalse(physical.getAutoCommit()); + assertFalse(outer.getAutoCommit()); + } + } + } + + @Test + void testSetAutoCommitSuccessUpdatesCache() throws SQLException { + try (Connection physical = DriverManager.getConnection("jdbc:h2:mem:dcp522_autocommit_success"); + DelegatingConnection outer = new DelegatingConnection<>(physical)) { + assertTrue(outer.getAutoCommit()); + outer.setAutoCommit(false); + assertFalse(physical.getAutoCommit()); + assertFalse(outer.getAutoCommit()); + } + } + @Test void testSetClientInfo() throws Exception { // TODO From 0fb5efe54a2a4607a8a4586f14f72836216b4463 Mon Sep 17 00:00:00 2001 From: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:29:09 +0800 Subject: [PATCH 2/2] Remove DBCP-522 changelog entry per review --- src/changes/changes.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d40db94c8..d14221bbf 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -64,7 +64,6 @@ The type attribute can be add,update,fix,remove. - Invalidate the cached auto-commit state before delegating setAutoCommit to the underlying connection, so a failed state update does not leave a stale cached value. Handle SQLException on connection initialization with SQL statements (#540).