This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 46b14fa120e0e374020268c88701ad46ff284b18 Author: opencode <[email protected]> AuthorDate: Mon Oct 26 16:22:15 2026 +0100 Keep equals on the pooled connection a contract-respecting false DisposableConnectionFacade.invoke() answered the equals method by extracting the invocation handler of the compared object with an unguarded Proxy.getInvocationHandler() call. That helper throws NullPointerException on a null argument and IllegalArgumentException on a non-proxy, so conn.equals(null) threw an NPE and conn.equals(someObject) threw an IllegalArgumentException - both forbidden by the java.lang.Object.equals contract, which requires false and forbids throwing. The violation was reachable on the default configuration through ordinary code such as List.contains, Objects.equals and assertion helpers, and even on a disposed facade because the equals branch runs ahead of the disposal guard. The extraction is now guarded, so a null or foreign argument, or a proxy that is not backed by this facade, simply compares unequal, matching the same guard StatementFacade already uses. --- .../jdbc/pool/DisposableConnectionFacade.java | 10 ++++++-- .../apache/tomcat/jdbc/pool/TestProxyContract.java | 28 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DisposableConnectionFacade.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DisposableConnectionFacade.java index a83ef4930a..669bebb9e9 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DisposableConnectionFacade.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DisposableConnectionFacade.java @@ -66,8 +66,14 @@ public class DisposableConnectionFacade extends JdbcInterceptor { public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { if (compare(EQUALS_VAL, method)) { - return Boolean.valueOf( - this.equals(Proxy.getInvocationHandler(args[0]))); + // Object.equals must answer false - never throw - for a null + // or foreign argument, so the handler extraction has to be + // guarded: Proxy.getInvocationHandler throws NPE on null and + // IllegalArgumentException on a non-proxy. + Object other = args[0]; + return Boolean.valueOf(other != null + && Proxy.isProxyClass(other.getClass()) + && this.equals(Proxy.getInvocationHandler(other))); } else if (compare(HASHCODE_VAL, method)) { return Integer.valueOf(this.hashCode()); } else if (getNext()==null) { diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestProxyContract.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestProxyContract.java index 7bbd043a26..8e5c0b3857 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestProxyContract.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestProxyContract.java @@ -108,6 +108,34 @@ class TestProxyContract extends JdbcPoolBaseTest { } } + @Test + void equalsAgainstNullOrForeignObjectsIsFalseNotAnException() + throws Exception { + DataSource ds = newDataSource(); + ds.createPool(); + + Connection pooled = ds.getConnection(); + Assertions.assertFalse(pooled.equals(null), + "Object.equals(null) must answer false, not throw"); + Assertions.assertFalse(pooled.equals("a foreign object"), + "equals against a non-proxy must answer false, not throw"); + // a proxy that is not the pool's own passes the proxy guard but + // must still compare unequal - its handler is not this facade + Object foreignProxy = java.lang.reflect.Proxy.newProxyInstance( + getClass().getClassLoader(), + new Class<?>[] { Runnable.class }, + (p, m, a) -> null); + Assertions.assertFalse(pooled.equals(foreignProxy), + "equals against a foreign dynamic proxy must answer false"); + pooled.close(); + // the equals branch runs ahead of the disposal guard, so the same + // contract must hold on a disposed facade too + Assertions.assertFalse(pooled.equals(null), + "a disposed facade must keep the equals(null) contract"); + Assertions.assertFalse(pooled.equals("a foreign object"), + "a disposed facade must keep the equals(non-proxy) contract"); + } + @Test void thePhysicalSessionStaysReachableThroughThePooledConnectionInterface() throws Exception { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
