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 bcc069815df392bb697736b9bbc79880e5594703 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 11:26:47 2026 +0100 Make generic naming setter selection deterministic The generic JNDI resource factory applied a property through the first setFoo overload returned by getMethods() whose parameter type could be built from the configured value. That order is unspecified, so a bean declaring several convertible setters, such as setPort(int) and setPort(long) or setRatio(float) and setRatio(double), could receive the value through a different overload across JDK versions or recompiles. Collect the convertible candidates and sort them by parameter type, preferring the canonical type of the value (int before long, double before float, and so on), then invoke the first candidate the value converts for. Add a regression test with ambiguous int/long and float/double overloads. --- .../jdbc/naming/GenericNamingResourcesFactory.java | 263 ++++++++++----------- .../tomcat/jdbc/naming/TestNamingFactory.java | 49 ++++ 2 files changed, 180 insertions(+), 132 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 5400b0702d..4b0df56c69 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 @@ -23,6 +23,7 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.Enumeration; import java.util.HashSet; import java.util.Hashtable; @@ -231,149 +232,50 @@ public class GenericNamingResourcesFactory implements ObjectFactory { // property bag below knows the bean has its own opinion about // this property. boolean typedSetterMatched = false; - for (int i = 0; i < methods.length; i++) { - boolean ok = true; - if (setter.equals(methods[i].getName()) && methods[i].getParameterTypes().length == 1) { - - // Match - find the type and invoke it + List<Method> typedSetters = new ArrayList<>(); + for (Method method : methods) { + if (setter.equals(method.getName()) && method.getParameterTypes().length == 1) { typedSetterMatched = true; - Class<?> paramType = methods[i].getParameterTypes()[0]; - Object[] params = new Object[1]; - - // Try a setFoo ( int ) - if ("java.lang.Integer".equals(paramType.getName()) || "int".equals(paramType.getName())) { - try { - params[0] = Integer.valueOf(value); - } catch (NumberFormatException ex) { - ok = false; - } - // Try a setFoo ( long ) - } else if ("java.lang.Long".equals(paramType.getName()) || "long".equals(paramType.getName())) { - try { - params[0] = Long.valueOf(value); - } catch (NumberFormatException ex) { - ok = false; - } - // Try a setFoo ( short ) - } else if ("java.lang.Short".equals(paramType.getName()) || - "short".equals(paramType.getName())) { - try { - params[0] = Short.valueOf(value); - } catch (NumberFormatException ex) { - ok = false; - } - // Try a setFoo ( byte ) - } else if ("java.lang.Byte".equals(paramType.getName()) || - "byte".equals(paramType.getName())) { - try { - params[0] = Byte.valueOf(value); - } catch (NumberFormatException ex) { - ok = false; - } - // Try a setFoo ( float ) - } else if ("java.lang.Float".equals(paramType.getName()) || - "float".equals(paramType.getName())) { - try { - params[0] = Float.valueOf(value); - } catch (NumberFormatException ex) { - ok = false; - } - // Try a setFoo ( double ) - } else if ("java.lang.Double".equals(paramType.getName()) || - "double".equals(paramType.getName())) { - try { - params[0] = Double.valueOf(value); - } catch (NumberFormatException ex) { - ok = false; - } - // Try a setFoo ( char or Character ) - } else if ("java.lang.Character".equals(paramType.getName()) || - "char".equals(paramType.getName())) { - // Take the first character of the value, like - // BeanFactory. Empty content has no first - // character: do not fabricate one. - if (value.isEmpty()) { - ok = false; - } else { - params[0] = Character.valueOf(value.charAt(0)); - } - - // Try a setFoo ( boolean ) - } else if ("java.lang.Boolean".equals(paramType.getName()) || - "boolean".equals(paramType.getName())) { - // Boolean.valueOf() reports everything it does not - // recognise, empty content included, as false. - // Anything that does not spell a boolean - empty - // content, whitespace-padded or misspelled values - - // is not a configured value: let the caller report - // the property as not applied instead of silently - // applying false. Same policy as the pool's own - // DataSourceFactory.parseBoolean(). - String trimmed = value.trim(); - if ("true".equalsIgnoreCase(trimmed)) { - params[0] = Boolean.TRUE; - } else if ("false".equalsIgnoreCase(trimmed)) { - params[0] = Boolean.FALSE; - } else { - ok = false; - } - - // Try a setFoo ( InetAddress ) - } else if ("java.net.InetAddress".equals(paramType.getName())) { - // getByName() documents the empty string as the - // loopback address instead of failing, and an empty - // host is not a request for loopback: do not invoke - // the setter with a fabricated address. - if (value.isEmpty()) { - ok = false; - } else { - try { - params[0] = InetAddress.getByName(value); - } catch (UnknownHostException exc) { - if (log.isDebugEnabled()) { - log.debug("Unable to resolve host name: " + value); - } - ok = false; - } - } - - // Unknown type - } else { - // Unsupported parameter type: the value cannot be - // built for it, so the setter is not invoked with a - // fabricated null and the property is reported as not - // applied. - ok = false; - if (log.isDebugEnabled()) { - log.debug("Unknown type " + paramType.getName()); - } - } - - if (ok) { - methods[i].invoke(o, params); - return SetResult.APPLIED; + if (setterTypeRank(method.getParameterTypes()[0]) >= 0) { + typedSetters.add(method); } } + } + // getMethods() returns the overloads in an unspecified order. + // Sort them by parameter type so a bean with several convertible + // setters (for example setPort(int) and setPort(long)) always + // receives the value through the same one. + typedSetters.sort(Comparator + .comparingInt((Method m) -> setterTypeRank(m.getParameterTypes()[0])) + .thenComparing(m -> m.getParameterTypes()[0].getName())); + for (Method method : typedSetters) { + Object converted = convertSetterValue(method.getParameterTypes()[0], value); + if (converted != CONVERSION_FAILED) { + method.invoke(o, new Object[] { converted }); + return SetResult.APPLIED; + } + } - // 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())) { - Class<?>[] paramT = methods[i].getParameterTypes(); + // 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. + for (Method method : methods) { + if ("setProperty".equals(method.getName())) { + Class<?>[] paramT = method.getParameterTypes(); if (paramT.length == 2 && paramT[0].isAssignableFrom(String.class) && paramT[1].isAssignableFrom(String.class)) { // A boxed Boolean bag refuses a property the same // way a primitive boolean one does: it belongs in // the bucket whose return value is evaluated, not // in the fire-and-forget bucket. - if (methods[i].getReturnType() == Boolean.TYPE - || methods[i].getReturnType() == Boolean.class) { - setPropertyMethodBool = methods[i]; + if (method.getReturnType() == Boolean.TYPE + || method.getReturnType() == Boolean.class) { + setPropertyMethodBool = method; } else { - setPropertyMethodVoid = methods[i]; + setPropertyMethodVoid = method; } } } @@ -419,6 +321,103 @@ public class GenericNamingResourcesFactory implements ObjectFactory { return SetResult.UNREPORTED; } + /** Sentinel returned by {@link #convertSetterValue} when the value does not fit the type. */ + private static final Object CONVERSION_FAILED = new Object(); + + /** + * The supported typed-setter parameter types, ordered so that a bean with + * several convertible {@code setFoo} overloads always receives a value + * through the same (most specific) setter. The order follows Java's + * preference for the canonical type of a literal: {@code int} before the + * other integral types, {@code double} before {@code float}. + * + * @param paramType The setter parameter type + * + * @return The rank of a supported type, or -1 if the type is not supported + */ + private static int setterTypeRank(Class<?> paramType) { + String name = paramType.getName(); + if ("java.lang.Integer".equals(name) || "int".equals(name)) { + return 0; + } else if ("java.lang.Long".equals(name) || "long".equals(name)) { + return 1; + } else if ("java.lang.Double".equals(name) || "double".equals(name)) { + return 2; + } else if ("java.lang.Float".equals(name) || "float".equals(name)) { + return 3; + } else if ("java.lang.Short".equals(name) || "short".equals(name)) { + return 4; + } else if ("java.lang.Byte".equals(name) || "byte".equals(name)) { + return 5; + } else if ("java.lang.Boolean".equals(name) || "boolean".equals(name)) { + return 6; + } else if ("java.lang.Character".equals(name) || "char".equals(name)) { + return 7; + } else if ("java.net.InetAddress".equals(name)) { + return 8; + } + return -1; + } + + /** + * Builds a setter argument from the configured value. + * + * @param paramType The setter parameter type + * @param value The configured value + * + * @return The converted value, or {@link #CONVERSION_FAILED} when the + * value cannot be built for the type + */ + private static Object convertSetterValue(Class<?> paramType, String value) { + String name = paramType.getName(); + try { + if ("java.lang.Integer".equals(name) || "int".equals(name)) { + return Integer.valueOf(value); + } else if ("java.lang.Long".equals(name) || "long".equals(name)) { + return Long.valueOf(value); + } else if ("java.lang.Double".equals(name) || "double".equals(name)) { + return Double.valueOf(value); + } else if ("java.lang.Float".equals(name) || "float".equals(name)) { + return Float.valueOf(value); + } else if ("java.lang.Short".equals(name) || "short".equals(name)) { + return Short.valueOf(value); + } else if ("java.lang.Byte".equals(name) || "byte".equals(name)) { + return Byte.valueOf(value); + } else if ("java.lang.Boolean".equals(name) || "boolean".equals(name)) { + // Boolean.valueOf() reports everything it does not recognise, + // empty content included, as false. Only a spelled-out boolean + // is a configured value. + String trimmed = value.trim(); + if ("true".equalsIgnoreCase(trimmed)) { + return Boolean.TRUE; + } else if ("false".equalsIgnoreCase(trimmed)) { + return Boolean.FALSE; + } + } else if ("java.lang.Character".equals(name) || "char".equals(name)) { + // Empty content has no first character: do not fabricate one. + if (!value.isEmpty()) { + return Character.valueOf(value.charAt(0)); + } + } else if ("java.net.InetAddress".equals(name)) { + // getByName() documents the empty string as the loopback + // address instead of failing, and an empty host is not a + // request for loopback. + if (!value.isEmpty()) { + try { + return InetAddress.getByName(value); + } catch (UnknownHostException exc) { + if (log.isDebugEnabled()) { + log.debug("Unable to resolve host name: " + value); + } + } + } + } + } catch (NumberFormatException ex) { + // The value is not a number this type can represent. + } + return CONVERSION_FAILED; + } + /** * Apply a configured property through the setProperty(name, value) * fallback. Called from inside setProperty's try, so every exception the 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 aeada41078..041fcbe1f3 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 @@ -1189,6 +1189,55 @@ public class TestNamingFactory extends JdbcPoolBaseTest { } } + @Test + void convertibleSetterOverloadsAreChosenDeterministically() throws Exception { + Reference ref = new Reference(AmbiguousOverloadTarget.class.getName(), + null, null); + ref.add(new StringRefAddr("port", "1527")); + ref.add(new StringRefAddr("ratio", "1.5")); + + AmbiguousOverloadTarget.last = null; + Object result = new GenericNamingResourcesFactory().getObjectInstance( + ref, new CompositeName("test/ambiguous"), null, null); + + Assertions.assertInstanceOf(AmbiguousOverloadTarget.class, result); + AmbiguousOverloadTarget target = (AmbiguousOverloadTarget) result; + Assertions.assertEquals("int", target.getPortSetter(), + "the int overload must win over the long one for an integral value"); + Assertions.assertEquals("double", target.getRatioSetter(), + "the double overload must win over the float one for a decimal value"); + } + + /** Target with several convertible setters for the same property. */ + public static class AmbiguousOverloadTarget { + static AmbiguousOverloadTarget last; + private String portSetter; + private String ratioSetter; + + public AmbiguousOverloadTarget() { + last = this; + } + + public void setPort(int port) { + portSetter = "int"; + } + public void setPort(long port) { + portSetter = "long"; + } + public String getPortSetter() { + return portSetter; + } + public void setRatio(float ratio) { + ratioSetter = "float"; + } + public void setRatio(double ratio) { + ratioSetter = "double"; + } + public String getRatioSetter() { + return ratioSetter; + } + } + @Test void reservedReferenceAddressesAreNotAppliedAsProperties() throws Exception { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
