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 521b7e3accedd61e9f62a257a4af494de21f5fb0 Author: opencode <[email protected]> AuthorDate: Mon Oct 26 23:46:46 2026 +0100 Report malformed interceptor properties by name in jdbc-pool The jdbcInterceptors parser took each comma-separated segment inside a Name(...) parameter list and split it on '=' unconditionally. A segment without an '=' - which includes Name() and Name( ), because splitting an empty string yields one empty element - computed substring(0,-1) and aborted pool startup with a raw, position-agnostic StringIndexOutOfBoundsException that named neither the property nor the configuration it came from. Segments that are empty after trimming are now tolerated (they spell "no property", the same as leaving the property out), and a named segment without an '=' fails with an IllegalArgumentException that quotes the property and the full jdbcInterceptors value it came from. The change is behavior-preserving for every configuration that parsed successfully before - both new branches are only reachable for segments that previously threw - and it introduces no new throw. The parsing test that had documented the old raw-failure behavior, including its own note that cleaner handling was called for, is rewritten to pin the new contract: empty and redundant separators tolerated in every position, named failure for valueless segments, and the shapes that still only fail at instantiation unchanged. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 20 +++++++-- .../pool/TestJdbcInterceptorConfigParsing.java | 52 +++++++++++++++++----- 2 files changed, 58 insertions(+), 14 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 dec13f524c..66e1c91c83 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 @@ -431,9 +431,23 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl String propsAsString = interceptorValues[i].substring(propIndex+1, endIndex); String[] props = propsAsString.split(","); for (int j=0; j<props.length; j++) { - int pidx = props[j].indexOf('='); - String propName = props[j].substring(0,pidx).trim(); - String propValue = props[j].substring(pidx+1).trim(); + String prop = props[j]; + if (prop.trim().isEmpty()) { + // an empty segment comes from Name(), Name( ) + // or a redundant separator: there is no property + // to add, so tolerate it rather than fail the + // whole pool start on a cosmetic spelling + continue; + } + int pidx = prop.indexOf('='); + if (pidx < 0) { + throw new IllegalArgumentException( + "Invalid interceptor property '" + prop.trim() + + "', missing '=', in jdbcInterceptors value [" + + interceptorValues[i] + "]"); + } + String propName = prop.substring(0,pidx).trim(); + String propValue = prop.substring(pidx+1).trim(); definitions[i+1].addProperty(new InterceptorProperty(propName,propValue)); } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java index 39f0545767..27de71ba15 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestJdbcInterceptorConfigParsing.java @@ -107,24 +107,54 @@ class TestJdbcInterceptorConfigParsing { } /* - * Some of these should probably be handled more cleanly by the parser, but a few known - * exception scenarios are presented here just to document current behavior. In many cases - * failure in parsing will just be propagated to a definition that will fail later - * when instantiated. Should we be failing faster (and with more detail)? + * Empty parenthesis lists are tolerated (they spell "no properties"); + * a property segment without a '=' fails the parse with a named, + * locating exception instead of the raw StringIndexOutOfBoundsException + * the parser used to let substring(0,-1) produce. Other malformed + * shapes still pass the parser and fail later when instantiated. */ @Test - void someMalformedConfigsFailAtParsingAndSomeOnlyLater() { - String[] exceptionInducingConfigs = { - "EmptyParmsInterceptor()", - "WhitespaceParmsInterceptor( )" + void emptyParameterListIsTolerated() { + String[][] tolerated = { + { "EmptyParmsInterceptor()", "EmptyParmsInterceptor", "0" }, + { "WhitespaceParmsInterceptor( )", "WhitespaceParmsInterceptor", "0" }, + { "RedundantCommaInterceptor(a=1,)", "RedundantCommaInterceptor", "1" }, + { "LeadingCommaInterceptor(,a=1)", "LeadingCommaInterceptor", "1" }, + { "MiddleCommaInterceptor(a=1,,b=2)", "MiddleCommaInterceptor", "2" }, + { "WhitespaceSegmentInterceptor(a=1, ,b=2)", "WhitespaceSegmentInterceptor", "2" }, }; - for (String badConfig : exceptionInducingConfigs) { + for (String[] entry : tolerated) { + PoolProperties props = new PoolProperties(); + props.setJdbcInterceptors(entry[0]); + InterceptorDefinition[] defs = props.getJdbcInterceptorsAsArray(); + Assertions.assertEquals(entry[1], defs[1].getClassName(), + entry[0] + " must parse as a named interceptor"); + Assertions.assertEquals(Integer.parseInt(entry[2]), + defs[1].getProperties().size(), + entry[0] + " must keep only its real properties"); + } + } + + @Test + void propertySegmentWithoutEqualsFailsWithANamedException() { + String[] badConfigs = { + "MalformedParmsInterceptor(a=1,b)", + "MalformedParmsInterceptor(just-a-name)", + }; + for (String badConfig : badConfigs) { PoolProperties props = new PoolProperties(); props.setJdbcInterceptors(badConfig); - Assertions.assertThrows(Exception.class, props::getJdbcInterceptorsAsArray, - "expected a failure for " + badConfig); + IllegalArgumentException thrown = Assertions.assertThrows( + IllegalArgumentException.class, props::getJdbcInterceptorsAsArray, + "expected a named failure for " + badConfig); + Assertions.assertTrue(thrown.getMessage().contains(badConfig), + "the failure must name the configuration it came from, was: " + + thrown.getMessage()); } + } + @Test + void otherMalformedConfigsOnlyFailLater() { String[] noExceptionButInvalidConfigs = { "MalformedParmsInterceptor(= )", "MalformedParmsInterceptor( =)", --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
