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 62145cc939c4d8bf810729242533788af45ee1e8 Author: opencode <[email protected]> AuthorDate: Fri Oct 30 10:01:59 2026 +0100 Do not report a preparation that carries no SQL in the query report interceptors createStatement guarded the prepareStatement hook against a null SQL string but handed the null straight to the prepareCall hook, so on a driver that tolerates prepareCall(null) - the pool itself never passes one; it takes a permissive driver plus an application error - the shared per-pool statistics map gained a permanently empty-named entry that consumed one of the maxQueries slots and surfaced through JMX as an empty query. The prepareCall branch now mirrors the prepareStatement guard; the proxy constructor is acquired before the hook in both branches, so skipping the report cannot disturb proxy creation, and a test pins that neither preparation hook nor the statistics map sees SQL that does not exist while the statement proxies are still created for every overload. --- .../jdbc/pool/interceptor/AbstractQueryReport.java | 4 +- .../interceptor/TestQueryReportExtraction.java | 86 +++++++++++++++++++++- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java index 719df84a9f..5fcfd132ec 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java @@ -200,7 +200,9 @@ public abstract class AbstractQueryReport extends AbstractCreateStatementInterce // prepareCall sql = (String) args[0]; constructor = getConstructor(PREPARE_CALL_IDX, CallableStatement.class); - prepareCall(sql, time); + if (sql != null) { + prepareCall(sql, time); + } } else { // Do nothing, might be a future unsupported method // so we better bail out and let the system continue diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestQueryReportExtraction.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestQueryReportExtraction.java index 7eeaa8329a..6f2b53ecd0 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestQueryReportExtraction.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestQueryReportExtraction.java @@ -16,6 +16,15 @@ */ package org.apache.tomcat.jdbc.pool.interceptor; +import java.lang.reflect.Proxy; +import java.sql.CallableStatement; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.Statement; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -24,7 +33,9 @@ import org.junit.jupiter.api.Test; * isolation. The SQL a statement was prepared with takes precedence over * the argument of the invoked method, an argument that is not a string is * not a query - it must not abort the report - and a batch executed on a - * plain statement is reported under the name {@code batch}. + * plain statement is reported under the name {@code batch}. A preparation + * with no SQL at all is not a query either - it reaches no hook, yet the + * proxy is still created. */ class TestQueryReportExtraction { @@ -73,4 +84,77 @@ class TestQueryReportExtraction { Assertions.assertEquals("SELECT 1", REPORT.reportSlowQuery(null, new Object[] { "SELECT 1" }, "executeQuery", 0, 1)); } + + /** + * Report that records whether its preparation hooks are invoked and + * with what, and that books the statistics the real + * {@code SlowQueryReport} books. + */ + private static class RecordingReport extends SlowQueryReport { + private boolean statementHookCalled; + private boolean callHookCalled; + private String calledSQL; + + @Override + public void prepareStatement(String sql, long time) { + statementHookCalled = true; + super.prepareStatement(sql, time); + } + + @Override + public void prepareCall(String sql, long time) { + callHookCalled = true; + calledSQL = sql; + super.prepareCall(sql, time); + } + } + + private static Statement delegateStatement() { + return (Statement) Proxy.newProxyInstance(TestQueryReportExtraction.class.getClassLoader(), + new Class<?>[] { Statement.class }, (proxy, method, args) -> null); + } + + @Test + void aPreparationWithoutSQLReachesNoHookAndBooksNoStats() throws Exception { + // a driver that tolerates prepareCall(null) - the pool itself never + // passes a null SQL, only a permissive driver plus an application + // error gets this far - must report nothing for SQL that does not + // exist: neither through the hook nor through a statistics entry + // keyed by the empty query, which would occupy one of the + // maxQueries slots of the per-pool map for good + RecordingReport report = new RecordingReport(); + report.queries = new ConcurrentHashMap<>(); + + Object callable = report.createStatement(null, + Connection.class.getMethod("prepareCall", String.class), + new Object[] { null }, delegateStatement(), 0); + Assertions.assertFalse(report.callHookCalled || report.statementHookCalled, + "a null SQL must not reach the preparation hooks"); + Assertions.assertTrue(report.queries.isEmpty(), + "a null SQL must not book a statistics entry, keys were " + report.queries.keySet()); + // skipping the hook must not disturb the proxy creation either + Assertions.assertTrue(Proxy.isProxyClass(callable.getClass()) && callable instanceof CallableStatement, + "the callable statement proxy must still be created for a null SQL"); + + Object prepared = report.createStatement(null, + Connection.class.getMethod("prepareStatement", String.class), + new Object[] { null }, delegateStatement(), 0); + Assertions.assertFalse(report.statementHookCalled || report.callHookCalled, + "a null SQL must not reach the preparation hooks"); + Assertions.assertTrue(report.queries.isEmpty(), + "a null SQL must not book a statistics entry, keys were " + report.queries.keySet()); + Assertions.assertTrue(Proxy.isProxyClass(prepared.getClass()) && prepared instanceof PreparedStatement, + "the prepared statement proxy must still be created for a null SQL"); + + // a real SQL on any of the overloads is still reported as before + report.createStatement(null, + Connection.class.getMethod("prepareCall", String.class, int.class, int.class), + new Object[] { "CALL p()", Integer.valueOf(1), Integer.valueOf(2) }, delegateStatement(), 0); + Assertions.assertEquals("CALL p()", report.calledSQL, + "the SQL of the prepareCall overload must still reach the hook"); + Set<String> keys = new HashSet<>(); + keys.add("CALL p()"); + Assertions.assertEquals(keys, report.queries.keySet(), + "the SQL of the prepareCall overload must still be booked"); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
