This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f1a86d1564d7b2d7643b42e785863f8323db40ee Author: opencode <[email protected]> AuthorDate: Thu Oct 29 10:11:18 2026 +0100 Answer Wrapper questions from the layers below a pooled connection A driver built around another driver's connection - like the AWS wrapper over the PostgreSQL driver - hands the pool a connection that is a wrapper of the vendor's, not the vendor's itself. The pooled handle asked only the class of that presented object whether it implemented an interface, so the vendor object behind the driver's own wrapper was unreachable: isWrapperFor() answered false for it and unwrap() reported it as not wrapped. The handle now asks the driver's connection when the presented object does not implement the interface, and an unwrap through it is the driver's answer to give. The pooled connection's released state is guarded, and javax.sql.XAConnection is excluded from the delegation to keep what isWrapperFor() promises answerable by unwrap(). The pool's DataSource, whose Wrapper methods always answered false and returned null, now recognises itself and the configured dataSource - and one layer beneath that, through its own Wrapper answers - and a failed unwrap reports the SQLException the contract requires instead of the null that surfaces at the caller as an unrelated NullPointerException. Patch by: Shayan Davarzani Fixes https://bz.apache.org/bugzilla/show_bug.cgi?id=59569 Pull request: https://github.com/apache/tomcat/pull/1077 --- .../apache/tomcat/jdbc/pool/DataSourceProxy.java | 55 +++-- .../apache/tomcat/jdbc/pool/ProxyConnection.java | 34 ++- .../tomcat/jdbc/pool/TestWrapperDelegation.java | 250 +++++++++++++++++++++ webapps/docs/changelog.xml | 15 ++ 4 files changed, 338 insertions(+), 16 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java index be3180e93c..b6e700665a 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java @@ -85,35 +85,64 @@ public class DataSourceProxy implements PoolConfiguration { /** - * Check if this proxy wraps an instance of the given interface. - * This implementation always returns {@code false}. + * Check if this proxy implements, or wraps an instance of, the given + * interface. The configured {@link PoolConfiguration#getDataSource() + * DataSource}, if any, is checked as the wrapped object. * <p> * Has to match signature in DataSource. * - * @param iface The interface to check - ignored - * @return false - * @throws SQLException never thrown + * @param iface The interface to check + * @return {@code true} if this proxy implements the interface or wraps an + * object that does + * @throws SQLException if the wrapped data source throws one */ public boolean isWrapperFor(Class<?> iface) throws SQLException { - // we are not a wrapper of anything + if (iface.isInstance(this)) { + return true; + } + Object dataSource = poolProperties.getDataSource(); + if (iface.isInstance(dataSource)) { + return true; + } + if (dataSource instanceof java.sql.Wrapper) { + //like the pooled connections, the configured data source may + //itself be a wrapper - of the driver's data source, of a + //pooling data source the configuration replaced - and it is + //what answers for its own layers + return ((java.sql.Wrapper) dataSource).isWrapperFor(iface); + } return false; } /** - * Unwrap the proxy to the given interface. - * This implementation always returns {@code null}. + * Unwrap the proxy to the given interface. The configured + * {@link PoolConfiguration#getDataSource() DataSource}, if any, is + * checked as the wrapped object. * <p> * Has to match signature in DataSource * * @param <T> The interface type - * @param iface The interface to unwrap to - ignored - * @return null - * @throws SQLException never thrown + * @param iface The interface to unwrap to + * @return an object that implements the interface + * @throws SQLException if no object implementing the interface is found */ public <T> T unwrap(Class<T> iface) throws SQLException { - //we can't unwrap anything - return null; + if (iface.isInstance(this)) { + return iface.cast(this); + } + Object dataSource = poolProperties.getDataSource(); + if (iface.isInstance(dataSource)) { + return iface.cast(dataSource); + } + if (dataSource instanceof java.sql.Wrapper + && ((java.sql.Wrapper) dataSource).isWrapperFor(iface)) { + return ((java.sql.Wrapper) dataSource).unwrap(iface); + } + // The Wrapper contract asks for a failure, not for a silent null + // that turns into an unrelated NullPointerException at the caller's + // first use of the "unwrapped" value. + throw new SQLException("Not a wrapper of " + iface.getName()); } /** diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java index c303548653..4e469ba866 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java @@ -18,6 +18,7 @@ package org.apache.tomcat.jdbc.pool; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.sql.Connection; import java.sql.SQLException; import javax.sql.XAConnection; @@ -108,8 +109,31 @@ public class ProxyConnection extends JdbcInterceptor { public boolean isWrapperFor(Class<?> iface) { if (iface == XAConnection.class && connection.getXAConnection()!=null) { return true; - } else { - return iface.isInstance(connection.getConnection()); + } + Connection physical = connection.getConnection(); + if (physical == null) { + //released handle: there is nothing left below to be a wrapper + return false; + } + if (iface.isInstance(physical)) { + return true; + } + if (iface == XAConnection.class) { + //javax.sql.XAConnection is only ever handed out from the pooled + //connection's own XA handle - unwrap() says so - so the driver + //below is not asked about it + return false; + } + //The driver's own connection may itself wrap a further connection - + //a driver built around another driver's, like the AWS wrapper over + //the PostgreSQL one - and it, not the class of the object it + //presents, is what knows which interfaces it can unwrap to. + try { + return physical.isWrapperFor(iface); + } catch (SQLException x) { + //a driver that cannot answer the question is not a wrapper + //for the interface + return false; } } @@ -132,8 +156,12 @@ public class ProxyConnection extends JdbcInterceptor { throw new SQLException("Not a wrapper of " + iface.getName()); } return xa; - } else if (isWrapperFor(iface)) { + } else if (iface.isInstance(connection.getConnection())) { return connection.getConnection(); + } else if (isWrapperFor(iface)) { + //the driver's connection is a wrapper further down, and + //unwrapping that far is its answer to give, not ours + return connection.getConnection().unwrap(iface); } else { throw new SQLException("Not a wrapper of "+iface.getName()); } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperDelegation.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperDelegation.java new file mode 100644 index 0000000000..27f0fb78cb --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperDelegation.java @@ -0,0 +1,250 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.pool; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.Driver; +import java.sql.DriverPropertyInfo; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.sql.Statement; +import java.util.Properties; +import java.util.logging.Logger; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; + +/** + * The {@code Wrapper} contract of the objects the pool hands out, on the + * layers below the first one. + * <p> + * A pooled handle must unwrap to the vendor's own connection even when the + * driver's connection is not that connection but a wrapper around it - a + * driver built around another driver's, like the AWS Advanced JDBC Wrapper + * over the PostgreSQL driver. The wrapper, not the class of the object it + * presents, is what knows which interfaces it can unwrap to, so the handle + * has to delegate {@code isWrapperFor()} and {@code unwrap()} to it when + * the object it presents does not implement the interface itself. The + * pool's own {@code DataSource}, whose implementations of the two methods + * always answered false and returned null, must recognise itself and the + * configured {@code dataSource}, and must report a failed unwrap as the + * {@code SQLException} the contract requires.</p> + * + * <p>Scenario harvested from pull request 1077 (bz 59569); where its test + * ran against the retired stub driver, this one puts the wrapping layer + * over real H2 connections, so the unwrapped vendor object is asserted + * against the database as well.</p> + */ +class TestWrapperDelegation extends JdbcPoolBaseTest { + + @Test + void aPooledHandleUnwrapsThroughADriverThatWrapsAnotherDriversConnection() + throws Exception { + DataSource ds = newDataSource(); + ds.setDriverClassName(WrappingDriver.class.getName()); + + try (Connection con = ds.getConnection()) { + // the layer the driver presents directly: as it always was + Assertions.assertTrue(con.isWrapperFor(WrapperConnection.class)); + Assertions.assertInstanceOf(WrapperConnection.class, + con.unwrap(WrapperConnection.class)); + + // the vendor layer underneath it: only reachable by asking the + // presented wrapper about itself + Assertions.assertTrue(con.isWrapperFor(org.h2.jdbc.JdbcConnection.class), + "the handle must reach through the driver's own wrapper"); + Object vendor = con.unwrap(org.h2.jdbc.JdbcConnection.class); + Assertions.assertInstanceOf(org.h2.jdbc.JdbcConnection.class, vendor); + + // and the unwrapped vendor object is the real session, not a + // look-alike: it answers the database + try (Statement st = ((Connection) vendor).createStatement(); + ResultSet rs = st.executeQuery("SELECT 1")) { + Assertions.assertTrue(rs.next()); + Assertions.assertEquals(1, rs.getInt(1)); + } + + Assertions.assertFalse(con.isWrapperFor(Runnable.class), + "no layer of this connection is a Runnable"); + Assertions.assertThrows(SQLException.class, + () -> con.unwrap(Runnable.class)); + } + } + + @Test + void thePoolDataSourceRecognisesItselfAndTheConfiguredDataSource() + throws Exception { + DataSource ds = newDataSource(); + + // the handle's own interfaces, whatever else is or is not configured + Assertions.assertTrue(ds.isWrapperFor(javax.sql.DataSource.class)); + Assertions.assertSame(ds, ds.unwrap(org.apache.tomcat.jdbc.pool.DataSource.class)); + + // the configured data source, and the layers that one itself wraps + Object claimed = new Claimed(); + MarkerDataSource marker = (MarkerDataSource) Proxy.newProxyInstance( + TestWrapperDelegation.class.getClassLoader(), + new Class<?>[] { MarkerDataSource.class }, + (proxy, method, args) -> { + String name = method.getName(); + if ("isWrapperFor".equals(name)) { + Class<?> iface = (Class<?>) args[0]; + return Boolean.valueOf(iface == UnderlyingDataSource.class + || iface.isInstance(proxy)); + } + if ("unwrap".equals(name)) { + Class<?> iface = (Class<?>) args[0]; + if (iface == UnderlyingDataSource.class) { + return claimed; + } + if (iface.isInstance(proxy)) { + return proxy; + } + throw new SQLException("Not a wrapper of " + iface.getName()); + } + throw new UnsupportedOperationException(name); + }); + DataSource configured = manage(new DataSource()); + configured.setDataSource(marker); + + Assertions.assertTrue(configured.isWrapperFor(MarkerDataSource.class), + "the configured data source is what the proxy wraps"); + Assertions.assertSame(marker, configured.unwrap(MarkerDataSource.class)); + Assertions.assertTrue(configured.isWrapperFor(UnderlyingDataSource.class), + "and a layer beneath the configured data source is reached " + + "through its own Wrapper answers"); + Assertions.assertSame(claimed, configured.unwrap(UnderlyingDataSource.class)); + + Assertions.assertFalse(ds.isWrapperFor(Runnable.class)); + Assertions.assertThrows(SQLException.class, () -> ds.unwrap(Runnable.class), + "a failed unwrap is the contract's SQLException, not a null"); + } + + /** The marker interface of the driver's presenting layer. */ + public interface WrapperConnection extends Connection { + } + + /** A data source the pool can be configured with, of its own kind. */ + public interface MarkerDataSource extends javax.sql.DataSource { + } + + /** Another marker, claimed by {@link #MarkerDataSource} as a layer. */ + public interface UnderlyingDataSource { + } + + /** What the marker above claims to be underneath itself. */ + private static class Claimed implements UnderlyingDataSource { + } + + /** + * Driver that presents every H2 connection it returns as a wrapper of + * it, answering {@code isWrapperFor()} and {@code unwrap()} about the + * H2 connection underneath. Configured through {@code driverClassName}, + * so it must stay a public static class with a no-arg constructor. + */ + public static class WrappingDriver implements Driver { + + private final Driver delegate = new org.h2.Driver(); + + @Override + public Connection connect(String url, Properties info) throws SQLException { + Connection vendor = delegate.connect(url, info); + if (vendor == null) { + return null; + } + return (Connection) Proxy.newProxyInstance( + WrappingDriver.class.getClassLoader(), + new Class<?>[] { WrapperConnection.class }, + new WrappingHandler(vendor)); + } + + @Override + public boolean acceptsURL(String url) throws SQLException { + return delegate.acceptsURL(url); + } + + @Override + public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { + return delegate.getPropertyInfo(url, info); + } + + @Override + public int getMajorVersion() { + return delegate.getMajorVersion(); + } + + @Override + public int getMinorVersion() { + return delegate.getMinorVersion(); + } + + @Override + public boolean jdbcCompliant() { + return delegate.jdbcCompliant(); + } + + @Override + public Logger getParentLogger() throws SQLFeatureNotSupportedException { + return delegate.getParentLogger(); + } + } + + /* + * Presents the H2 connection as its own layer plus the vendor below: + * the Wrapper answers mirror the AWS wrapper's - the presented object + * is a Connection and a marker, and unwrapping reaches into the vendor. + */ + private static class WrappingHandler implements InvocationHandler { + + private final Connection vendor; + + WrappingHandler(Connection vendor) { + this.vendor = vendor; + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + String name = method.getName(); + if ("isWrapperFor".equals(name)) { + Class<?> iface = (Class<?>) args[0]; + return Boolean.valueOf(iface.isInstance(vendor) || iface.isInstance(proxy)); + } + if ("unwrap".equals(name)) { + Class<?> iface = (Class<?>) args[0]; + if (iface.isInstance(vendor)) { + return vendor; + } + if (iface.isInstance(proxy)) { + return proxy; + } + throw new SQLException("Not a wrapper of " + iface.getName()); + } + try { + return method.invoke(vendor, args); + } catch (InvocationTargetException x) { + throw x.getCause(); + } + } + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 13b9c3ad68..7f7ebf4a4e 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -1332,6 +1332,21 @@ dump; the text reported for a connection that really is abandoned is unchanged. Pull request <pr>1048</pr> provided by sainadh777. (remm) </fix> + <fix> + <bug>59569</bug>: Unwrap a pooled connection through a driver whose + own connection is itself a wrapper around another driver's + connection: when the physical connection does not directly implement + the requested interface, <code>isWrapperFor()</code> and + <code>unwrap()</code> now delegate to it, so the vendor object behind + a wrapping driver - like the AWS JDBC wrapper over the PostgreSQL + driver - is reachable through the pooled connection. The pool's + <code>DataSource</code>, whose implementations of the two methods + always answered false and returned null, now recognises itself and + the configured <code>dataSource</code>, and reports a failed unwrap + as the <code>SQLException</code> that the <code>Wrapper</code> + contract requires. Pull request <pr>1077</pr> provided by Shayan + Davarzani. (remm) + </fix> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
