Skip to content

Commit 3336226

Browse files
committed
WW-5352 First draft implementation
1 parent 5594738 commit 3336226

File tree

1 file changed

+140
-9
lines changed

1 file changed

+140
-9
lines changed

core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java

Lines changed: 140 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.opensymphony.xwork2.interceptor.MethodFilterInterceptor;
2626
import com.opensymphony.xwork2.interceptor.ValidationAware;
2727
import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
28+
import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker;
2829
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
2930
import com.opensymphony.xwork2.util.ClearableValueStack;
3031
import com.opensymphony.xwork2.util.MemberAccessValueStack;
@@ -43,16 +44,26 @@
4344
import org.apache.struts2.dispatcher.Parameter;
4445
import org.apache.struts2.ognl.ThreadAllowlist;
4546

47+
import java.beans.BeanInfo;
48+
import java.beans.IntrospectionException;
49+
import java.beans.Introspector;
50+
import java.beans.PropertyDescriptor;
51+
import java.lang.reflect.AnnotatedElement;
52+
import java.lang.reflect.Field;
53+
import java.lang.reflect.Modifier;
54+
import java.util.Arrays;
4655
import java.util.Collection;
4756
import java.util.Comparator;
4857
import java.util.HashSet;
4958
import java.util.Map;
59+
import java.util.Optional;
5060
import java.util.Set;
5161
import java.util.TreeMap;
5262
import java.util.regex.Pattern;
5363

5464
import static java.util.Collections.unmodifiableSet;
5565
import static java.util.stream.Collectors.joining;
66+
import static org.apache.commons.lang3.StringUtils.indexOfAny;
5667
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
5768

5869
/**
@@ -196,13 +207,19 @@ protected void addParametersToContext(ActionContext ac, Map<String, ?> newParams
196207
}
197208

198209
protected void setParameters(final Object action, ValueStack stack, HttpParameters parameters) {
199-
Map<String, Parameter> acceptableParameters = toAcceptableParameters(parameters, action);
210+
Map<String, Parameter> acceptableParameters;
211+
ValueStack newStack;
212+
try {
213+
acceptableParameters = toAcceptableParameters(parameters, action); // Side-effect: Allowlist required types
200214

201-
ValueStack newStack = toNewStack(stack);
202-
batchSetReflectionContextState(newStack.getContext(), true);
203-
setMemberAccessProperties(newStack);
215+
newStack = toNewStack(stack);
216+
batchSetReflectionContextState(newStack.getContext(), true);
217+
setMemberAccessProperties(newStack);
204218

205-
setParametersOnStack(newStack, acceptableParameters, action);
219+
setParametersOnStack(newStack, acceptableParameters, action);
220+
} finally {
221+
threadAllowlist.clearAllowlist();
222+
}
206223

207224
if (newStack instanceof ClearableValueStack) {
208225
stack.getActionContext().withConversionErrors(newStack.getActionContext().getConversionErrors());
@@ -300,19 +317,133 @@ protected void notifyDeveloperParameterException(Object action, String property,
300317
* @return true if parameter is accepted
301318
*/
302319
protected boolean isAcceptableParameter(String name, Object action) {
303-
return acceptableName(name) && isAcceptableParameterNameAware(name, action) && isParameterAnnotated(name, action);
320+
return acceptableName(name) && isAcceptableParameterNameAware(name, action) && isParameterAnnotatedAndAllowlist(name, action);
304321
}
305322

306323
protected boolean isAcceptableParameterNameAware(String name, Object action) {
307324
return !(action instanceof ParameterNameAware) || ((ParameterNameAware) action).acceptableParameterName(name);
308325
}
309326

