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 01c5ea613199a275ec761da48b1e4fcc7c48a5c3 Author: opencode <[email protected]> AuthorDate: Tue Nov 3 08:41:56 2026 +0100 Resolve in the copy what the source resolves beneath a non-String entry PoolUtilities.clone() materializes the String values the source resolves through its defaults table, so the tableless copy the driver receives and the masked JMX and bean views still resolve every setting the source resolves. The pass skipped a key whenever the copy already contained it directly, but that containsKey(...) guard was blind to the value's type. A java.util.Properties direct value that is not a String shadows nothing: getProperty(...) falls through to the defaults whenever the direct value is not a String, and stringPropertyNames() reports such a key because the source does resolve a String through it. The copy therefore kept the raw non-String object, skipped the key, and resolved nothing where the source resolved the defaults setting - a driver property the configuration carries silently reaching no connection, and the masked views under-reporting the configuration. The skip is now decided on the resolution the copy already has: only a directly copied String, which resolves in the copy exactly as it does in the source, keeps its precedence. Where the source resolves a String beneath a non-String direct value, the resolved String takes the raw value's place in the copy, since one key cannot carry both. Entries the source resolves to no String are still copied verbatim. Password masking is unaffected in every shape: the copy has no defaults table, so the mask's remove(...) still sees a resolved password wherever it lands. Found by a clean-room review of the class; pinned by two tests. --- .../org/apache/tomcat/jdbc/pool/PoolUtilities.java | 24 ++++++++++----- .../apache/tomcat/jdbc/pool/TestPoolUtilities.java | 36 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) 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 b9b8abf029..fbc8117377 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 @@ -42,11 +42,15 @@ public class PoolUtilities { /** * 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. + * copy as plain values, so the copy resolves every String the source + * resolves and needs no defaults table of its own. A String directly + * present in the source under the same key shadows the defaults there and + * takes precedence in the copy. A non-String directly present value + * shadows nothing, because {@link Properties#getProperty(String)} falls + * through to the defaults whenever the direct value is not a String, so + * where the source resolves a String under such a key the resolved String + * takes its place in the copy; entries the source resolves to no String + * are copied as they are. The defaults table itself is not carried over. * * @param p the properties to clone * @@ -59,10 +63,16 @@ public class PoolUtilities { // 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 + // a snapshotted key may resolve to nothing by the time it is read. + // Skip only a directly copied String, which is what the source + // resolves too; a non-String directly copied value resolves to + // nothing on its own - getProperty(...) only falls through to the + // defaults when the direct value is not a String - so the value the + // source resolves replaces it, keeping the copy able to resolve every + // String the source resolves for (String key : p.stringPropertyNames()) { String value = p.getProperty(key); - if (value != null && !c.containsKey(key)) { + if (value != null && !(c.get(key) instanceof String)) { c.setProperty(key, value); } } 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 index 60f88daddf..af2f2d1a11 100644 --- 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 @@ -79,6 +79,25 @@ class TestPoolUtilities { Assertions.assertEquals("direct", clone.get("sslmode")); } + @Test + void theCopyResolvesTheDefaultsStringBeneathANonStringDirectlyPresentValue() { + // Properties.getProperty(...) falls through to the defaults whenever + // the directly present value is not a String, so the source really + // does resolve "verify-full" under this key. A containsKey(...) guard + // in the materialization pass was blind to that: it saw the copied + // non-String entry and skipped the key, leaving the defaults-backed + // setting unresolvable in the tableless copy the driver receives. + Properties defaults = new Properties(); + defaults.setProperty("sslmode", "verify-full"); + Properties source = new Properties(defaults); + source.put("sslmode", new Object()); + + Properties clone = PoolUtilities.clone(source); + + Assertions.assertEquals("verify-full", clone.getProperty("sslmode"), + "the copy must resolve what the source resolves"); + } + @Test void directlyPresentValuesAreCopiedAsTheyAre() { // Properties tolerates non-String values through the Hashtable API; @@ -113,6 +132,23 @@ class TestPoolUtilities { Assertions.assertEquals("bob", masked.getProperty(PoolUtilities.PROP_USER)); } + @Test + void cloneWithoutPasswordMasksAResolvedPasswordBeneathANonStringDirectValue() { + // The shape of the test above with the password also shadowed by a + // non-String direct value: the source resolves the defaults-held + // password through it, so the materialization must book it into the + // copy - and the mask must then remove it with the key. + Properties defaults = new Properties(); + defaults.setProperty(PoolUtilities.PROP_PASSWORD, "s3cret"); + Properties source = new Properties(defaults); + source.put(PoolUtilities.PROP_PASSWORD, new Object()); + + Properties masked = PoolUtilities.cloneWithoutPassword(source); + + Assertions.assertNull(masked.getProperty(PoolUtilities.PROP_PASSWORD)); + Assertions.assertFalse(masked.containsKey(PoolUtilities.PROP_PASSWORD)); + } + @Test void cloneWithoutPasswordMasksADirectlyPresentPassword() { Properties source = new Properties(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
