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 2c3cb94158572e1f0777d5d280e15d57394d2e88 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 23:37:09 2026 +0100 Harden PoolProperties diagnostics and validator publication toString() formatted a failing property's throwable by appending it directly, so a Throwable whose own toString() throws escaped the per-field catch and truncated every later field. Fall back to the throwable's class name when formatting it fails. The validator and its class name are one configuration, but setValidator and setValidatorClassName published the two fields without the instance monitor the class documentation promises. Synchronize both setters so a concurrent reader cannot observe a torn pair. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 14 ++++++-- .../tomcat/jdbc/pool/TestPoolProperties.java | 40 ++++++++++++++++++++++ 2 files changed, 51 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 2a6a58779e..d2516e7e41 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 @@ -416,7 +416,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl } @Override - public void setValidator(Validator validator) { + public synchronized void setValidator(Validator validator) { this.validator = validator; if (validator != null) { this.validatorClassName = validator.getClass().getName(); @@ -894,7 +894,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl @Override - public void setValidatorClassName(String className) { + public synchronized void setValidatorClassName(String className) { this.validatorClassName = className; validator = null; @@ -1057,7 +1057,15 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl } catch (ReflectiveOperationException | RuntimeException | Error x) { Throwable cause = ExceptionUtils.unwrapInvocationTargetException(x); ExceptionUtils.handleThrowable(cause); - buf.append("<error: ").append(cause).append('>'); + try { + buf.append("<error: ").append(cause).append('>'); + } catch (Throwable hostile) { + // A Throwable whose own toString() throws must + // cost only its own field, not every field after + // it: fall back to its class name. + ExceptionUtils.handleThrowable(hostile); + buf.append("<error: ").append(cause.getClass().getName()).append('>'); + } } buf.append("; "); break; 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 44fc5ac93d..8366a2ff53 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 @@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.lang.reflect.Modifier; import java.sql.Connection; import java.util.ArrayList; import java.util.List; @@ -170,6 +171,33 @@ class TestPoolProperties { "the dump must still identify the configured data source by class"); } + @Test + void aHostileThrowableToStringCostsOnlyItsOwnField() { + PoolProperties properties = new PoolProperties() { + private static final long serialVersionUID = 1L; + + @Override + public String getUrl() { + throw new RuntimeException() { + private static final long serialVersionUID = 1L; + + @Override + public String toString() { + throw new IllegalStateException("hostile toString"); + } + }; + } + }; + + String dump = properties.toString(); + + Assertions.assertTrue(dump.contains("url=<error: "), + "the failing field must be reported, was: " + dump); + Assertions.assertTrue(dump.contains("useStatementFacade="), + "a hostile toString() must not truncate the fields that follow it"); + Assertions.assertTrue(dump.endsWith("]"), "the dump must stay bracket-balanced"); + } + @Test void getJdbcInterceptorsAsArrayReturnsACopy() { PoolProperties properties = new PoolProperties(); @@ -990,6 +1018,18 @@ class TestPoolProperties { "a null string must print as null, was: " + none); } + @Test + void validatorSettersPublishThePairUnderTheInstanceMonitor() throws Exception { + Assertions.assertTrue(Modifier.isSynchronized(PoolProperties.class + .getMethod("setValidator", Validator.class).getModifiers()), + "the validator and its class name are one configuration and must be" + + " published together"); + Assertions.assertTrue(Modifier.isSynchronized(PoolProperties.class + .getMethod("setValidatorClassName", String.class).getModifiers()), + "the validator and its class name are one configuration and must be" + + " published together"); + } + /** The implementation a configured validator must be rebuilt to. */ public static class RebuiltValidator implements Validator { @Override --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
