This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d730d7d75c0864712a07b6541fe18c17d4386ff9 Author: opencode <[email protected]> AuthorDate: Tue Nov 3 09:44:28 2026 +0100 Mask the URL the driver-instantiation debug line prints The credential masking the pool applies to its configuration dump did not reach the debug line connectUsingDriver() writes when it instantiates the configured driver: that line printed the configured JDBC URL verbatim, and a JDBC URL legitimately carries inline secrets. Enabling DEBUG on the pool's package is a standard support step, so a configuration whose credential travels inside the URL wrote that credential to the log once per connection created - exactly the exposure the masking of the dump was meant to close. The line now prints the URL through the same masking the dump uses, which the configuration keeps package-visible for the purpose; the rest of the URL stays readable for diagnostics, and the driver is still handed the real URL. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 9 +- .../apache/tomcat/jdbc/pool/PooledConnection.java | 7 +- .../jdbc/pool/TestDriverInstantiationLog.java | 165 +++++++++++++++++++++ 3 files changed, 178 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java index 5765e79f06..02213982c8 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java @@ -989,7 +989,12 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl * Masks the credentials embedded in a raw configuration string so that a * password typed into the JDBC URL or the connection properties is not * printed in cleartext the way the dedicated password field is masked in - * {@link #toString()}. Two shapes are scrubbed. + * {@link #toString()}. Beyond that dump, the pool applies this masking + * wherever it logs diagnostics about these configuration strings - the + * driver-instantiation debug line of + * {@link PooledConnection#connectUsingDriver()} is one - so the log and + * the dump agree on what a masked configuration looks like. Two shapes + * are scrubbed. * <p> * The value of a pair whose key - at the start of a segment, ignoring * case - is one of {@code password}, {@code pass} and {@code pwd}. A @@ -1032,7 +1037,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl * @return the string with every recognised credential replaced by the mask, * or the string unchanged when it holds none */ - private static String maskCredentials(String value) { + static String maskCredentials(String value) { if (value == null) { return null; } diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java index 6ee3233b1f..748ab20f76 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java @@ -461,8 +461,13 @@ public class PooledConnection implements PooledConnectionMBean { } } else { if (log.isDebugEnabled()) { + // The dump of the configuration masks this URL (see + // PoolProperties#maskCredentials), and a DEBUG level + // is a standard support step, so a URL that carries + // its credential inline must reach this line masked + // as well - the driver is handed the real URL below. log.debug("Instantiating driver using class: " + poolProperties.getDriverClassName() + - " [url=" + poolProperties.getUrl() + "]"); + " [url=" + PoolProperties.maskCredentials(poolProperties.getUrl()) + "]"); } driver = (java.sql.Driver) ClassLoaderUtil.loadClass(poolProperties.getDriverClassName(), PooledConnection.class.getClassLoader(), Thread.currentThread().getContextClassLoader()) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestDriverInstantiationLog.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestDriverInstantiationLog.java new file mode 100644 index 0000000000..43643fbd0a --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestDriverInstantiationLog.java @@ -0,0 +1,165 @@ +/* + * 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.sql.Connection; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.juli.logging.LogFactory; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; + +/** + * The debug line {@code connectUsingDriver()} writes when it instantiates the + * configured driver prints the JDBC URL of the configuration, and enabling + * DEBUG on this package is a standard support step. A pool whose configuration + * carries its credential inside the URL - the {@code ;PASSWORD=...} pair an H2 + * or PostgreSQL style URL accepts - must therefore not have that credential + * written to the log: the same masking that keeps the configuration dump + * clean ({@code TestPoolProperties}) has to cover this line too, while the + * driver itself still receives the real URL. + */ +class TestDriverInstantiationLog extends JdbcPoolBaseTest { + + /** + * Captures what the pooled connection class logs. The handler goes in + * before anything runs, and a canary record - written through the very + * JULI logger the class logs through, not through JUL directly - proves + * the wiring observes those records before the line under test gets its + * chance to log. + */ + private static final class ConnectionLogCapture implements AutoCloseable { + private static final String CANARY = "pooledconnection-handler-wiring-canary"; + + private final Logger logger = Logger.getLogger(PooledConnection.class.getName()); + private final Level originalLevel = logger.getLevel(); + private final List<LogRecord> records = Collections.synchronizedList(new ArrayList<>()); + private final Handler handler = new Handler() { + @Override + public void publish(LogRecord record) { + records.add(record); + } + + @Override + public void flush() { + // no buffering + } + + @Override + public void close() { + // nothing to release + } + }; + + ConnectionLogCapture() { + logger.addHandler(handler); + logger.setLevel(Level.ALL); + LogFactory.getLog(PooledConnection.class).error(CANARY); + if (records.stream().noneMatch(r -> CANARY.equals(r.getMessage()))) { + close(); + throw new AssertionError("the test handler must observe what this class logs"); + } + records.clear(); + } + + /** + * The driver-instantiation line, which must have been logged exactly + * once - that is what makes an assertion about its content a real + * assertion rather than a pass-by on a line that never printed. + */ + String onlyDriverInstantiationLine() { + List<String> lines = new ArrayList<>(); + for (LogRecord record : records.toArray(new LogRecord[0])) { + String message = record.getMessage(); + if (message != null && message.startsWith("Instantiating driver using class:")) { + lines.add(message); + } + } + Assertions.assertEquals(1, lines.size(), + "the first connect must log exactly one driver instantiation line, saw: " + lines); + return lines.get(0); + } + + /** Nothing this class logged - message or thrown - may carry the secret. */ + void assertNothingLoggedCarries(String secret) { + for (LogRecord record : records.toArray(new LogRecord[0])) { + String message = String.valueOf(record.getMessage()); + String thrown = String.valueOf(record.getThrown()); + Assertions.assertFalse(message.contains(secret) || thrown.contains(secret), + "a credential must not reach the log, but this record carries it: " + + message + " " + thrown); + } + } + + @Override + public void close() { + logger.removeHandler(handler); + logger.setLevel(originalLevel); + } + } + + @Test + void aUrlCarryingItsCredentialReachesTheDebugLineMasked() throws Exception { + String secret = "S3cret-In-The-Url"; + // A fresh database on the test server, created by this very connect + // (-ifNotExists) under the credential the URL embeds, so the pool's + // borrow genuinely succeeds and the line under test is the only thing + // the failure paths could also have printed. + String credentialUrl = getUrl() + "_driver_log;USER=sa;PASSWORD=" + secret; + + try (ConnectionLogCapture log = new ConnectionLogCapture()) { + DataSource ds = newDataSource(); + ds.getPoolProperties().setUrl(credentialUrl); + ds.getPoolProperties().setPassword(secret); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + try (Connection c = ds.getConnection()) { + Assertions.assertTrue(c.isValid(1), "the pool must genuinely connect through this URL"); + } + + String line = log.onlyDriverInstantiationLine(); + Assertions.assertTrue(line.contains("[url=" + getUrl() + "_driver_log;USER=sa;PASSWORD=********]"), + "the URL must stay readable with only its credential masked, was: " + line); + log.assertNothingLoggedCarries(secret); + } + } + + @Test + void aUrlCarryingNoCredentialStillReachesTheDebugLineWhole() throws Exception { + try (ConnectionLogCapture log = new ConnectionLogCapture()) { + DataSource ds = newDataSource(); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + try (Connection c = ds.getConnection()) { + Assertions.assertTrue(c.isValid(1), "the pool must connect"); + } + + String line = log.onlyDriverInstantiationLine(); + Assertions.assertTrue(line.contains("[url=" + getUrl() + "]"), + "a URL with nothing to mask must print exactly as configured, was: " + line); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
