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 adb157e5a4062ba56854e5ecfc9b12a47a994ae3 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 14:16:15 2026 +0100 Do not print the live dataSource object in the configuration dump PoolProperties.toString() masked the password and the URL, connectionProperties and interceptor strings, but appended a configured live dataSource object through its own toString(). A driver or wrapper whose toString() includes the URL, user or password therefore leaked the credential into the configuration dump and logs. Print only the data source's class name and identity hash, and update the dump contract. Add a regression test with a dataSource whose toString() carries a secret, and move the throwing-property test to a throwing getter now that the dataSource object is no longer rendered. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 16 ++++- .../tomcat/jdbc/pool/TestPoolProperties.java | 73 +++++++++------------- 2 files changed, 42 insertions(+), 47 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 e5930a7620..ecaa1802c1 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 @@ -943,7 +943,9 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl * masked whenever it carries a secret and prints as {@code null} when * none is configured, and the {@code url}, {@code connectionProperties} * and {@code jdbcInterceptors} strings pass through credential masking, - * so the dump can be logged without carrying a cleartext credential. The + * and a configured live {@code dataSource} object is printed as its class + * and identity rather than through its own {@code toString()}, so the dump + * can be logged without carrying a cleartext credential. The * masking is best effort in the way {@link #maskCredentials(String)} and * {@link #maskInterceptorCredentials(String)} describe, so it removes the * credentials an operator normally configures rather than proving the @@ -971,8 +973,8 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl } buf.append(field); buf.append('='); - // One bad property - a subclass getter that throws, or a - // dataSource object whose toString() throws - must cost + // One bad property - a subclass getter that throws, or an + // object-valued property whose toString() throws - must cost // only its own field, not the rest of the dump. The // append itself is inside this try because buf.append // calls the object's toString() directly: an Error from a @@ -999,6 +1001,14 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl // same way; its pairs live inside parentheses, // which the segment-start walk above cannot see. buf.append(value == null ? "null" : maskInterceptorCredentials((String) value)); + } else if (DataSourceFactory.PROP_DATASOURCE.equals(field)) { + // A live data source object prints its identity + // only: its own toString() may carry the URL, the + // user or a password, and this dump promises not + // to hand those back. + buf.append(value == null ? "null" + : value.getClass().getName() + "@" + + Integer.toHexString(System.identityHashCode(value))); } else { buf.append(value); } 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 93232d7bbc..3c2cceb85a 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 @@ -151,6 +151,25 @@ class TestPoolProperties { + "factory path"); } + @Test + void toStringDoesNotPrintTheConfiguredDataSourceObject() { + PoolProperties properties = new PoolProperties(); + Object dataSource = new Object() { + @Override + public String toString() { + return "jdbc:vendor://host/db?user=sa&password=datasource-secret"; + } + }; + properties.setDataSource(dataSource); + + String dump = properties.toString(); + + Assertions.assertFalse(dump.contains("datasource-secret"), + "the dump must not call the configured data source object's toString()"); + Assertions.assertTrue(dump.contains("dataSource=" + dataSource.getClass().getName()), + "the dump must still identify the configured data source by class"); + } + @Test void setConnectionPropertiesRoundTripsThroughTheFactory() { Properties attrs = new Properties(); @@ -362,41 +381,6 @@ class TestPoolProperties { "an empty string must clear the pairs just like a null one does"); } - @Test - void anErrorFromAPropertyObjectToStringStillProducesADump() { - // buf.append(Object) calls the object's toString() directly, so an - // Error from the arbitrary dataSource object arrives unwrapped, unlike - // one from inside a getter. The dump must not throw over it. - PoolProperties properties = new PoolProperties(); - properties.setDataSource(new Object() { - @Override - public String toString() { - throw new StackOverflowError("boom"); - } - }); - - String asString = Assertions.assertDoesNotThrow(properties::toString); - - Assertions.assertTrue(asString.endsWith("]"), - "the dump must stay bracket-balanced, was: " + asString); - Assertions.assertTrue(asString.contains("dataSource=<error:"), - "the failing property must be reported in its own field, was: " + asString); - } - - @Test - void aFatalErrorFromAPropertyObjectToStringIsPropagated() { - PoolProperties properties = new PoolProperties(); - properties.setDataSource(new Object() { - @Override - public String toString() { - throw new OutOfMemoryError("pretend"); - } - }); - - Assertions.assertThrows(OutOfMemoryError.class, properties::toString, - "fatal errors are propagated, per the module's handleThrowable convention"); - } - @Test void aNamelessInterceptorPropertyIsRefusedAtOnce() { NullPointerException npe = Assertions.assertThrows(NullPointerException.class, @@ -450,23 +434,24 @@ class TestPoolProperties { @Test void aThrowingPropertyFailsOnlyItsOwnFieldOfTheDump() { - PoolProperties properties = new PoolProperties(); - properties.setUrl("jdbc:h2:mem:shown"); - properties.setDataSource(new Object() { + PoolProperties properties = new PoolProperties() { + private static final long serialVersionUID = 1L; + @Override - public String toString() { - throw new IllegalStateException("hostile dataSource"); + public String getUrl() { + throw new IllegalStateException("hostile getter"); } - }); + }; + properties.setDataSourceJNDI("java:comp/env/jdbc/test"); String asString = properties.toString(); Assertions.assertTrue(asString.endsWith("]"), "one throwing property must not truncate the dump, was: " + asString); - Assertions.assertTrue(asString.contains("dataSource=<error:"), + Assertions.assertTrue(asString.contains("url=<error:"), "the failing property must be reported in its own field, was: " + asString); - // dataSourceJNDI is listed after dataSource, so it is lost if the - // failure still truncates the dump. + // dataSourceJNDI is listed after url, so it is lost if the failure + // still truncates the dump. Assertions.assertTrue(asString.contains("dataSourceJNDI"), "the properties must keep being listed after the failing one, was: " + asString); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