310-
protected boolean isParameterAnnotated(String name, Object action) {
327+
/**
328+
* Checks if the Action class member corresponding to a parameter is appropriately annotated with
329+
* {@link StrutsParameter} and OGNL allowlists any necessary classes.
330+
* <p>
331+
* Note that this logic relies on the use of {@link DefaultAcceptedPatternsChecker#ACCEPTED_PATTERNS} and may also
332+
* be adversely impacted by the use of custom OGNL property accessors.
333+
*/
334+
protected boolean isParameterAnnotatedAndAllowlist(String name, Object action) {
311335
if (!requireAnnotations) {
312336
return true;
313337
}
314-
// TODO: Implement
315-
return true;
338+
339+
int nestingIndex = indexOfAny(name, ".[");
340+
String rootProperty = nestingIndex == -1 ? name : name.substring(0, nestingIndex);
341+
long paramDepth = name.chars().filter(ch -> ch == '.' || ch == '[').count();
342+
343+
return hasValidAnnotatedMember(rootProperty, action, paramDepth);
344+
}
345+
346+
/**
347+
* Note that we check for a public field last or only if there is no valid, annotated property descriptor. This is
348+
* because this check is likely to fail more often than not, as the relative use of public fields is low - so we
349+
* save computation by checking this last.
350+
*/
351+
protected boolean hasValidAnnotatedMember(String rootProperty, Object action, long paramDepth) {
352+
BeanInfo beanInfo = getBeanInfo(action);
353+
if (beanInfo == null) {
354+
return hasValidAnnotatedField(action, rootProperty, paramDepth);
355+
}
356+
357+
Optional<PropertyDescriptor> propDescOpt = Arrays.stream(beanInfo.getPropertyDescriptors())
358+
.filter(desc -> desc.getName().equals(rootProperty)).findFirst();
359+
if (!propDescOpt.isPresent()) {
360+
return hasValidAnnotatedField(action, rootProperty, paramDepth);
361+
}
362+
363+
if (hasValidAnnotatedPropertyDescriptor(propDescOpt.get(), paramDepth)) {
364+
return true;
365+
}
366+
367+
return hasValidAnnotatedField(action, rootProperty, paramDepth);
368+
}
369+
370+
protected boolean hasValidAnnotatedPropertyDescriptor(PropertyDescriptor propDesc, long paramDepth) {
371+
Class<?> rootType = getValidAnnotatedPropertyDescriptorType(propDesc, paramDepth);
372+
if (rootType != null) {
373+
threadAllowlist.allowClass(rootType);
374+
return true;
375+
}
376+
return false;
377+
}
378+
379+
/**
380+
* @return getter return type or setter parameter type, if one corresponding to the <code>paramDepth</code> exists
381+
* with a valid annotation
382+
*/
383+
protected Class<?> getValidAnnotatedPropertyDescriptorType(PropertyDescriptor propDesc, long paramDepth) {
384+
StrutsParameter annotation = null;
385+
Class<?> returnType = null;
386+
if (paramDepth == 0) {
387+
if (propDesc.getWriteMethod() != null) {
388+
annotation = getParameterAnnotation(propDesc.getWriteMethod());
389+
returnType = propDesc.getWriteMethod().getParameterTypes()[0];
390+
}
391+
} else {
392+
if (propDesc.getReadMethod() != null) {
393+
annotation = getParameterAnnotation(propDesc.getReadMethod());
394+
returnType = propDesc.getReadMethod().getReturnType();
395+
}
396+
}
397+
if (annotation != null && annotation.depth() >= paramDepth) {
398+
return returnType;
399+
}
400+
return null;
401+
}
402+
403+
protected boolean hasValidAnnotatedField(Object action, String fieldName, long paramDepth) {
404+
Class<?> rootType = getValidAnnotatedFieldType(action, fieldName, paramDepth);
405+
if (rootType != null) {
406+
threadAllowlist.allowClass(rootType);
407+
return true;
408+
}
409+
return false;
410+
}
411+
412+
/**
413+
* @return field type if a public field exists on the action with a valid annotation
414+
*/
415+
protected Class<?> getValidAnnotatedFieldType(Object action, String fieldName, long paramDepth) {
416+
Field field;
417+
try {
418+
field = action.getClass().getDeclaredField(fieldName);
419+
} catch (NoSuchFieldException e) {
420+
return null;
421+
}
422+
if (!Modifier.isPublic(field.getModifiers())) {
423+
return null;
424+
}
425+
StrutsParameter annotation = getParameterAnnotation(field);
426+
if (annotation != null && annotation.depth() >= paramDepth) {
427+
return field.getType();
428+
}
429+
return null;
430+
}
431+
432+
/**
433+
* Annotation retrieval logic. Can be overridden to support extending annotations or some other form of annotation
434+
* inheritance.
435+
*/
436+
protected StrutsParameter getParameterAnnotation(AnnotatedElement element) {
437+
return element.getAnnotation(StrutsParameter.class);
438+
}
439+
440+
protected BeanInfo getBeanInfo(Object action) {
441+
try {
442+
return Introspector.getBeanInfo(action.getClass());
443+
} catch (IntrospectionException e) {
444+
LOG.warn("Error introspecting Action {} for parameter injection validation", action.getClass(), e);
445+
return null;
446+
}
316447
}
317448

318449
/**

0 commit comments

Comments
 (0)