lukaszlenart commented on code in PR #832:
URL: https://github.com/apache/struts/pull/832#discussion_r1451950824
##########
core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java:
##########
@@ -295,13 +340,150 @@ protected void notifyDeveloperParameterException(Object
action, String property,
* @return true if parameter is accepted
*/
protected boolean isAcceptableParameter(String name, Object action) {
- return acceptableName(name) && isAcceptableParameterNameAware(name,
action);
+ return acceptableName(name) && isAcceptableParameterNameAware(name,
action) && isParameterAnnotatedAndAllowlist(name, action);
}
protected boolean isAcceptableParameterNameAware(String name, Object
action) {
return !(action instanceof ParameterNameAware) ||
((ParameterNameAware) action).acceptableParameterName(name);
}
+ /**
+ * Checks if the Action class member corresponding to a parameter is
appropriately annotated with
+ * {@link StrutsParameter} and OGNL allowlists any necessary classes.
+ * <p>
+ * Note that this logic relies on the use of {@link
DefaultAcceptedPatternsChecker#NESTING_CHARS} and may also
+ * be adversely impacted by the use of custom OGNL property accessors.
+ */
+ protected boolean isParameterAnnotatedAndAllowlist(String name, Object
action) {
+ if (!requireAnnotations) {
+ return true;
+ }
+
+ long paramDepth = name.codePoints().mapToObj(c -> (char)
c).filter(NESTING_CHARS::contains).count();
+ if (requireAnnotationsTransitionMode && paramDepth == 0) {
+ return true;
+ }
+
+ int nestingIndex = indexOfAny(name, NESTING_CHARS_STR);
+ String rootProperty = nestingIndex == -1 ? name : name.substring(0,
nestingIndex);
+
+ return hasValidAnnotatedMember(rootProperty, action, paramDepth);
+ }
+
+ /**
+ * Note that we check for a public field last or only if there is no
valid, annotated property descriptor. This is
+ * because this check is likely to fail more often than not, as the
relative use of public fields is low - so we
+ * save computation by checking this last.
+ */
+ protected boolean hasValidAnnotatedMember(String rootProperty, Object
action, long paramDepth) {
+ BeanInfo beanInfo = getBeanInfo(action);
+ if (beanInfo == null) {
+ return hasValidAnnotatedField(action, rootProperty, paramDepth);
+ }
+
+ Optional<PropertyDescriptor> propDescOpt =
Arrays.stream(beanInfo.getPropertyDescriptors())
+ .filter(desc ->
desc.getName().equals(rootProperty)).findFirst();
+ if (!propDescOpt.isPresent()) {
+ return hasValidAnnotatedField(action, rootProperty, paramDepth);
+ }
+
+ if (hasValidAnnotatedPropertyDescriptor(propDescOpt.get(),
paramDepth)) {
+ return true;
+ }
+
+ return hasValidAnnotatedField(action, rootProperty, paramDepth);
+ }
+
+ protected boolean hasValidAnnotatedPropertyDescriptor(PropertyDescriptor
propDesc, long paramDepth) {
+ Method relevantMethod = paramDepth == 0 ? propDesc.getWriteMethod() :
propDesc.getReadMethod();
+ if (relevantMethod == null) {
+ return false;
+ }
+ StrutsParameter annotation = getParameterAnnotation(relevantMethod);
+ if (annotation == null || annotation.depth() < paramDepth) {
+ return false;
+ }
+ if (paramDepth >= 1) {
+ allowlistClass(relevantMethod.getReturnType());
+ }
+ if (paramDepth >= 2) {
+ allowlistReturnTypeIfParameterized(relevantMethod);
+ }
+ return true;
+ }
+
+ protected void allowlistReturnTypeIfParameterized(Method method) {
+ allowlistParameterizedTypeArg(method.getGenericReturnType());
+ }
+
+ protected void allowlistParameterizedTypeArg(Type genericType) {
+ if (!(genericType instanceof ParameterizedType)) {
+ return;
+ }
+ Type[] paramTypes = ((ParameterizedType)
genericType).getActualTypeArguments();
+ allowlistParamType(paramTypes[0]);
+ if (paramTypes.length > 1) {
+ // Probably useful for Map or Map-like classes
+ allowlistParamType(paramTypes[1]);
+ }
+ }
+
+ protected void allowlistParamType(Type paramType) {
+ if (paramType instanceof Class) {
+ allowlistClass((Class<?>) paramType);
+ }
+ }
+
+ protected void allowlistClass(Class<?> clazz) {
+ threadAllowlist.allowClass(clazz);
+
ClassUtils.getAllSuperclasses(clazz).forEach(threadAllowlist::allowClass);
+
ClassUtils.getAllInterfaces(clazz).forEach(threadAllowlist::allowClass);
+ }
+
+ protected boolean hasValidAnnotatedField(Object action, String fieldName,
long paramDepth) {
+ Field field;
+ try {
+ field = action.getClass().getDeclaredField(fieldName);
+ } catch (NoSuchFieldException e) {
+ return false;
+ }
+ if (!Modifier.isPublic(field.getModifiers())) {
+ return false;
+ }
+ StrutsParameter annotation = getParameterAnnotation(field);
+ if (annotation == null || annotation.depth() < paramDepth) {
+ return false;
+ }
+ if (paramDepth >= 1) {
+ allowlistClass(field.getType());
+ }
+ if (paramDepth >= 2) {
+ allowlistFieldIfParameterized(field);
+ }
+ return true;
+ }
+
+ protected void allowlistFieldIfParameterized(Field field) {
+ allowlistParameterizedTypeArg(field.getGenericType());
+ }
+
+ /**
+ * Annotation retrieval logic. Can be overridden to support extending
annotations or some other form of annotation
+ * inheritance.
+ */
+ protected StrutsParameter getParameterAnnotation(AnnotatedElement element)
{
+ return element.getAnnotation(StrutsParameter.class);
Review Comment:
There is `AnnotationUtils#isAnnotatedBy()` - wouldn't be better to use it
here or instead of `getParameterAnnotation()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
