This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 12d14a44a44f964aaa55313c1a0aaac2e2ff6749 Author: opencode <[email protected]> AuthorDate: Mon Oct 26 23:32:48 2026 +0100 Round-trip connection properties through the JNDI factory PoolConfiguration pairs setConnectionProperties() with a getter, and PoolProperties honours the pair by storing the string and parsing the pairs into the db-properties. The JNDI/factory path bypassed the setter and fed the parsed pairs to setDbProperties() directly, so getConnectionProperties() - and the ConnectionProperties MBean attribute and the configuration dump that delegate to it - reported null for pools configured through a <Resource>, even though the identical value set via JMX round-tripped. This completes the fix of the same contract defect on the DataSourceProxy path taken in 401360bcfb. parsePoolProperties now calls the canonical setter. The parsed pairs reach the db-properties exactly as before - the setter merges with the very same parser the factory helper used, into the fresh PoolProperties' empty db-properties, so the merge is equivalent to the old replacement (verified over adversarial inputs, including credential precedence and charset edge cases, with no db-properties difference in any case). One visible consequence, accepted as it was for the proxy half: a connectionProperties string that carries driver credentials is now rendered by PoolProperties.toString() diagnostics on this path, whereas the getter's null used to hide it; the documentation and the interface javadoc direct operators not to put credentials there, and Tomcat's JMX/config channels are trusted input. The now-unused protected static factory helper is retained as it is a published extension point; a removal or deprecation would be a separate API change. Regression test setConnectionPropertiesRoundTripsThroughTheFactory (fails pre-fix: getter null; pins raw-string round-trip, parsed pairs and the unconfigured case). --- .../apache/tomcat/jdbc/pool/DataSourceFactory.java | 7 ++++-- .../tomcat/jdbc/pool/TestPoolProperties.java | 26 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java index 7c422b3596..c949381bb4 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java @@ -622,8 +622,11 @@ public class DataSourceFactory implements ObjectFactory { value = properties.getProperty(PROP_CONNECTIONPROPERTIES); if (value != null) { - Properties p = getProperties(value); - poolProperties.setDbProperties(p); + // Use the canonical setter so that the configured string also + // round-trips through getConnectionProperties(); it parses the + // pairs into getDbProperties() with the very same parser that + // getProperties(value) below used to apply. + poolProperties.setConnectionProperties(value); } else { poolProperties.setDbProperties(new Properties()); } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java index 0aeb72cdda..22706bb387 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestPoolProperties.java @@ -18,6 +18,7 @@ package org.apache.tomcat.jdbc.pool; import java.util.ArrayList; import java.util.List; +import java.util.Properties; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -111,6 +112,31 @@ class TestPoolProperties { "clearing the configuration must round-trip as well"); } + @Test + void setConnectionPropertiesRoundTripsThroughTheFactory() { + Properties attrs = new Properties(); + attrs.setProperty("connectionProperties", "first=one;second=two"); + + PoolConfiguration configured = + DataSourceFactory.parsePoolProperties(attrs); + + Assertions.assertEquals("first=one;second=two", configured.getConnectionProperties(), + "the factory/JNDI path must store the raw string like every " + + "other implementation of the interface does"); + // the pairs must still reach the driver properties the pool connects with + Assertions.assertEquals("one", configured.getDbProperties().getProperty("first"), + "the parsed pairs must still reach the database properties"); + Assertions.assertEquals("two", configured.getDbProperties().getProperty("second"), + "the parsed pairs must still reach the database properties"); + + PoolConfiguration without = + DataSourceFactory.parsePoolProperties(new Properties()); + Assertions.assertNull(without.getConnectionProperties(), + "an unconfigured attribute must read back null"); + Assertions.assertTrue(without.getDbProperties().isEmpty(), + "an unconfigured attribute must leave the driver properties empty"); + } + @Test void toStringMasksThePasswordButStillShowsTheConfiguration() { PoolProperties properties = new PoolProperties(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
