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 0318dc9ea69071eb4f7020ea1a596eb5e07738af Author: opencode <[email protected]> AuthorDate: Sun Oct 25 16:43:29 2026 +0100 Do not invoke a naming factory setter with an unsupported parameter type using a null value GenericNamingResourcesFactory.setProperty(), a forked copy of the legacy IntrospectionUtils.setProperty(), only converts the configured string value for String, int/Integer, long/Long, boolean/Boolean and java.net.InetAddress parameter types. For any other single-argument setter parameter type the 'Unknown type' branch left the 'ok' flag at its initial value true, so the setter was invoked with a null argument and the failure was reported as a successful property set. A configured property such as a java.util.Properties setter was therefore silently nulled, destroying a usable default, and because the scan returned on the first name match, an overloaded setter that could have matched was shadowed: with setPort(int) and setPort(Properties) both present and the value '12o', setPort(null) was invoked, leaving the numeric default in place and nulling the other field, all while returning true. Set ok to false in the unsupported-type branch so the setter is never invoked with null and the property falls through to a generic setProperty(name, value) method if the target has one, or is otherwise reported as not configured. This matches the fix already present in org.apache.tomcat.util.IntrospectionUtils in this repository. Add regression tests to TestNamingFactory covering the unsupported parameter type case and the overloaded-setter case with a value that fails numeric conversion; both fail without the fix. --- .../jdbc/naming/GenericNamingResourcesFactory.java | 4 + .../tomcat/jdbc/naming/TestNamingFactory.java | 85 ++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java index 00dd8e80f1..525b0f84eb 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java @@ -159,6 +159,10 @@ public class GenericNamingResourcesFactory implements ObjectFactory { // Unknown type } else { + // Unsupported parameter type. Do not invoke the + // method with a null value. Let the caller fall back + // to setProperty(name, value) or handle the failure. + ok = false; if (log.isDebugEnabled()) { log.debug("IntrospectionUtils: Unknown type " + paramType.getName()); 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 ce21b11464..367ced9e8a 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 @@ -22,6 +22,7 @@ import javax.naming.Reference; import javax.naming.StringRefAddr; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; @@ -102,6 +103,90 @@ class TestNamingFactory extends JdbcPoolBaseTest { assertTrue(target.isEnabled()); } + @Test + void unsupportedSetterTypeIsNotInvokedWithNull() throws Exception { + Reference ref = new Reference(UnsupportedTypeTarget.class.getName(), + null, null); + // "props" only matches setProps(Properties) whose parameter type the + // factory cannot convert a string to. It must not be invoked with a + // null argument, wiping the configured default. + ref.add(new StringRefAddr("props", "anything")); + + Object result = new GenericNamingResourcesFactory().getObjectInstance( + ref, new CompositeName("test/unsupported"), null, null); + assertInstanceOf(UnsupportedTypeTarget.class, result); + UnsupportedTypeTarget target = (UnsupportedTypeTarget) result; + assertFalse(target.isPropsInvoked(), + "setter with an unsupported parameter type must not be " + + "invoked with a null value"); + assertEquals(UnsupportedTypeTarget.DEFAULT_PROPS, target.getProps(), + "the default value must survive an inapplicable property"); + } + + @Test + void overloadedSetterWithBadValueDoesNotNullOtherField() + throws Exception { + Reference ref = new Reference(OverloadedTarget.class.getName(), + null, null); + // "port" matches both setPort(int) and setPort(Properties). The + // numeric conversion of the value fails; the unsupported overload + // must not be invoked with null instead. + ref.add(new StringRefAddr("port", "12o")); + + Object result = new GenericNamingResourcesFactory().getObjectInstance( + ref, new CompositeName("test/overloaded"), null, null); + assertInstanceOf(OverloadedTarget.class, result); + OverloadedTarget target = (OverloadedTarget) result; + assertFalse(target.isPropsInvoked(), + "the unsupported overload must not be invoked with null when " + + "the numeric conversion fails"); + assertEquals(-1, target.getPort(), + "the numeric property must keep its default on a bad value"); + } + + /** Target with a setter of a type the factory cannot convert to. */ + public static class UnsupportedTypeTarget { + static final java.util.Properties DEFAULT_PROPS = + new java.util.Properties(); + private java.util.Properties props = DEFAULT_PROPS; + private boolean propsInvoked; + + public java.util.Properties getProps() { + return props; + } + public void setProps(java.util.Properties props) { + this.propsInvoked = true; + this.props = props; + } + public boolean isPropsInvoked() { + return propsInvoked; + } + } + + /** Target with an overloaded setter, one supported and one not. */ + public static class OverloadedTarget { + private int port = -1; + private java.util.Properties props; + private boolean propsInvoked; + + public int getPort() { + return port; + } + public void setPort(int port) { + this.port = port; + } + public java.util.Properties getProps() { + return props; + } + public void setPort(java.util.Properties props) { + this.propsInvoked = true; + this.props = props; + } + public boolean isPropsInvoked() { + return propsInvoked; + } + } + /** Plain configuration target for the generic naming factory. */ public static class NamingTarget { private String name; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
