This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 4681d1bf8b Fix xreflection stack overflow issue found by code review
4681d1bf8b is described below
commit 4681d1bf8b8ddb66b6378b843e9d111844ee0173
Author: remm <[email protected]>
AuthorDate: Mon Aug 31 10:38:54 2026 +0200
Fix xreflection stack overflow issue found by code review
Stack overflow due to bad fallback to generic getProperty (reproduced).
Missing capitalize for "is" property.
Additional fixes from commit review: generic getProperty could be
invoked first, missing default interface methods, auto boxing.
Tested with a harness and the Tomcat classes, the results now match
IntrospectionUtils, except for exceptions (no generated try/catch).
Additional fixes coauthored with OpenCode.
---
.../ObjectReflectionPropertyInspector.java | 69 +++++----
.../util/xreflection/ReflectionProperty.java | 6 +-
.../tomcat/util/xreflection/SetPropertyClass.java | 159 ++++++++++++++++-----
webapps/docs/changelog.xml | 6 +
4 files changed, 165 insertions(+), 75 deletions(-)
diff --git
a/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
b/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
index cd79192e65..18a6428cb2 100644
---
a/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
+++
b/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
@@ -110,7 +110,8 @@ public final class ObjectReflectionPropertyInspector {
// types of properties that IntrospectionUtils.setProperty supports
private static final Set<Class<?>> ALLOWED_TYPES =
Collections.unmodifiableSet(
- new LinkedHashSet<>(Arrays.asList(Boolean.TYPE, Integer.TYPE,
Long.TYPE, String.class, InetAddress.class)));
+ new LinkedHashSet<>(Arrays.asList(Boolean.TYPE, Boolean.class,
Integer.TYPE, Integer.class,
+ Long.TYPE, Long.class, String.class, InetAddress.class)));
private static final Map<Class<?>,SetPropertyClass> classes = new
LinkedHashMap<>();
/**
@@ -158,7 +159,7 @@ public final class ObjectReflectionPropertyInspector {
static Method findGetter(Class<?> declaringClass, String propertyName) {
for (String getterName : Arrays.asList("get" +
IntrospectionUtils.capitalize(propertyName),
- "is" + propertyName)) {
+ "is" + IntrospectionUtils.capitalize(propertyName))) {
try {
Method method = declaringClass.getMethod(getterName);
if (!Modifier.isPrivate(method.getModifiers())) {
@@ -168,14 +169,6 @@ public final class ObjectReflectionPropertyInspector {
// Ignore
}
}
- try {
- Method method = declaringClass.getMethod("getProperty",
String.class, String.class);
- if (!Modifier.isPrivate(method.getModifiers())) {
- return method;
- }
- } catch (NoSuchMethodException e) {
- // Ignore
- }
return null;
}
@@ -189,14 +182,6 @@ public final class ObjectReflectionPropertyInspector {
} catch (NoSuchMethodException e) {
// Ignore
}
- try {
- Method method = declaringClass.getMethod("setProperty",
String.class, String.class);
- if (!Modifier.isPrivate(method.getModifiers())) {
- return method;
- }
- } catch (NoSuchMethodException e) {
- // Ignore
- }
return null;
}
@@ -217,24 +202,16 @@ public final class ObjectReflectionPropertyInspector {
SetPropertyClass spc = getOrCreateSetPropertyClass(clazz);
final Method[] methods = clazz.getDeclaredMethods();
for (Method method : methods) {
- if (isAllowedSetMethod(method)) {
- String propertyName =
decapitalize(method.getName().substring(3));
- Class<?> propertyType = method.getParameterTypes()[0];
- Method getter = findGetter(clazz, propertyName);
- Method setter = findSetter(clazz, propertyName, propertyType);
- ReflectionProperty property =
- new ReflectionProperty(spc.getClazz().getName(),
propertyName, propertyType, setter, getter);
- spc.addProperty(property);
- } else if (isAllowedGetMethod(method)) {
- boolean startsWithIs = method.getName().startsWith("is");
- String propertyName =
decapitalize(method.getName().substring(startsWithIs ? 2 : 3));
- Class<?> propertyType = method.getReturnType();
- Method getter = findGetter(clazz, propertyName);
- Method setter = findSetter(clazz, propertyName, propertyType);
- ReflectionProperty property =
- new ReflectionProperty(spc.getClazz().getName(),
propertyName, propertyType, setter, getter);
- spc.addProperty(property);
+ addPropertyForMethod(spc, clazz, method);
+ }
+ // Default methods on implemented interfaces are not returned by
+ // getDeclaredMethods() but are visible to IntrospectionUtils, so they
+ // need to be processed as well
+ for (Method method : clazz.getMethods()) {
+ if (!method.getDeclaringClass().isInterface()) {
+ continue;
}
+ addPropertyForMethod(spc, clazz, method);
}
final Field[] fields = clazz.getDeclaredFields();
@@ -257,4 +234,26 @@ public final class ObjectReflectionPropertyInspector {
return spc;
}
}
+
+
+ private static void addPropertyForMethod(SetPropertyClass spc, Class<?>
clazz, Method method) {
+ if (isAllowedSetMethod(method)) {
+ String propertyName = decapitalize(method.getName().substring(3));
+ Class<?> propertyType = method.getParameterTypes()[0];
+ Method getter = findGetter(clazz, propertyName);
+ Method setter = findSetter(clazz, propertyName, propertyType);
+ ReflectionProperty property =
+ new ReflectionProperty(spc.getClazz().getName(),
propertyName, propertyType, setter, getter);
+ spc.addProperty(property);
+ } else if (isAllowedGetMethod(method)) {
+ boolean startsWithIs = method.getName().startsWith("is");
+ String propertyName =
decapitalize(method.getName().substring(startsWithIs ? 2 : 3));
+ Class<?> propertyType = method.getReturnType();
+ Method getter = findGetter(clazz, propertyName);
+ Method setter = findSetter(clazz, propertyName, propertyType);
+ ReflectionProperty property =
+ new ReflectionProperty(spc.getClazz().getName(),
propertyName, propertyType, setter, getter);
+ spc.addProperty(property);
+ }
+ }
}
diff --git a/java/org/apache/tomcat/util/xreflection/ReflectionProperty.java
b/java/org/apache/tomcat/util/xreflection/ReflectionProperty.java
index 17ea16f500..24a5a889bf 100644
--- a/java/org/apache/tomcat/util/xreflection/ReflectionProperty.java
+++ b/java/org/apache/tomcat/util/xreflection/ReflectionProperty.java
@@ -74,13 +74,13 @@ record ReflectionProperty(String clazz, String
propertyName, Class<?> propertyTy
if (getPropertyType() == String.class) {
return valueVarName;
}
- if (getPropertyType() == Boolean.TYPE) {
+ if (getPropertyType() == Boolean.TYPE || getPropertyType() ==
Boolean.class) {
return "Boolean.valueOf(" + valueVarName + ")";
}
- if (getPropertyType() == Long.TYPE) {
+ if (getPropertyType() == Long.TYPE || getPropertyType() == Long.class)
{
return "Long.valueOf(" + valueVarName + ")";
}
- if (getPropertyType() == Integer.TYPE) {
+ if (getPropertyType() == Integer.TYPE || getPropertyType() ==
Integer.class) {
return "Integer.valueOf(" + valueVarName + ")";
}
if (getPropertyType() == InetAddress.class) {
diff --git a/java/org/apache/tomcat/util/xreflection/SetPropertyClass.java
b/java/org/apache/tomcat/util/xreflection/SetPropertyClass.java
index 24de0ad0ee..aeb9df8ce9 100644
--- a/java/org/apache/tomcat/util/xreflection/SetPropertyClass.java
+++ b/java/org/apache/tomcat/util/xreflection/SetPropertyClass.java
@@ -261,7 +261,24 @@ public final class SetPropertyClass implements
Comparable<SetPropertyClass> {
.append('}')
.append(System.lineSeparator());
- // we have a generic setProperty(String, String) method, invoke it
+ // invoke the parent first so that a specific setter on a superclass
+ // is preferred over the generic setProperty method on this class
+ if (getParent() != null) {
+ String parentInvocation =
getParent().generateParentSetPropertyForMethodInvocation();
+ code.append(ReflectionLessCodeGenerator.getIndent(2))
+ .append("if (")
+ .append(parentInvocation, 0, parentInvocation.length() - 1)
+ .append(") {")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(3))
+ .append("return true;")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(2))
+ .append('}')
+ .append(System.lineSeparator());
+ }
+
+ // we have a generic setProperty method, invoke it
if (getGenericSetPropertyMethod() != null) {
ReflectionProperty p = new ReflectionProperty(
clazz.getName(),
@@ -270,26 +287,31 @@ public final class SetPropertyClass implements
Comparable<SetPropertyClass> {
getGenericSetPropertyMethod(),
null
);
- code.append(ReflectionLessCodeGenerator.getIndent(2))
- .append("if (")
- .append(SETP_VAR_NAME)
- .append(") {")
- .append(System.lineSeparator())
- .append(ReflectionLessCodeGenerator.getIndent(3))
- .append(generateSetPropertyMethod(p))
- .append(System.lineSeparator())
- .append(ReflectionLessCodeGenerator.getIndent(3))
- .append("return true;")
- .append(System.lineSeparator())
- .append(ReflectionLessCodeGenerator.getIndent(2))
- .append('}')
- .append(System.lineSeparator());
+ code.append(ReflectionLessCodeGenerator.getIndent(2))
+ .append("if (")
+ .append(SETP_VAR_NAME)
+ .append(") {")
+ .append(System.lineSeparator());
+ if (getGenericSetPropertyMethod().getReturnType() == Boolean.TYPE)
{
+ code.append(ReflectionLessCodeGenerator.getIndent(3))
+ .append("return ")
+ .append(generateSetPropertyMethod(p))
+ .append(System.lineSeparator());
+ } else {
+ code.append(ReflectionLessCodeGenerator.getIndent(3))
+ .append(generateSetPropertyMethod(p))
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(3))
+ .append("return true;")
+ .append(System.lineSeparator());
+ }
+ code.append(ReflectionLessCodeGenerator.getIndent(2))
+ .append('}')
+ .append(System.lineSeparator());
}
- // invoke parent or return false
code.append(ReflectionLessCodeGenerator.getIndent(2))
- .append("return ")
- .append(getSetPropertyForExitStatement())
+ .append("return false;")
.append(System.lineSeparator())
.append(ReflectionLessCodeGenerator.getIndent(1))
.append('}');
@@ -298,15 +320,6 @@ public final class SetPropertyClass implements
Comparable<SetPropertyClass> {
//@formatter:on
}
- private String getSetPropertyForExitStatement() {
-
- return (getParent() != null) ?
- // invoke the parent if we have one
- getParent().generateParentSetPropertyForMethodInvocation() :
- // if we invoke setProperty, return true, return false
otherwise
- getGenericSetPropertyMethod() != null ? "true;" : "false;";
- }
-
/**
* Generate a switch case statement that invokes the setProperty method for
* this class.
@@ -455,6 +468,38 @@ public final class SetPropertyClass implements
Comparable<SetPropertyClass> {
return "null;";
}
+ /**
+ * Return the names of the properties for which a specific getter method
+ * (a public zero-argument <code>get*</code> or <code>is*</code> method)
+ * is available on this class or one of its superclasses. Only names that
+ * {@code IntrospectionUtils.getProperty} can reconstruct from the getter
+ * name are included.
+ *
+ * @return the property names with a specific getter
+ */
+ private Set<String> getSpecificGetterPropertyNames() {
+ Set<String> names = new TreeSet<>();
+ for (Method method : clazz.getMethods()) {
+ if (method.getParameterTypes().length != 0) {
+ continue;
+ }
+ String methodName = method.getName();
+ String rest;
+ if (methodName.startsWith("get") && methodName.length() > 3) {
+ rest = methodName.substring(3);
+ } else if (methodName.startsWith("is") && methodName.length() > 2)
{
+ rest = methodName.substring(2);
+ } else {
+ continue;
+ }
+ String propertyName =
ObjectReflectionPropertyInspector.decapitalize(rest);
+ if (IntrospectionUtils.capitalize(propertyName).equals(rest)) {
+ names.add(propertyName);
+ }
+ }
+ return names;
+ }
+
/**
* Generate a complete getProperty method for this class with switch-case
@@ -516,7 +561,10 @@ public final class SetPropertyClass implements
Comparable<SetPropertyClass> {
.append('}')
.append(System.lineSeparator());
- // we have a generic getProperty(String, String) method, invoke it
+ // we have a generic getProperty(String) method, invoke it for property
+ // names that do not have a specific getter on this class or a
+ // superclass (IntrospectionUtils only falls back to the generic
+ // method when no specific getter is found)
if (getGenericGetPropertyMethod() != null) {
ReflectionProperty p = new ReflectionProperty(
clazz.getName(),
@@ -525,15 +573,52 @@ public final class SetPropertyClass implements
Comparable<SetPropertyClass> {
null,
getGenericGetPropertyMethod()
);
- code.append(ReflectionLessCodeGenerator.getIndent(2))
- .append("if (result == null) {")
- .append(System.lineSeparator())
- .append(ReflectionLessCodeGenerator.getIndent(3))
- .append(generateGetPropertyMethod(p))
- .append(System.lineSeparator())
- .append(ReflectionLessCodeGenerator.getIndent(2))
- .append('}')
- .append(System.lineSeparator());
+ Set<String> getterNames = getSpecificGetterPropertyNames();
+ if (getterNames.isEmpty()) {
+ code.append(ReflectionLessCodeGenerator.getIndent(2))
+ .append("if (result == null) {")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(3))
+ .append(generateGetPropertyMethod(p))
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(2))
+ .append('}')
+ .append(System.lineSeparator());
+ } else {
+ code.append(ReflectionLessCodeGenerator.getIndent(2))
+ .append("if (result == null) {")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(3))
+ .append("switch (")
+ .append(NAME_VAR_NAME)
+ .append(") {")
+ .append(System.lineSeparator());
+ for (String name : getterNames) {
+ code.append(ReflectionLessCodeGenerator.getIndent(4))
+ .append("case \"")
+ .append(name)
+ .append("\":")
+ .append(System.lineSeparator());
+ }
+ code.append(ReflectionLessCodeGenerator.getIndent(4))
+ .append("break;")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(4))
+ .append("default:")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(5))
+ .append(generateGetPropertyMethod(p))
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(4))
+ .append("break;")
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(3))
+ .append('}')
+ .append(System.lineSeparator())
+ .append(ReflectionLessCodeGenerator.getIndent(2))
+ .append('}')
+ .append(System.lineSeparator());
+ }
}
code.append(ReflectionLessCodeGenerator.getIndent(2))
.append("return result;")
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 89dc0dcdb6..b12fac47f6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -180,6 +180,12 @@
Fix parsing of client certificates that specify more than one OCSP
responder for configurations that use OpenSSL-FFM. (markt)
</fix>
+ <fix>
+ xreflection generated code stack overflow issue. (remm)
+ </fix>
+ <fix>
+ Align xreflection better with IntrospectionUtils. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
