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 36d186f9f7fb99c6ed65390521d90f0e8da32b68
Author: opencode <[email protected]>
AuthorDate: Thu Nov 5 13:24:08 2026 +0100

    Treat a blank connectionProperties as unset
    
    DataSourceFactory.parsePoolProperties treated any present 
connectionProperties attribute as configured, so a blank or whitespace-only 
value was stored as an empty string. The getter and configuration dump then 
reported a value that was never configured, contrary to the documented rule 
that a blank attribute leaves the property at its default.
    
    Treat a blank value like the absent one: store null and leave the driver 
properties empty. Extend the factory round-trip test with a whitespace-only 
attribute.
---
 .../org/apache/tomcat/jdbc/pool/DataSourceFactory.java | 15 ++++++++++-----
 .../TestDataSourceFactoryNoOpPropertyWarnings.java     | 18 ++++++++++++++++++
 .../apache/tomcat/jdbc/pool/TestPoolProperties.java    |  9 +++++++++
 3 files changed, 37 insertions(+), 5 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 343cdbaee6..f961ecd256 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
@@ -699,7 +699,7 @@ public class DataSourceFactory implements ObjectFactory {
         }
 
         value = properties.getProperty(PROP_NUMTESTSPEREVICTIONRUN);
-        if (value != null) {
+        if (value != null && !value.trim().isEmpty()) {
             log.warn(MessageFactory.get("dataSourceFactory.invalidSetting", 
PROP_NUMTESTSPEREVICTIONRUN));
             Integer parsed = parseInteger(PROP_NUMTESTSPEREVICTIONRUN, value);
             if (parsed != null) {
@@ -795,7 +795,7 @@ public class DataSourceFactory implements ObjectFactory {
         }
 
         value = 
properties.getProperty(PROP_ACCESSTOUNDERLYINGCONNECTIONALLOWED);
-        if (value != null) {
+        if (value != null && !value.trim().isEmpty()) {
             log.warn(MessageFactory.get("dataSourceFactory.invalidSetting", 
PROP_ACCESSTOUNDERLYINGCONNECTIONALLOWED));
             Boolean b = parseBoolean(PROP_ACCESSTOUNDERLYINGCONNECTIONALLOWED, 
value);
             if (b != null) {
@@ -828,21 +828,26 @@ public class DataSourceFactory implements ObjectFactory {
         }
 
         value = properties.getProperty(PROP_POOLPREPAREDSTATEMENTS);
-        if (value != null) {
+        if (value != null && !value.trim().isEmpty()) {
             log.warn(MessageFactory.get("dataSourceFactory.invalidSetting", 
PROP_POOLPREPAREDSTATEMENTS));
         }
 
         value = properties.getProperty(PROP_MAXOPENPREPAREDSTATEMENTS);
-        if (value != null) {
+        if (value != null && !value.trim().isEmpty()) {
             log.warn(MessageFactory.get("dataSourceFactory.invalidSetting", 
PROP_MAXOPENPREPAREDSTATEMENTS));
         }
 
         value = properties.getProperty(PROP_CONNECTIONPROPERTIES);
-        if (value != null) {
+        if (value != null && !value.trim().isEmpty()) {
             // The canonical setter parses the pairs into the driver
             // properties and keeps the string round-tripping through
             // getConnectionProperties().
             poolProperties.setConnectionProperties(value);
+        } else if (value != null) {
+            // A blank attribute means the default, null: storing "" would
+            // make the configuration dump and the getter report a value that
+            // was never configured.
+            poolProperties.setConnectionProperties(null);
         } else {
             poolProperties.setDbProperties(new Properties());
         }
diff --git 
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestDataSourceFactoryNoOpPropertyWarnings.java
 
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestDataSourceFactoryNoOpPropertyWarnings.java
index e9800dfb37..4e166050d5 100644
--- 
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestDataSourceFactoryNoOpPropertyWarnings.java
+++ 
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestDataSourceFactoryNoOpPropertyWarnings.java
@@ -236,6 +236,24 @@ class TestDataSourceFactoryNoOpPropertyWarnings {
         }
     }
 
+    @Test
+    void blankNoOpAttributesAreNotReportedAsConfigured() {
+        Properties properties = new Properties();
+        properties.setProperty("accessToUnderlyingConnectionAllowed", "   ");
+        properties.setProperty("numTestsPerEvictionRun", "   ");
+        properties.setProperty("poolPreparedStatements", "");
+        properties.setProperty("maxOpenPreparedStatements", "\t");
+
+        try (FactoryLogCapture logs = new FactoryLogCapture()) {
+            PoolConfiguration parsed = 
DataSourceFactory.parsePoolProperties(properties);
+            logs.assertNothingReportedAsHavingNoEffect();
+            Assertions.assertEquals(0, parsed.getNumTestsPerEvictionRun(),
+                    "a blank attribute must leave the default in place");
+            
Assertions.assertTrue(parsed.isAccessToUnderlyingConnectionAllowed(),
+                    "a blank attribute must leave the default in place");
+        }
+    }
+
     @Test
     void propertiesThatDoSomethingAreNotReportedAsNoOps() {
         Properties properties = 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 6068cb857f..93232d7bbc 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
@@ -174,6 +174,15 @@ class TestPoolProperties {
                 "an unconfigured attribute must read back null");
         Assertions.assertTrue(without.getDbProperties().isEmpty(),
                 "an unconfigured attribute must leave the driver properties 
empty");
+
+        Properties blankAttrs = new Properties();
+        blankAttrs.setProperty("connectionProperties", "   ");
+        PoolConfiguration blank =
+                DataSourceFactory.parsePoolProperties(blankAttrs);
+        Assertions.assertNull(blank.getConnectionProperties(),
+                "a blank attribute must read back the default null, not an 
empty string");
+        Assertions.assertTrue(blank.getDbProperties().isEmpty(),
+                "a blank attribute must leave the driver properties empty");
     }
 
     @Test


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to