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 bcf6724960736c8d1a116c1e41a0f66928a0bd20 Author: opencode <[email protected]> AuthorDate: Mon Nov 2 21:20:33 2026 +0100 Honor the blank-attribute contract for the JNDI factory's remaining string attributes The factory's own documentation promises that an attribute whose value is blank leaves its property at the PoolProperties default, and the string attributes around it implement exactly that: defaultCatalog, driverClassName and validationQuery all skip a blank and explain why. Four attributes were still storing the blank verbatim, each with a runtime consequence: - password: a cleared attribute stored an empty password entry in the driver properties, so the pool authenticated with an empty password instead of the default of carrying no password entry at all; - username: likewise, an empty user entry went into the driver properties; - url: a stored empty value failed every physical connection attempt against the driver, defeating a driver-property-only configuration; - validatorClassName: a stored empty value sent the connect path looking for a class named , warning of a ClassNotFoundException on every pool start before falling back to standard validation. All four now treat a blank value as not-configured, like every other attribute of the factory. Non-blank values keep their established shapes: credentials and the URL are stored verbatim, since they can legitimately carry surrounding whitespace, while validatorClassName is now applied trimmed, the way driverClassName already was, since a Java class name never carries meaningful whitespace. The parser javadoc's trimmed-value sentence names both class-name attributes now that the contract covers them. Two pinning tests in TestNamingFactory cover the blank-attributes- unset behavior and the verbatim/trimmed split; both fail against the pre-fix factory. --- .../apache/tomcat/jdbc/pool/DataSourceFactory.java | 40 +++++++++--- .../tomcat/jdbc/naming/TestNamingFactory.java | 76 ++++++++++++++++++++++ 2 files changed, 108 insertions(+), 8 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 e3b1d5d3bb..343cdbaee6 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 @@ -528,12 +528,13 @@ public class DataSourceFactory implements ObjectFactory { * template attribute yields - leaves the property at its * {@link PoolProperties} default. A value the property cannot take fails the * parse naming the attribute, rather than being silently dropped the way - * unparsed values used to be. Numeric, boolean and transaction-isolation + * unparsed values used to be. Numeric, boolean, transaction-isolation and + * class-name ({@code driverClassName} and {@code validatorClassName}) * attributes apply their trimmed value, and so does the lookup of the name * given by {@code dataSourceJNDI}: whitespace around a number, a keyword or a * class or JNDI name is always noise. Other string values are stored verbatim - * - a catalog, a validation query or a URL can legitimately carry - * surrounding whitespace. + * - a catalog, a validation query, a credential or a URL can legitimately + * carry surrounding whitespace. * </p> * * <p> @@ -723,17 +724,33 @@ public class DataSourceFactory implements ObjectFactory { } value = properties.getProperty(PROP_PASSWORD); - if (value != null) { + if (value != null && !value.trim().isEmpty()) { + // A stored empty value would go into the driver properties as an + // empty password entry and authenticate with it, while the + // documented meaning of a blank attribute is that the property + // stays at its default - no password entry at all. The value + // itself is kept untrimmed: a password may legitimately carry + // surrounding whitespace. poolProperties.setPassword(value); } value = properties.getProperty(PROP_URL); - if (value != null) { + if (value != null && !value.trim().isEmpty()) { + // A stored empty value would fail every physical connection + // attempt against the driver, while the documented meaning of a + // blank attribute is that the property stays at its default. The + // value itself is kept untrimmed, as the URL of a data source can + // carry whitespace the way a class name cannot. poolProperties.setUrl(value); } value = properties.getProperty(PROP_USERNAME); - if (value != null) { + if (value != null && !value.trim().isEmpty()) { + // A stored empty value would go into the driver properties as an + // empty user entry, while the documented meaning of a blank + // attribute is that the property stays at its default - no user + // entry at all. The value itself is kept untrimmed, as a user + // name is treated like the password here. poolProperties.setUsername(value); } @@ -758,8 +775,15 @@ public class DataSourceFactory implements ObjectFactory { } value = properties.getProperty(PROP_VALIDATOR_CLASS_NAME); - if (value != null) { - poolProperties.setValidatorClassName(value); + if (value != null && !value.trim().isEmpty()) { + // A stored empty value would send the connect path looking for a + // class named "" - a ClassNotFoundException warning on every pool + // start before it fell back to standard validation - while the + // documented meaning of a blank attribute is that the property + // stays at its default standard validation. The trimmed value is + // applied, as a Java class name can never carry meaningful + // surrounding whitespace. + poolProperties.setValidatorClassName(value.trim()); } value = properties.getProperty(PROP_VALIDATIONINTERVAL); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java index 47ee390bdb..137564ff2c 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java @@ -37,6 +37,7 @@ import org.apache.tomcat.jdbc.pool.DataSource; import org.apache.tomcat.jdbc.pool.DataSourceFactory; import org.apache.tomcat.jdbc.pool.PoolConfiguration; import org.apache.tomcat.jdbc.pool.PoolProperties; +import org.apache.tomcat.jdbc.pool.Validator; import org.apache.tomcat.jdbc.pool.XADataSource; /** @@ -202,6 +203,64 @@ public class TestNamingFactory extends JdbcPoolBaseTest { + "PoolProperties default in place"); } + @Test + void emptyCredentialUrlAndValidatorAttributesLeaveThePropertiesUnset() { + // Pre-fix these four attributes stored the blank verbatim even + // though the factory contract promises a blank leaves the property + // at its PoolProperties default: a cleared password attribute left + // the pool authenticating with an empty password entry, a cleared + // url made every physical connection attempt fail against the + // driver, and a cleared validatorClassName looked for a class named + // "" on every pool start and warned of the ClassNotFoundException + // before falling back to standard validation. + java.util.Properties attrs = new java.util.Properties(); + attrs.setProperty("username", " "); + attrs.setProperty("password", ""); + attrs.setProperty("url", " "); + attrs.setProperty("validatorClassName", ""); + + PoolConfiguration configured = + DataSourceFactory.parsePoolProperties(attrs); + + Assertions.assertNull(configured.getUsername(), + "a blank username attribute must leave the driver " + + "properties without a user entry"); + Assertions.assertNull(configured.getPassword(), + "a blank password attribute must leave the driver " + + "properties without a password entry"); + Assertions.assertNull(configured.getUrl(), + "a blank url attribute must leave the url unset"); + Assertions.assertNull(configured.getValidatorClassName(), + "a blank validatorClassName attribute must keep the " + + "standard validation default"); + } + + @Test + void credentialAttributesApplyVerbatimAndValidatorClassNameAppliesTrimmed() { + // A credential can legitimately carry surrounding whitespace, so it + // is stored verbatim; a Java class name never can, so the factory + // trims it the same way it trims driverClassName. + java.util.Properties attrs = new java.util.Properties(); + attrs.setProperty("username", " scott "); + attrs.setProperty("password", " secret "); + attrs.setProperty("validatorClassName", + " " + ParsingValidator.class.getName() + " "); + + PoolConfiguration configured = + DataSourceFactory.parsePoolProperties(attrs); + + Assertions.assertEquals(" scott ", configured.getUsername(), + "a non-blank username must be stored verbatim"); + Assertions.assertEquals(" secret ", configured.getPassword(), + "a non-blank password must be stored verbatim"); + Assertions.assertEquals(ParsingValidator.class.getName(), + configured.getValidatorClassName(), + "a non-blank validatorClassName must be stored trimmed"); + Assertions.assertInstanceOf(ParsingValidator.class, + configured.getValidator(), + "the trimmed class name must be the one loaded"); + } + @Test void emptyDefaultAutoCommitLeavesConnectionsAtTheDriverDefault() throws Exception { @@ -1540,6 +1599,23 @@ public class TestNamingFactory extends JdbcPoolBaseTest { ne.getRootCause()); } + /** + * Validator the trimmed {@code validatorClassName} attribute applied by + * the parsing tests above must resolve to. Instantiated reflectively by + * PoolProperties, hence public with a public constructor, the same + * shape as the naming targets below. + */ + public static class ParsingValidator implements Validator { + + public ParsingValidator() { + } + + @Override + public boolean validate(java.sql.Connection connection, int validateAction) { + return true; + } + } + /** * Target whose no-argument constructor throws. The constructor must * keep its explicit public modifier: the factory locates it with --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
