[
https://issues.apache.org/jira/browse/WW-5352?focusedWorklogId=900350&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-900350
]
ASF GitHub Bot logged work on WW-5352:
--------------------------------------
Author: ASF GitHub Bot
Created on: 18/Jan/24 09:05
Start Date: 18/Jan/24 09:05
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #832:
URL: https://github.com/apache/struts/pull/832#discussion_r1457131807
##########
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:
Doesn't look that utility method provides any additional value here -
`#getParameterAnnotation` exists purely for overriding purposes. Applications
can extend this default interceptor and add support for their own annotations
or so on.
Issue Time Tracking
-------------------
Worklog Id: (was: 900350)
Time Spent: 7h 20m (was: 7h 10m)
> Implement annotation mechanism for injectable fields via parameters
> -------------------------------------------------------------------
>
> Key: WW-5352
> URL: https://issues.apache.org/jira/browse/WW-5352
> Project: Struts 2
> Issue Type: Improvement
> Components: Core, Core Interceptors
> Reporter: Kusal Kithul-Godage
> Priority: Minor
> Fix For: 6.4.0
>
> Time Spent: 7h 20m
> Remaining Estimate: 0h
>
> struts.parameters.requireAnnotations
>
> Require an explicit annotation '@StrutsParameter' on one of:
> Getter/Setter/Field/ReturnType for injecting parameters.
>
> This mechanism is intended to be a more usable replacement for
> 'ParameterNameAware'
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
