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 e9454d992c81e9fde6dfcc33a62f1fe71bee632a Author: opencode <[email protected]> AuthorDate: Fri Oct 30 15:45:58 2026 +0100 Copy defaults-resolved dbProperties into the driver's connect properties PoolUtilities.clone() copied the configured dbProperties with putAll(), which copies only the entries directly present in the source java.util.Properties. A Properties object may also carry a second-level defaults table, and the source resolves those entries normally through getProperty() and reports them in stringPropertyNames(), but putAll() never consults that table. Every setting held only there was therefore silently missing from the copy, and that copy is exactly what PooledConnection.connect() passes to DriverManager.getConnection() or driver.connect(), so the setting never reached the new connection. The copy also backs the masked dbProperties getters on DataSourceProxy and the pool's JMX MBean, which likewise under-reported the configuration. Since dbProperties is a plain bean property, accepted without normalization by PoolProperties.setDbProperties() and reachable through DataSourceProxy.setDbProperties() and the JMX attribute setter, a value with a defaults table is a legal configuration; the defect is only that the copy no longer matched it. clone() now follows the copy with a pass over the source's stringPropertyNames(), materializing each String value the source resolves, so a source that chains one or more defaults tables contributes all of its readable settings to the single table of the copy. The copy needs no defaults table of its own, which keeps the contract of cloneWithoutPassword() intact: remove(PROP_PASSWORD) sees the password wherever it was configured, so a password held only in defaults cannot stay resolvable through the masked getter. Entries directly present in the source, including non-String values that Properties tolerates, are still copied verbatim and take precedence over any defaults entry under the same key, leaving the configuration of every source without a defaults table unchanged. Each value is re-read from the source rather than taken from the stringPropertyNames() snapshot, so a source modified concurrently cannot make the copy store a null value. Add TestPoolUtilities covering the defaults-held value, the snapshot being independent of later source modification, precedence of a directly present value, verbatim copying of non-String values, and password masking for a password held directly and one held only in defaults. --- .../org/apache/tomcat/jdbc/pool/PoolUtilities.java | 19 ++- .../apache/tomcat/jdbc/pool/TestPoolUtilities.java | 130 +++++++++++++++++++++ 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java index 7c8c14be43..b9b8abf029 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolUtilities.java @@ -40,7 +40,13 @@ public class PoolUtilities { } /** - * Creates a shallow copy of the given properties. + * Creates a shallow copy of the given properties. Entries that the source + * resolves to a String through a defaults table are materialized into the + * copy as plain values, unless the source has a directly present entry + * under the same key, so the copy needs no defaults table of its own for + * them. Entries directly present in the source are copied as they are, + * including non-String values, and take precedence; the defaults table + * itself is not carried over. * * @param p the properties to clone * @@ -49,6 +55,17 @@ public class PoolUtilities { public static Properties clone(Properties p) { Properties c = new Properties(); c.putAll(p); + // putAll(...) copies only the entries directly present in the source, + // so materialize the String values the source resolves through + // defaults. Re-read each value from the source: stringPropertyNames() + // is only a snapshot and the source may be modified concurrently, so + // a snapshotted key may resolve to nothing by the time it is read + for (String key : p.stringPropertyNames()) { + String value = p.getProperty(key); + if (value != null && !c.containsKey(key)) { + c.setProperty(key, value); + } + } return c; } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java new file mode 100644 index 0000000000..60f88daddf --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolUtilities.java @@ -0,0 +1,130 @@ +/* + * 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.util.Properties; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link PoolUtilities}, the helpers that build the + * {@link Properties} object {@link PooledConnection} hands to the JDBC driver + * and the masked copies the JMX and bean property getters expose. A + * {@code dbProperties} with a defaults table is a legal value for a plain + * bean property, so the copy the driver receives must still resolve the + * values the source resolves, and the password mask must not leave a + * defaults-held password resolvable through the masked getter. + */ +class TestPoolUtilities { + + @Test + void theCopyResolvesValuesHeldOnlyInTheDefaultsTable() { + Properties defaults = new Properties(); + defaults.setProperty("sslmode", "verify-full"); + Properties source = new Properties(defaults); + source.setProperty(PoolUtilities.PROP_USER, "bob"); + + Properties clone = PoolUtilities.clone(source); + + // Before the fix putAll(...) copied only the directly present entry, + // so the driver never saw the defaults-held setting. + Assertions.assertEquals("verify-full", clone.getProperty("sslmode")); + Assertions.assertEquals("bob", clone.getProperty(PoolUtilities.PROP_USER)); + Assertions.assertEquals(source.stringPropertyNames(), clone.stringPropertyNames()); + } + + @Test + void theCopyIsASnapshotOfTheSourceAndItsDefaults() { + Properties defaults = new Properties(); + defaults.setProperty("sslmode", "verify-full"); + Properties source = new Properties(defaults); + source.setProperty(PoolUtilities.PROP_USER, "bob"); + + Properties clone = PoolUtilities.clone(source); + defaults.setProperty("sslmode", "disable"); + source.setProperty(PoolUtilities.PROP_USER, "mallory"); + source.remove("sslmode"); + + Assertions.assertEquals("verify-full", clone.getProperty("sslmode")); + Assertions.assertEquals("bob", clone.getProperty(PoolUtilities.PROP_USER)); + } + + @Test + void aDirectlyPresentValueTakesPrecedenceOverTheSameKeyInTheDefaultsTable() { + Properties defaults = new Properties(); + defaults.setProperty("sslmode", "from-defaults"); + Properties source = new Properties(defaults); + source.setProperty("sslmode", "direct"); + + Properties clone = PoolUtilities.clone(source); + + // The materialization must not overwrite what putAll(...) already + // copied: the directly present entry wins over the defaults entry. + Assertions.assertEquals("direct", clone.getProperty("sslmode")); + Assertions.assertEquals("direct", clone.get("sslmode")); + } + + @Test + void directlyPresentValuesAreCopiedAsTheyAre() { + // Properties tolerates non-String values through the Hashtable API; + // the copy keeps them verbatim rather than dropping the entry. + Properties source = new Properties(); + Object value = new Object(); + source.put("object", value); + source.setProperty(PoolUtilities.PROP_USER, "bob"); + + Properties clone = PoolUtilities.clone(source); + + Assertions.assertSame(value, clone.get("object")); + Assertions.assertEquals("bob", clone.getProperty(PoolUtilities.PROP_USER)); + } + + @Test + void cloneWithoutPasswordMasksAPasswordHeldOnlyInTheDefaultsTable() { + // A reference copy (new Properties(source)) would pass a remove(...) + // over the defaults table and leave this password resolvable through + // the masked getter. + Properties defaults = new Properties(); + defaults.setProperty(PoolUtilities.PROP_PASSWORD, "s3cret"); + defaults.setProperty("sslmode", "verify-full"); + Properties source = new Properties(defaults); + source.setProperty(PoolUtilities.PROP_USER, "bob"); + + Properties masked = PoolUtilities.cloneWithoutPassword(source); + + Assertions.assertNull(masked.getProperty(PoolUtilities.PROP_PASSWORD)); + Assertions.assertFalse(masked.containsKey(PoolUtilities.PROP_PASSWORD)); + Assertions.assertEquals("verify-full", masked.getProperty("sslmode")); + Assertions.assertEquals("bob", masked.getProperty(PoolUtilities.PROP_USER)); + } + + @Test + void cloneWithoutPasswordMasksADirectlyPresentPassword() { + Properties source = new Properties(); + source.setProperty(PoolUtilities.PROP_USER, "bob"); + source.setProperty(PoolUtilities.PROP_PASSWORD, "s3cret"); + + Properties masked = PoolUtilities.cloneWithoutPassword(source); + + Assertions.assertFalse(masked.containsKey(PoolUtilities.PROP_PASSWORD)); + Assertions.assertEquals("bob", masked.getProperty(PoolUtilities.PROP_USER)); + Assertions.assertEquals("s3cret", + source.getProperty(PoolUtilities.PROP_PASSWORD), + "masking the copy must leave the source alone"); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
