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 f4004a0416ab6a69e3f76a2ff1c9ed97eaae8f73 Author: opencode <[email protected]> AuthorDate: Thu Oct 29 22:41:20 2026 +0100 DataSourceProxy: clear mirrored credential in dbProperties on null set setPassword(null)/setUsername(null) threw a NullPointerException after the null had already been stored in PoolProperties, because java.util.Properties rejects null values. The half-applied change left the previous password/user mirrored in dbProperties, and since the connect path clones dbProperties and only overrides from PoolProperties when the stored value is non-null, freshly established physical connections kept authenticating with the credential the caller had tried to clear. The setters now remove the mirrored entry when the credential is null, mirror it otherwise, and tolerate a null dbProperties. Adds TestDataSourceProxyCredentials pinning the behavior. --- .../apache/tomcat/jdbc/pool/DataSourceProxy.java | 36 +++++++- .../jdbc/pool/TestDataSourceProxyCredentials.java | 100 +++++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java index 727eedfa2c..862146dd12 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java @@ -471,10 +471,26 @@ public class DataSourceProxy implements PoolConfiguration { } + /** + * Sets the password used to establish connections with, and mirrors it into the {@code password} database + * property. A {@code null} password clears the credential: the entry is removed from the database properties + * instead of the properties being set to null, which {@link java.util.Properties} rejects and which previously + * threw after the field had already been cleared, leaving the previous password behind for the driver to keep + * using. A configuration whose database properties are {@code null} only stores the field, as there is no + * property collection to mirror into. {@inheritDoc} + */ @Override public void setPassword(String password) { this.poolProperties.setPassword(password); - this.poolProperties.getDbProperties().setProperty("password", this.poolProperties.getPassword()); + Properties dbProperties = this.poolProperties.getDbProperties(); + if (dbProperties != null) { + String storedPassword = this.poolProperties.getPassword(); + if (storedPassword == null) { + dbProperties.remove(PoolUtilities.PROP_PASSWORD); + } else { + dbProperties.setProperty(PoolUtilities.PROP_PASSWORD, storedPassword); + } + } } @@ -526,10 +542,26 @@ public class DataSourceProxy implements PoolConfiguration { } + /** + * Sets the user name used to establish connections with, and mirrors it into the {@code user} database property. + * A {@code null} user name clears the credential: the entry is removed from the database properties instead of the + * properties being set to null, which {@link java.util.Properties} rejects and which previously threw after the + * field had already been cleared, leaving the previous user name behind for the driver to keep using. A + * configuration whose database properties are {@code null} only stores the field, as there is no property + * collection to mirror into. {@inheritDoc} + */ @Override public void setUsername(String username) { this.poolProperties.setUsername(username); - this.poolProperties.getDbProperties().setProperty("user", getPoolProperties().getUsername()); + Properties dbProperties = this.poolProperties.getDbProperties(); + if (dbProperties != null) { + String storedUsername = this.poolProperties.getUsername(); + if (storedUsername == null) { + dbProperties.remove(PoolUtilities.PROP_USER); + } else { + dbProperties.setProperty(PoolUtilities.PROP_USER, storedUsername); + } + } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestDataSourceProxyCredentials.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestDataSourceProxyCredentials.java new file mode 100644 index 0000000000..bfb9d91b7e --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestDataSourceProxyCredentials.java @@ -0,0 +1,100 @@ +/* + * 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; + +/** + * Configuration only, no database and no pool: the credential setters on + * {@link DataSourceProxy} mirror the credential into the database properties + * that {@link PooledConnection} clones when it hands them to the driver, and + * passing {@code null} must clear the credential from both stores instead of + * throwing halfway. The connection path only overrides the clone with the + * stored password when that is non-null, so a mirror left behind keeps + * authenticating freshly established connections with the cleared credential. + */ +class TestDataSourceProxyCredentials { + + @Test + void aNullPasswordClearsTheMirroredDatabaseProperty() { + DataSourceProxy proxy = new DataSourceProxy(); + proxy.setUsername("alice"); + proxy.setPassword("s3cret"); + Properties dbProperties = proxy.getPoolProperties().getDbProperties(); + Assertions.assertEquals("alice", dbProperties.getProperty(PoolUtilities.PROP_USER)); + Assertions.assertEquals("s3cret", dbProperties.getProperty(PoolUtilities.PROP_PASSWORD)); + + // Before the fix this threw after clearing the field, leaving the + // old password in the properties the driver receives. + Assertions.assertDoesNotThrow(() -> proxy.setPassword(null)); + + Assertions.assertNull(proxy.getPoolProperties().getPassword()); + Assertions.assertFalse(dbProperties.containsKey(PoolUtilities.PROP_PASSWORD), + "the cleared password must not stay in the properties the driver is given"); + Assertions.assertEquals("alice", dbProperties.getProperty(PoolUtilities.PROP_USER), + "clearing the password must leave the user name alone"); + } + + @Test + void aNullUserNameClearsTheMirroredDatabaseProperty() { + DataSourceProxy proxy = new DataSourceProxy(); + proxy.setUsername("alice"); + proxy.setPassword("s3cret"); + Properties dbProperties = proxy.getPoolProperties().getDbProperties(); + + Assertions.assertDoesNotThrow(() -> proxy.setUsername(null)); + + Assertions.assertNull(proxy.getPoolProperties().getUsername()); + Assertions.assertFalse(dbProperties.containsKey(PoolUtilities.PROP_USER), + "the cleared user name must not stay in the properties the driver is given"); + Assertions.assertEquals("s3cret", dbProperties.getProperty(PoolUtilities.PROP_PASSWORD), + "clearing the user name must leave the password alone"); + } + + @Test + void credentialsCanBeSetAgainAfterTheyWereCleared() { + DataSourceProxy proxy = new DataSourceProxy(); + proxy.setPassword("first"); + proxy.setUsername("alice"); + proxy.setPassword(null); + proxy.setUsername(null); + proxy.setPassword("second"); + proxy.setUsername("bob"); + Properties dbProperties = proxy.getPoolProperties().getDbProperties(); + + Assertions.assertEquals("second", dbProperties.getProperty(PoolUtilities.PROP_PASSWORD)); + Assertions.assertEquals("bob", dbProperties.getProperty(PoolUtilities.PROP_USER)); + } + + @Test + void settingCredentialsToleratesNullDatabaseProperties() { + // PoolProperties.setDbProperties accepts null, and a credential + // setter must not turn that into a NullPointerException that leaves + // the value half-applied; the field is the whole of the state. + DataSourceProxy proxy = new DataSourceProxy(); + proxy.getPoolProperties().setDbProperties(null); + + Assertions.assertDoesNotThrow(() -> proxy.setPassword("hunter2")); + Assertions.assertDoesNotThrow(() -> proxy.setUsername(null)); + + Assertions.assertEquals("hunter2", proxy.getPoolProperties().getPassword()); + Assertions.assertNull(proxy.getPoolProperties().getUsername()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
