You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2021/08/14 12:32:04 UTC

[commons-dbcp] branch master updated: Fix StackOverflowError in PoolableConnection.isDisconnectionSqlException #123.

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git


The following commit(s) were added to refs/heads/master by this push:
     new 0ae96ce  Fix StackOverflowError in PoolableConnection.isDisconnectionSqlException #123.
0ae96ce is described below

commit 0ae96ced0ffbeada4d08c78373b33f25d40a3ea7
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Aug 14 08:32:01 2021 -0400

    Fix StackOverflowError in PoolableConnection.isDisconnectionSqlException
    #123.
    
    This is a cleaned up version of the GitHub PR 123 by newnewcoder.
---
 src/changes/changes.xml                            |  3 ++
 .../apache/commons/dbcp2/PoolableConnection.java   | 47 +++++++++++++++-------
 .../commons/dbcp2/TestPoolableConnection.java      | 19 +++++++++
 3 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 1210870..49b8e9d 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -65,6 +65,9 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="ggregory" type="fix" due-to="Gary Gregory">
         Reimplement time tracking in AbandonedTrace with an Instant instead of a long.
       </action>
+      <action dev="ggregory" type="fix" due-to="newnewcoder, Gary Gregory">
+        Fix StackOverflowError in PoolableConnection.isDisconnectionSqlException #123.
+      </action>
       <!-- ADDS -->
       <action dev="ggregory" type="add" due-to="Gary Gregory">
         Add and use AbandonedTrace#setLastUsed(Instant).
diff --git a/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java b/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java
index 6151429..8145db9 100644
--- a/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java
+++ b/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java
@@ -212,7 +212,7 @@ public class PoolableConnection extends DelegatingConnection<Connection> impleme
 
     @Override
     protected void handleException(final SQLException e) throws SQLException {
-        fatalSqlExceptionThrown |= isDisconnectionSqlException(e);
+        fatalSqlExceptionThrown |= isFatalException(e);
         super.handleException(e);
     }
 
@@ -241,6 +241,29 @@ public class PoolableConnection extends DelegatingConnection<Connection> impleme
     }
 
     /**
+     * Checks the SQLState of the input exception.
+     * <p>
+     * If {@link #disconnectionSqlCodes} has been set, sql states are compared to those in the configured list of fatal
+     * exception codes. If this property is not set, codes are compared against the default codes in
+     * {@link Utils#DISCONNECTION_SQL_CODES} and in this case anything starting with #{link
+     * Utils.DISCONNECTION_SQL_CODE_PREFIX} is considered a disconnection.
+     * </p>
+     *
+     * @param e SQLException to be examined
+     * @return true if the exception signals a disconnection
+     */
+    boolean isDisconnectionSqlException(final SQLException e) {
+        boolean fatalException = false;
+        final String sqlState = e.getSQLState();
+        if (sqlState != null) {
+            fatalException = disconnectionSqlCodes == null
+                ? sqlState.startsWith(Utils.DISCONNECTION_SQL_CODE_PREFIX) || Utils.DISCONNECTION_SQL_CODES.contains(sqlState)
+                : disconnectionSqlCodes.contains(sqlState);
+        }
+        return fatalException;
+    }
+
+    /**
      * Checks the SQLState of the input exception and any nested SQLExceptions it wraps.
      * <p>
      * If {@link #disconnectionSqlCodes} has been set, sql states are compared to those in the
@@ -253,19 +276,15 @@ public class PoolableConnection extends DelegatingConnection<Connection> impleme
      *            SQLException to be examined
      * @return true if the exception signals a disconnection
      */
-    private boolean isDisconnectionSqlException(final SQLException e) {
-        boolean fatalException = false;
-        final String sqlState = e.getSQLState();
-        if (sqlState != null) {
-            fatalException = disconnectionSqlCodes == null
-                    ? sqlState.startsWith(Utils.DISCONNECTION_SQL_CODE_PREFIX)
-                            || Utils.DISCONNECTION_SQL_CODES.contains(sqlState)
-                    : disconnectionSqlCodes.contains(sqlState);
-            if (!fatalException) {
-                final SQLException nextException = e.getNextException();
-                if (nextException != null && nextException != e) {
-                    fatalException = isDisconnectionSqlException(e.getNextException());
-                }
+    boolean isFatalException(final SQLException e) {
+        boolean fatalException = isDisconnectionSqlException(e);
+        if (!fatalException) {
+            SQLException parentException = e;
+            SQLException nextException = e.getNextException();
+            while(nextException != null && nextException != parentException && !fatalException) {
+                fatalException = isDisconnectionSqlException(nextException);
+                parentException = nextException;
+                nextException = parentException.getNextException();
             }
         }
         return fatalException;
diff --git a/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java b/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java
index ff7f00c..7c28446 100644
--- a/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java
+++ b/src/test/java/org/apache/commons/dbcp2/TestPoolableConnection.java
@@ -20,10 +20,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.fail;
 
+import java.lang.management.ManagementFactory;
+import java.lang.management.RuntimeMXBean;
+import java.lang.reflect.Method;
 import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.ArrayList;
 
+import java.util.List;
 import org.apache.commons.pool2.impl.GenericObjectPool;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
@@ -187,4 +191,19 @@ public class TestPoolableConnection {
 
         assertEquals(0, pool.getNumActive(), "The pool should have no active connections");
     }
+
+    @Test
+    public void testIsDisconnectionSqlExceptionStackOverflow() throws Exception {
+        final int maxDeep = 100_000;
+        final SQLException rootException = new SQLException("Data truncated", "22001");
+        SQLException parentException = rootException;
+        for (int i = 0; i <= maxDeep; i++) {
+            final SQLException childException = new SQLException("Data truncated: " + i, "22001");
+            parentException.setNextException(childException);
+            parentException = childException;
+        }
+        final Connection conn = pool.borrowObject();
+        assertEquals(false, ((PoolableConnection) conn).isDisconnectionSqlException(rootException));
+        assertEquals(false, ((PoolableConnection) conn).isFatalException(rootException));
+    }
 }