This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3163e6eb2493bfd32375e07d455c6cc600ea544e Author: opencode <[email protected]> AuthorDate: Thu Oct 29 13:24:28 2026 +0100 Match the naming factory setProperty fallback by signature, not name Only methods named setProperty were recorded as the generic (name, value) property-bag fallback, whatever their parameter types, so a setProperty of any other signature was then invoked reflectively with the two string arguments and failed with an IllegalArgumentException that was logged as a warning with a reflective stack trace, masking the accurate "property was not applied" report that followed it; when a usable overload existed next to the uncallable one, which method the name-only recording kept also depended on the reflection order. The recording now requires two parameters that can actually accept a String, which keeps every fallback that worked before, Hashtable-style setProperty(Object, Object) bags included, and leaves an uncallable overload unrecorded so the single honest warning names the property that was not applied. --- .../jdbc/naming/GenericNamingResourcesFactory.java | 26 ++-- .../tomcat/jdbc/naming/TestNamingFactory.java | 156 +++++++++++++++++++++ 2 files changed, 174 insertions(+), 8 deletions(-) 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 c43522b62d..16c2d4c6de 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 @@ -217,14 +217,22 @@ public class GenericNamingResourcesFactory implements ObjectFactory { } } - // Save "setProperty" for later + // Save "setProperty" for later, but only one that the two + // string arguments below can actually be passed to: a + // setProperty of any other signature is not a (name, value) + // property bag, and invoking it reflectively with those + // arguments would only fail with an IllegalArgumentException + // that masks the "property was not applied" report. if ("setProperty".equals(methods[i].getName())) { - if (methods[i].getReturnType() == Boolean.TYPE) { - setPropertyMethodBool = methods[i]; - } else { - setPropertyMethodVoid = methods[i]; + Class<?>[] paramT = methods[i].getParameterTypes(); + if (paramT.length == 2 && paramT[0].isAssignableFrom(String.class) + && paramT[1].isAssignableFrom(String.class)) { + if (methods[i].getReturnType() == Boolean.TYPE) { + setPropertyMethodBool = methods[i]; + } else { + setPropertyMethodVoid = methods[i]; + } } - } } @@ -237,8 +245,10 @@ public class GenericNamingResourcesFactory implements ObjectFactory { try { return ((Boolean) setPropertyMethodBool.invoke(o, params)).booleanValue(); } catch (IllegalArgumentException biae) { - // The boolean method had the wrong - // parameter types. lets try the other + // Defensive: the recorded method's parameter types + // were checked above, so invoking it with the name + // and value should not fail on the arguments; if it + // somehow does, fall back to the other method. if (setPropertyMethodVoid != null) { setPropertyMethodVoid.invoke(o, params); return true; 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 01b7026ec3..96de663cea 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 @@ -366,6 +366,162 @@ public class TestNamingFactory extends JdbcPoolBaseTest { } } + @Test + void incompatibleSetPropertyOverloadIsNotInvokedAsFallback() + throws Exception { + Logger logger = Logger.getLogger( + GenericNamingResourcesFactory.class.getName()); + Level originalLevel = logger.getLevel(); + final List<LogRecord> records = new ArrayList<>(); + Handler handler = new Handler() { + @Override + public void publish(LogRecord record) { + records.add(record); + } + @Override + public void flush() { + // no buffering + } + @Override + public void close() { + // nothing to release + } + }; + logger.addHandler(handler); + logger.setLevel(Level.FINEST); + try { + Reference ref = new Reference( + DecoySetPropertyTarget.class.getName(), null, null); + // "whatever" matches no setter; only the setProperty overloads + // could be consulted as the (name, value) fallback, and neither + // one can accept those arguments + ref.add(new StringRefAddr("whatever", "v")); + + Object result = new GenericNamingResourcesFactory() + .getObjectInstance( + ref, new CompositeName("test/decoy"), null, null); + Assertions.assertInstanceOf(DecoySetPropertyTarget.class, result); + Assertions.assertFalse( + ((DecoySetPropertyTarget) result).isInvoked(), + "a setProperty overload the factory cannot call with the " + + "configured name and value must not run"); + } finally { + logger.removeHandler(handler); + logger.setLevel(originalLevel); + } + boolean warned = false; + for (LogRecord record : records) { + Assertions.assertFalse( + record.getThrown() instanceof IllegalArgumentException, + "the fallback must not be recorded by method name only: " + + "invoking an unreachable setProperty overload " + + "reflectively produced " + record.getMessage() + + " with " + record.getThrown()); + if (record.getLevel().intValue() >= Level.WARNING.intValue() + && record.getMessage() != null + && record.getMessage().contains("[whatever]")) { + warned = true; + } + } + Assertions.assertTrue(warned, + "the unapplied property must be reported once, at warn level"); + } + + /** + * Target whose setProperty overloads cannot accept the (name, value) + * arguments of the generic fallback. The two overloads cover both + * recording buckets, boolean and default return type; either one, + * invoked with two arguments, fails reflectively. + */ + public static class DecoySetPropertyTarget { + private boolean invoked; + + public boolean isInvoked() { + return invoked; + } + public boolean setProperty(String name) { + invoked = true; + return true; + } + public void setProperty(int index) { + invoked = true; + } + } + + @Test + void propertyBagFallbackSurvivesIncompatibleOverload() + throws Exception { + Reference ref = new Reference(MixedBagTarget.class.getName(), + null, null); + ref.add(new StringRefAddr("databaseName", "sample")); + + Object result = new GenericNamingResourcesFactory().getObjectInstance( + ref, new CompositeName("test/mixedbag"), null, null); + Assertions.assertInstanceOf(MixedBagTarget.class, result); + MixedBagTarget target = (MixedBagTarget) result; + Assertions.assertEquals(java.util.Collections.singletonMap( + "databaseName", "sample"), + target.getBag(), + "the (name, value) overload must be used as the fallback " + + "even when an unreachable overload exists"); + Assertions.assertFalse(target.isIndexInvoked(), + "the unreachable overload must not be invoked"); + } + + /** + * Target carrying a usable setProperty(name, value) property bag next + * to an unrelated setProperty overload. Recording used to match on the + * method name only, so which of the two overloads the fallback ended + * up calling depended on the reflection method order. + */ + public static class MixedBagTarget { + private final java.util.Map<String, String> bag = + new java.util.LinkedHashMap<>(); + private boolean indexInvoked; + + public void setProperty(String name, String value) { + bag.put(name, value); + } + public java.util.Map<String, String> getBag() { + return bag; + } + public void setProperty(int index) { + indexInvoked = true; + } + public boolean isIndexInvoked() { + return indexInvoked; + } + } + + @Test + void hashtableStyleSetPropertyBagIsApplied() throws Exception { + Reference ref = new Reference(ObjectBagTarget.class.getName(), + null, null); + ref.add(new StringRefAddr("databaseName", "sample")); + + Object result = new GenericNamingResourcesFactory().getObjectInstance( + ref, new CompositeName("test/objectbag"), null, null); + Assertions.assertInstanceOf(ObjectBagTarget.class, result); + Assertions.assertEquals(java.util.Collections.singletonMap( + "databaseName", "sample"), + ((ObjectBagTarget) result).getBag(), + "a setProperty(Object, Object) bag can accept the string " + + "name and value and must stay usable"); + } + + /** Target with a Hashtable-style setProperty(Object, Object) bag. */ + public static class ObjectBagTarget { + private final java.util.Map<Object, Object> bag = + new java.util.LinkedHashMap<>(); + + public void setProperty(Object key, Object value) { + bag.put(key, value); + } + public java.util.Map<Object, Object> getBag() { + return bag; + } + } + @Test void propertySetFailuresAreReportedAsWarnings() throws Exception { Logger logger = Logger.getLogger( --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
