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 ba192d2059c116f087137d07749bc4d8c3769876 Author: opencode <[email protected]> AuthorDate: Thu Oct 29 23:40:19 2026 +0100 Correct stale driver properties surviving connectionProperties reconfiguration setConnectionProperties() parsed the new value into the existing dbProperties, and Properties parsing only adds or overwrites, so a key dropped from the string - and every key parsed from it once the string was cleared - stayed in dbProperties and kept being sent to java.sql.Driver#connect() by each new physical connection for the life of the pool. The setter now removes the keys the previous value contributed before parsing the new one, so the pairs handed to the driver match the configuration again; entries the string never contributed, such as the user and password injected by the JNDI factory, are unaffected. A configuration deserialized from a stream written before the tracking field existed is tolerated. Adds tests for the reduced-set and clear paths. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 60 ++++++++++++++++++++-- .../tomcat/jdbc/pool/TestPoolProperties.java | 40 +++++++++++++++ 2 files changed, 97 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 798878b112..b42c98e6ab 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 @@ -20,10 +20,13 @@ import java.io.IOException; import java.io.Serializable; import java.io.StringReader; import java.lang.reflect.Method; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Locale; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -69,6 +72,8 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl private volatile String defaultCatalog = null; /** Additional connection properties. */ private volatile String connectionProperties; + /** Keys the last parse of {@link #connectionProperties} contributed to {@link #dbProperties}. */ + private volatile Set<String> connectionPropertyKeys = Collections.emptySet(); /** The initial number of connections in the pool. */ private volatile int initialSize = 10; /** The maximum number of active connections. */ @@ -472,16 +477,65 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl } + /** + * Stores the connection properties string and parses the pairs into the + * database properties the pool connects with. The pairs contributed by the + * previous value of the string are removed first, so reconfiguring a running + * pool with a reduced or cleared string really stops sending the dropped + * properties to the driver instead of leaving them merged in. Entries the + * string never contributed, such as the {@code user} and {@code password} + * injected by {@link DataSourceFactory#parsePoolProperties(Properties)} or + * mirrored in by {@link DataSourceProxy}, are kept. A configuration whose + * database properties are {@code null} only stores the raw string, as there + * is no property collection to parse into. The remove and re-add run inside + * this one synchronized method; an unsynchronised reader of the database + * properties, which {@link #getDbProperties()} has always handed out live, + * can therefore observe the contributed keys briefly missing. + * + * @param connectionProperties the property string with semicolon-separated + * key=value pairs, or {@code null} to clear it + */ @Override - public void setConnectionProperties(String connectionProperties) { + public synchronized void setConnectionProperties(String connectionProperties) { this.connectionProperties = connectionProperties; - getProperties(connectionProperties, getDbProperties()); + Properties dbProps = getDbProperties(); + if (dbProps == null) { + // The pairs have no collection to land in; nothing was contributed, + // so there is nothing to track for a later removal either. + connectionPropertyKeys = Collections.emptySet(); + return; + } + // A configuration serialized before this field existed deserializes + // with it null, as field initializers do not run on deserialization. + if (connectionPropertyKeys != null) { + for (String key : connectionPropertyKeys) { + dbProps.remove(key); + } + } + // Parse into a fresh collection first: that is what tells apart the keys + // this string contributes from the ones dbProperties holds from elsewhere. + Properties parsed = getProperties(connectionProperties, new Properties()); + Set<String> keys = new HashSet<>(parsed.size()); + for (String key : parsed.stringPropertyNames()) { + dbProps.setProperty(key, parsed.getProperty(key)); + keys.add(key); + } + connectionPropertyKeys = keys; } + /** + * Replaces the database properties the pool connects with. The keys tracked + * from the last connection properties parse contributed to the previous + * collection, not to the replacement, so the tracking is reset: the caller + * owns whatever the new collection holds. + * + * @param dbProperties the new database properties, may be {@code null} + */ @Override - public void setDbProperties(Properties dbProperties) { + public synchronized void setDbProperties(Properties dbProperties) { this.dbProperties = dbProperties; + connectionPropertyKeys = Collections.emptySet(); } 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 7f04021e43..bfd2202d6d 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 @@ -191,6 +191,46 @@ class TestPoolProperties { "a non-ASCII value must survive the factory path as well"); } + @Test + void reconfiguredConnectionPropertiesDropTheKeysTheStringDropped() { + PoolProperties properties = new PoolProperties(); + // the credential injection DataSourceFactory.parsePoolProperties does + // around the connectionProperties setter: these entries must survive + properties.getDbProperties().setProperty("user", DEFAULT_USER); + properties.getDbProperties().setProperty("password", DEFAULT_PASSWD); + + properties.setConnectionProperties("keep=yes;stale=leftover"); + properties.setConnectionProperties("keep=yes"); + + Assertions.assertEquals("yes", properties.getDbProperties().getProperty("keep"), + "a pair the new value still names must carry the new value"); + Assertions.assertNull(properties.getDbProperties().getProperty("stale"), + "a pair dropped by the new value must not keep being sent to" + + " the driver for the life of the pool"); + Assertions.assertEquals(DEFAULT_USER, properties.getDbProperties().getProperty("user"), + "a credential injected outside the string must survive reconfiguration"); + Assertions.assertEquals(DEFAULT_PASSWD, properties.getDbProperties().getProperty("password"), + "a credential injected outside the string must survive reconfiguration"); + } + + @Test + void clearedConnectionPropertiesRemoveEveryParsedPair() { + PoolProperties properties = new PoolProperties(); + properties.setConnectionProperties("stale=leftover"); + + properties.setConnectionProperties(null); + + Assertions.assertNull(properties.getConnectionProperties(), + "clearing the configuration must still round-trip as null"); + Assertions.assertTrue(properties.getDbProperties().isEmpty(), + "clearing the string must remove the pairs it contributed"); + + properties.setConnectionProperties("first=one;second=two"); + properties.setConnectionProperties(""); + Assertions.assertTrue(properties.getDbProperties().isEmpty(), + "an empty string must clear the pairs just like a null one does"); + } + @Test void toStringMasksThePasswordButStillShowsTheConfiguration() { PoolProperties properties = new PoolProperties(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
