This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new fad2a3f649 Correctly Proxy Statement returned from ResultSet
fad2a3f649 is described below
commit fad2a3f649d97ea3c2e355c455cc5e9f2e44950a
Author: Michael Clarke <[email protected]>
AuthorDate: Thu Jul 18 17:05:57 2024 +0100
Correctly Proxy Statement returned from ResultSet
The tomcat-jdbc connection pool creates reflective proxies around
Connections, Statements, and ResultSets to allow interception of 'close'
calls, and consistency of traversing the link back from a child element
to its parent - e.g. Statement to Connection. However, the link from
ResultSet to Statement does not intercept the call so a non-proxied
Statement is returned, as well as the `equals` method not handling the
comparison against a non-proxied class correctly. To overcome this, an
invocation handler is being created for wrapping a ResultSet returned
from any of the relevant Statement methods, with that invocation handler
intercepting the `getStatement` call and returning the proxied creating
statement. The `equals` checks have also been altered to handle
non-proxied instances being passed in to them.
---
.../apache/tomcat/jdbc/pool/StatementFacade.java | 96 ++++++++++
.../AbstractCreateStatementInterceptor.java | 21 ++-
.../tomcat/jdbc/test/ProxiedResultSetTest.java | 193 +++++++++++++++++++++
.../tomcat/jdbc/test/ProxiedStatementTest.java | 61 +++++++
.../apache/tomcat/jdbc/test/driver/ResultSet.java | 11 +-
.../apache/tomcat/jdbc/test/driver/Statement.java | 8 +-
6 files changed, 382 insertions(+), 8 deletions(-)
diff --git
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java
index 4771b321b4..e30f7d91ec 100644
---
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java
+++
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/StatementFacade.java
@@ -23,6 +23,7 @@ import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.sql.CallableStatement;
import java.sql.PreparedStatement;
+import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
@@ -93,6 +94,9 @@ public class StatementFacade extends
AbstractCreateStatementInterceptor {
return toString();
}
if (compare(EQUALS_VAL, method)) {
+ if (args[0] == null ||
!Proxy.isProxyClass(args[0].getClass())) {
+ return Boolean.FALSE;
+ }
return Boolean.valueOf(
this.equals(Proxy.getInvocationHandler(args[0])));
}
@@ -112,6 +116,17 @@ public class StatementFacade extends
AbstractCreateStatementInterceptor {
if (delegate == null) {
throw new SQLException("Statement closed.");
}
+
+ if (compare(GET_RESULTSET, method)) {
+ return getConstructor(RESULTSET_IDX,
ResultSet.class).newInstance(new ResultSetProxy(method.invoke(delegate,args),
proxy));
+ }
+ if (compare(GET_GENERATED_KEYS, method)) {
+ return getConstructor(RESULTSET_IDX,
ResultSet.class).newInstance(new ResultSetProxy(method.invoke(delegate,args),
proxy));
+ }
+ if (compare(EXECUTE_QUERY, method)) {
+ return getConstructor(RESULTSET_IDX,
ResultSet.class).newInstance(new ResultSetProxy(method.invoke(delegate,args),
proxy));
+ }
+
Object result = null;
try {
//invoke next
@@ -154,4 +169,85 @@ public class StatementFacade extends
AbstractCreateStatementInterceptor {
}
}
+ protected class ResultSetProxy implements InvocationHandler {
+
+ private final Object parent;
+ private Object delegate;
+
+ public ResultSetProxy(Object delegate, Object parent) {
+ this.delegate = delegate;
+ this.parent = parent;
+ }
+
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
+ if (compare(TOSTRING_VAL,method)) {
+ return toString();
+ }
+ if (compare(EQUALS_VAL, method)) {
+ if (args[0] == null ||
!Proxy.isProxyClass(args[0].getClass())) {
+ return Boolean.FALSE;
+ }
+ return Boolean.valueOf(
+ this.equals(Proxy.getInvocationHandler(args[0])));
+ }
+ if (compare(HASHCODE_VAL, method)) {
+ return Integer.valueOf(this.hashCode());
+ }
+ if (compare(CLOSE_VAL, method)) {
+ if (delegate == null) {
+ return null;
+ }
+ }
+ if (compare(ISCLOSED_VAL, method)) {
+ if (delegate == null) {
+ return Boolean.TRUE;
+ }
+ }
+ if (delegate == null) {
+ throw new SQLException("ResultSet closed.");
+ }
+
+ if (compare(GET_STATEMENT, method)) {
+ return parent;
+ }
+
+ Object result;
+ try {
+ //invoke next
+ result = method.invoke(delegate,args);
+ } catch (Throwable t) {
+ if (t instanceof InvocationTargetException && t.getCause() !=
null) {
+ throw t.getCause();
+ } else {
+ throw t;
+ }
+ }
+ //perform close cleanup
+ if (compare(CLOSE_VAL, method)) {
+ delegate = null;
+ }
+ return result;
+ }
+
+ @Override
+ public int hashCode() {
+ return System.identityHashCode(this);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ return this==obj;
+ }
+
+ @Override
+ public String toString() {
+ return ResultSetProxy.class.getName() + "[Proxy=" +
+ hashCode() +
+ "; Delegate=" +
+ delegate +
+ ']';
+ }
+ }
+
}
diff --git
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java
index 9e64a931da..8ce289b577 100644
---
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java
+++
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java
@@ -20,6 +20,8 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.sql.ResultSet;
+import java.sql.Statement;
import org.apache.tomcat.jdbc.pool.ConnectionPool;
import org.apache.tomcat.jdbc.pool.JdbcInterceptor;
@@ -38,6 +40,23 @@ public abstract class AbstractCreateStatementInterceptor
extends JdbcIntercepto
protected static final String PREPARE_CALL = "prepareCall";
protected static final int PREPARE_CALL_IDX = 2;
+ /**
+ * {@link Statement#getResultSet()}
+ */
+ protected static final String GET_RESULTSET = "getResultSet";
+
+ /**
+ * {@link Statement#getGeneratedKeys()}
+ */
+ protected static final String GET_GENERATED_KEYS = "getGeneratedKeys";
+
+ /**
+ * {@link ResultSet#getStatement()}
+ */
+ protected static final String GET_STATEMENT = "getStatement";
+
+ protected static final int RESULTSET_IDX = 3;
+
protected static final String[] STATEMENT_TYPES = {CREATE_STATEMENT,
PREPARE_STATEMENT, PREPARE_CALL};
protected static final int STATEMENT_TYPE_COUNT =
STATEMENT_TYPES.length;
@@ -51,7 +70,7 @@ public abstract class AbstractCreateStatementInterceptor
extends JdbcIntercepto
/**
* the constructors that are used to create statement proxies
*/
- protected static final Constructor<?>[] constructors = new
Constructor[STATEMENT_TYPE_COUNT];
+ protected static final Constructor<?>[] constructors = new
Constructor[STATEMENT_TYPE_COUNT + 1];
public AbstractCreateStatementInterceptor() {
super();
diff --git
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ProxiedResultSetTest.java
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ProxiedResultSetTest.java
new file mode 100644
index 0000000000..ae4d951f2c
--- /dev/null
+++
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ProxiedResultSetTest.java
@@ -0,0 +1,193 @@
+/*
+ * 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.test;
+
+import org.apache.tomcat.jdbc.test.driver.Driver;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+
+public class ProxiedResultSetTest extends DefaultTestCase {
+
+ @Test
+ public void
shouldReturnWrappedOwningPreparedStatementFromExecuteQueryResultSet() throws
Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("");
+ ResultSet resultSet = statement.executeQuery()) {
+ assertEquals(statement, resultSet.getStatement());
+ }
+ }
+
+ @Test
+ public void
shouldReturnWrappedOwningStatementFromGetGeneratedKeyResultSet() throws
Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ Statement statement =
con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+ ResultSet resultSet = statement.getGeneratedKeys()) {
+ assertEquals(statement, resultSet.getStatement());
+ }
+ }
+
+ @Test
+ public void shouldReturnWrappedOwningStatementFromExecuteQuery() throws
Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ Statement statement = con.createStatement();
+ ResultSet resultSet = statement.executeQuery("")) {
+ assertEquals(statement, resultSet.getStatement());
+ }
+ }
+
+ @Test
+ public void shouldReturnWrappedOwningStatementForGetResultSet() throws
Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ Statement statement = con.createStatement();
+ ResultSet resultSet = statement.getResultSet()) {
+ assertEquals(statement, resultSet.getStatement());
+ }
+ }
+
+ @Test
+ public void shouldReturnCorrectClosedStatus() throws SQLException {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = datasource.getConnection();
+ Statement statement = con.createStatement();
+ ResultSet resultSet = statement.getResultSet()) {
+ assertFalse(resultSet.isClosed());
+ resultSet.close();
+ assertTrue(resultSet.isClosed());
+ }
+ }
+
+ @Test
+ public void shouldReturnHashcodeRegardlessOfClosedStatus() throws
SQLException {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = datasource.getConnection();
+ Statement statement = con.createStatement();
+ ResultSet resultSet = statement.getResultSet()) {
+ int hashcode = resultSet.hashCode();
+ resultSet.close();
+ assertEquals(hashcode, resultSet.hashCode());
+ }
+ }
+
+ @Test
+ public void shouldReturnEqualsRegardlessOfClosedStatus() throws
SQLException {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = datasource.getConnection();
+ Statement statement = con.createStatement();
+ ResultSet resultSet = statement.getResultSet()) {
+ assertNotEquals(resultSet, "");
+ assertEquals(resultSet, resultSet);
+ resultSet.close();
+ assertNotEquals(resultSet, "");
+ assertEquals(resultSet, resultSet);
+ }
+ }
+
+
+ @Test
+ public void shouldReturnToStringRegardlessOfClosedStatus() throws
SQLException {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = datasource.getConnection();
+ Statement statement = con.createStatement();
+ ResultSet resultSet = statement.getResultSet()) {
+ String toStringResult = resultSet.toString();
+ resultSet.close();
+ // the delegate will change, so we can't compare the whole string
+ assertEquals(toStringResult.substring(0, 50),
resultSet.toString().substring(0, 50));
+ }
+ }
+
+
+ @Test
+ public void shouldReturnFalseForNullEqualsComparison() throws Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("sql");
+ ResultSet resultSet = statement.executeQuery()) {
+ assertNotEquals(resultSet, null);
+ }
+ }
+
+ @Test
+ public void shouldReturnFalseForDifferentObjectTypeEqualsComparison()
throws Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("sql");
+ ResultSet resultSet = statement.executeQuery()) {
+ assertNotEquals(resultSet, "");
+ }
+ }
+
+ @Test
+ public void shouldReturnFalseForNonProxiedResultSetEqualsComparison()
throws Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("sql");
+ ResultSet resultSet = statement.executeQuery()) {
+ assertNotEquals(resultSet, new
org.apache.tomcat.jdbc.test.driver.ResultSet(statement));
+ }
+ }
+
+ @Test
+ public void shouldReturnTrueForSameProxiedResultSetEqualsComparison()
throws Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("sql");
+ ResultSet resultSet = statement.executeQuery()) {
+ assertEquals(resultSet, resultSet);
+ }
+ }
+
+ @Test
+ public void shouldReturnFalseForNonEqualProxiedResultSetEqualsComparison()
throws Exception {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("sql");
+ ResultSet resultSet = statement.executeQuery();
+ Connection con2 = this.datasource.getConnection();
+ PreparedStatement statement2 = con2.prepareStatement("sql");
+ ResultSet resultSet2 = statement2.executeQuery()) {
+ assertNotEquals(resultSet, resultSet2);
+ }
+ }
+}
diff --git
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ProxiedStatementTest.java
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ProxiedStatementTest.java
new file mode 100644
index 0000000000..c7b16b3125
--- /dev/null
+++
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ProxiedStatementTest.java
@@ -0,0 +1,61 @@
+/*
+ * 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.test;
+
+import org.apache.tomcat.jdbc.test.driver.Driver;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+import static org.junit.Assert.assertNotEquals;
+
+public class ProxiedStatementTest extends DefaultTestCase {
+
+ @Test
+ public void shouldReturnFalseForNullEqualsComparison() throws SQLException
{
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ PreparedStatement statement = con.prepareStatement("sql")) {
+ assertNotEquals(statement, null);
+ }
+ }
+
+ @Test
+ public void shouldReturnFalseForDifferentObjectTypeEqualsComparison()
throws SQLException {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ Statement statement =
con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) {
+ assertNotEquals(statement, "");
+ }
+ }
+
+ @Test
+ public void shouldReturnFalseForNonProxiedResultSetEqualsComparison()
throws SQLException {
+ this.datasource.setDriverClassName(Driver.class.getName());
+ this.datasource.setUrl("jdbc:tomcat:test");
+ try (Connection con = this.datasource.getConnection();
+ Statement statement =
con.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) {
+ assertNotEquals(statement, new
org.apache.tomcat.jdbc.test.driver.Statement());
+ }
+ }
+}
diff --git
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/ResultSet.java
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/ResultSet.java
index 71d26b8d75..86ce153b7a 100644
---
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/ResultSet.java
+++
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/ResultSet.java
@@ -38,7 +38,13 @@ import java.util.Calendar;
import java.util.Map;
public class ResultSet implements java.sql.ResultSet {
- boolean hasNext = true;
+
+ private final Statement owner;
+ private boolean hasNext = true;
+
+ public ResultSet(Statement owner) {
+ this.owner = owner;
+ }
@Override
public boolean absolute(int row) throws SQLException {
@@ -437,7 +443,7 @@ public class ResultSet implements java.sql.ResultSet {
@Override
public Statement getStatement() throws SQLException {
// TODO Auto-generated method stub
- return null;
+ return owner;
}
@Override
@@ -1219,5 +1225,4 @@ public class ResultSet implements java.sql.ResultSet {
return null;
}
-
}
diff --git
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Statement.java
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Statement.java
index c37bb086b4..1d48adcc7e 100644
---
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Statement.java
+++
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Statement.java
@@ -729,7 +729,7 @@ public class Statement implements CallableStatement {
@Override
public ResultSet executeQuery() throws SQLException {
// TODO Auto-generated method stub
- return new org.apache.tomcat.jdbc.test.driver.ResultSet();
+ return new org.apache.tomcat.jdbc.test.driver.ResultSet(this);
}
@Override
@@ -1101,7 +1101,7 @@ public class Statement implements CallableStatement {
@Override
public ResultSet executeQuery(String sql) throws SQLException {
// TODO Auto-generated method stub
- return new org.apache.tomcat.jdbc.test.driver.ResultSet();
+ return new org.apache.tomcat.jdbc.test.driver.ResultSet(this);
}
@Override
@@ -1149,7 +1149,7 @@ public class Statement implements CallableStatement {
@Override
public ResultSet getGeneratedKeys() throws SQLException {
// TODO Auto-generated method stub
- return new org.apache.tomcat.jdbc.test.driver.ResultSet();
+ return new org.apache.tomcat.jdbc.test.driver.ResultSet(this);
}
@Override
@@ -1185,7 +1185,7 @@ public class Statement implements CallableStatement {
@Override
public ResultSet getResultSet() throws SQLException {
// TODO Auto-generated method stub
- return null;
+ return new org.apache.tomcat.jdbc.test.driver.ResultSet(this);
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
