This service extracts the authorization logic from {@link ParametersInterceptor} so that it can be reused by other + * input channels (e.g. JSON plugin, REST plugin) that also need to enforce {@code @StrutsParameter} rules.
+ * + *Implementations must NOT perform OGNL ThreadAllowlist side effects — those remain specific to + * {@link ParametersInterceptor}.
+ * + * @since 7.2.0 + */ +public interface ParameterAuthorizer { + + /** + * Determines whether a parameter with the given name is authorized for injection into the given target object. + * + *When {@code struts.parameters.requireAnnotations} is {@code false}, this method always returns {@code true} + * for backward compatibility.
+ * + * @param parameterName the parameter name (e.g. "name", "address.city", "items[0].name") + * @param target the object receiving the parameter value (the action, or the model for ModelDriven actions) + * @param action the action instance; used to detect ModelDriven exemption (when {@code target != action}, + * the target is the model and is exempt from annotation requirements) + * @return {@code true} if the parameter is authorized for injection, {@code false} otherwise + */ + boolean isAuthorized(String parameterName, Object target, Object action); +} diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index 293f4968a9..6173a85c65 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -100,6 +100,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { private AcceptedPatternsChecker acceptedPatterns; private Set+ * Authorization is delegated to {@link ParameterAuthorizer}. If authorized, OGNL allowlisting is performed as a + * second pass (this is specific to the OGNL-based parameter injection path and not shared with other input channels). + *
* 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.
*/
@@ -360,23 +369,67 @@ protected boolean isParameterAnnotatedAndAllowlist(String name, Object action) {
return true;
}
- long paramDepth = name.codePoints().mapToObj(c -> (char) c).filter(NESTING_CHARS::contains).count();
+ // Resolve target for ModelDriven: if the ValueStack peek is different from the action, it's the model
+ Object target = action;
+ if (action instanceof ModelDriven) {
+ Object stackTop = ActionContext.getContext().getValueStack().peek();
+ if (!stackTop.equals(action)) {
+ target = stackTop;
+ }
+ }
- if (action instanceof ModelDriven && !ActionContext.getContext().getValueStack().peek().equals(action)) {
- LOG.debug("Model driven Action detected, exempting from @StrutsParameter annotation requirement");
- return true;
+ // Delegate authorization check to shared ParameterAuthorizer (no OGNL side effects)
+ if (!parameterAuthorizer.isAuthorized(name, target, action)) {
+ return false;
}
- if (requireAnnotationsTransitionMode && paramDepth == 0) {
- LOG.debug("Annotation transition mode enabled, exempting non-nested parameter [{}] from @StrutsParameter annotation requirement", name);
- return true;
+ // OGNL-specific allowlisting: only needed for nested params (depth >= 1)
+ long paramDepth = name.codePoints().mapToObj(c -> (char) c).filter(NESTING_CHARS::contains).count();
+ if (paramDepth >= 1) {
+ performOgnlAllowlisting(name, target, paramDepth);
}
+ return true;
+ }
+ /**
+ * Performs OGNL ThreadAllowlist side effects for an authorized parameter. This is specific to OGNL-based parameter
+ * injection and must NOT be shared with other input channels (JSON, REST).
+ */
+ private void performOgnlAllowlisting(String name, Object target, long paramDepth) {
int nestingIndex = indexOfAny(name, NESTING_CHARS_STR);
String rootProperty = nestingIndex == -1 ? name : name.substring(0, nestingIndex);
String normalisedRootProperty = Character.toLowerCase(rootProperty.charAt(0)) + rootProperty.substring(1);
- return hasValidAnnotatedMember(normalisedRootProperty, action, paramDepth);
+ BeanInfo beanInfo = getBeanInfo(target);
+ if (beanInfo != null) {
+ Optional This implementation extracts the authorization logic from {@link ParametersInterceptor} so that it can be shared
+ * with other input channels (JSON plugin, REST plugin) without duplicating code. Unlike {@link ParametersInterceptor}, this implementation does NOT perform OGNL ThreadAllowlist side effects.
+ * Those remain specific to the OGNL-based parameter injection path.
+ * When {@code struts.parameters.requireAnnotations} is enabled, only properties annotated with
+ * {@link org.apache.struts2.interceptor.parameter.StrutsParameter} will be populated from the request body,
+ * consistent with the parameter authorization enforced by
+ * {@link org.apache.struts2.interceptor.parameter.ParametersInterceptor} for form/query parameters.
*/
public class ContentTypeInterceptor extends AbstractInterceptor {
+ private static final Logger LOG = LogManager.getLogger(ContentTypeInterceptor.class);
+
private ContentTypeHandlerManager selector;
+ private ParameterAuthorizer parameterAuthorizer;
+ private boolean requireAnnotations = false;
@Inject
public void setContentTypeHandlerSelector(ContentTypeHandlerManager selector) {
this.selector = selector;
}
+ @Inject
+ public void setParameterAuthorizer(ParameterAuthorizer parameterAuthorizer) {
+ this.parameterAuthorizer = parameterAuthorizer;
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS, required = false)
+ public void setRequireAnnotations(String requireAnnotations) {
+ this.requireAnnotations = BooleanUtils.toBoolean(requireAnnotations);
+ }
+
public String intercept(ActionInvocation invocation) throws Exception {
HttpServletRequest request = ServletActionContext.getRequest();
ContentTypeHandler handler = selector.getHandlerForRequest(request);
@@ -54,9 +94,252 @@ public String intercept(ActionInvocation invocation) throws Exception {
final String encoding = request.getCharacterEncoding();
InputStream is = request.getInputStream();
InputStreamReader reader = encoding == null ? new InputStreamReader(is) : new InputStreamReader(is, encoding);
- handler.toObject(invocation, reader, target);
+
+ if (requireAnnotations) {
+ // Two-phase deserialization: deserialize into a fresh instance, then copy only authorized properties.
+ // Requires a public no-arg constructor on the target class.
+ // If absent, body processing is rejected entirely — a best-effort scrub cannot guarantee
+ // that every nested unauthorized property is nulled out, so the safer choice is to skip.
+ Object freshInstance = createFreshInstance(target.getClass());
+ if (freshInstance != null) {
+ handler.toObject(invocation, reader, freshInstance);
+ copyAuthorizedProperties(freshInstance, target, invocation.getAction(), target, "");
+ } else {
+ LOG.warn("REST body rejected: requireAnnotations=true but [{}] has no no-arg constructor; "
+ + "body deserialization skipped to preserve @StrutsParameter authorization integrity",
+ target.getClass().getName());
+ }
+ } else {
+ // Direct deserialization (backward compat when requireAnnotations is not enabled)
+ handler.toObject(invocation, reader, target);
+ }
}
return invocation.invoke();
}
+ private Object createFreshInstance(Class clazz) {
+ try {
+ return clazz.getDeclaredConstructor().newInstance();
+ } catch (ReflectiveOperationException e) {
+ LOG.debug("Cannot create fresh instance of [{}] via no-arg constructor: {}", clazz.getName(), e.getMessage());
+ return null;
+ }
+ }
+
+ /**
+ * Recursively copies only authorized properties from {@code source} to {@code target},
+ * enforcing {@code @StrutsParameter} depth semantics for nested object graphs.
+ *
+ * {@code authTarget} is always the root action/model passed unchanged through all levels.
+ * {@code isAuthorized} uses the full dot/bracket path against the root class, so the root
+ * target must be used — not the nested object being visited at the current recursion depth.
+ */
+ private void copyAuthorizedProperties(
+ Object source, Object target, Object action, Object authTarget, String prefix) throws Exception {
+ BeanInfo beanInfo = Introspector.getBeanInfo(source.getClass(), Object.class);
+ for (PropertyDescriptor pd : beanInfo.getPropertyDescriptors()) {
+ Method readMethod = pd.getReadMethod();
+ Method writeMethod = pd.getWriteMethod();
+ if (readMethod == null || writeMethod == null) {
+ continue;
+ }
+
+ String fullPath = prefix.isEmpty() ? pd.getName() : prefix + "." + pd.getName();
+
+ // Always check against authTarget (root action/model), never the nested object being traversed
+ if (!parameterAuthorizer.isAuthorized(fullPath, authTarget, action)) {
+ LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization on [{}]",
+ fullPath, authTarget.getClass().getName());
+ continue;
+ }
+
+ Object sourceValue = readMethod.invoke(source);
+ if (sourceValue == null) {
+ // Intentionally skip null values: in two-phase deserialization, properties NOT present in the
+ // request body will be null in the fresh instance. Copying null would clear pre-initialized
+ // fields on the target. This is the safer default — an explicit JSON null and a missing field
+ // are indistinguishable after deserialization into a fresh POJO.
+ continue;
+ }
+
+ if (isNestedBeanType(sourceValue.getClass())) {
+ // Complex bean: recurse to authorize nested fields, passing authTarget unchanged
+ Object targetValue = readMethod.invoke(target);
+ if (targetValue == null) {
+ Object newTarget = createFreshInstance(sourceValue.getClass());
+ if (newTarget != null) {
+ writeMethod.invoke(target, newTarget);
+ targetValue = newTarget;
+ } else {
+ // No no-arg constructor for the nested bean: skip rather than bulk-copy the
+ // unfiltered source value, which would bypass per-path authorization for every
+ // property underneath this node.
+ LOG.warn("REST nested bean [{}] skipped — no no-arg constructor for [{}],"
+ + " cannot authorize its nested properties",
+ fullPath, sourceValue.getClass().getName());
+ continue;
+ }
+ }
+ copyAuthorizedProperties(sourceValue, targetValue, action, authTarget, fullPath);
+ } else if (sourceValue instanceof Collection) {
+ writeMethod.invoke(target,
+ deepCopyAuthorizedCollection((Collection) sourceValue, fullPath, authTarget, action));
+ } else if (sourceValue instanceof Map) {
+ writeMethod.invoke(target,
+ deepCopyAuthorizedMap((Map) sourceValue, fullPath, authTarget, action));
+ } else if (sourceValue.getClass().isArray()) {
+ writeMethod.invoke(target,
+ deepCopyAuthorizedArray(sourceValue, fullPath, authTarget, action));
+ } else {
+ writeMethod.invoke(target, sourceValue);
+ }
+ }
+ }
+
+ /**
+ * Authorizes each complex element of a collection using indexed-path semantics ({@code path[0].field}),
+ * matching {@code ParametersInterceptor} depth counting. Scalar elements are copied directly.
+ * Elements whose class has no no-arg constructor are skipped to avoid copying an unfiltered object graph.
+ */
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ private Collection deepCopyAuthorizedCollection(
+ Collection source, String collectionPath, Object authTarget, Object action) throws Exception {
+ // Preserve the collection type so that writeMethod.invoke does not fail when the setter
+ // parameter is typed as Set, SortedSet, etc. Fall back to ArrayList for unrecognised types.
+ Collection result;
+ if (source instanceof SortedSet) {
+ result = new TreeSet(((SortedSet) source).comparator());
+ } else if (source instanceof Set) {
+ result = new LinkedHashSet();
+ } else {
+ result = new ArrayList();
+ }
+ for (Object element : source) {
+ if (element != null && isNestedBeanType(element.getClass())) {
+ String elementPath = collectionPath + "[0]";
+ if (!parameterAuthorizer.isAuthorized(elementPath, authTarget, action)) {
+ LOG.warn("REST collection element [{}] rejected by @StrutsParameter authorization", elementPath);
+ continue;
+ }
+ Object newElement = createFreshInstance(element.getClass());
+ if (newElement != null) {
+ copyAuthorizedProperties(element, newElement, action, authTarget, elementPath);
+ result.add(newElement);
+ } else {
+ // No no-arg constructor: skip element rather than copy an unfiltered object graph
+ LOG.warn("REST collection element [{}] skipped — no no-arg constructor for [{}]",
+ elementPath, element.getClass().getName());
+ }
+ } else {
+ result.add(element);
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Authorizes each complex map value using indexed-path semantics ({@code path[0]}),
+ * consistent with OGNL bracket notation depth counting. Scalar values are copied directly.
+ */
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ private Map deepCopyAuthorizedMap(
+ Map source, String mapPath, Object authTarget, Object action) throws Exception {
+ // Preserve the map type so that writeMethod.invoke does not fail when the setter
+ // parameter is typed as SortedMap, TreeMap, etc.
+ Map result;
+ if (source instanceof SortedMap) {
+ result = new TreeMap(((SortedMap) source).comparator());
+ } else {
+ result = new LinkedHashMap();
+ }
+ for (Map.Entry entry : source.entrySet()) {
+ Object value = entry.getValue();
+ if (value != null && isNestedBeanType(value.getClass())) {
+ String valuePath = mapPath + "[0]";
+ if (!parameterAuthorizer.isAuthorized(valuePath, authTarget, action)) {
+ LOG.warn("REST map value [{}] rejected by @StrutsParameter authorization", valuePath);
+ continue;
+ }
+ Object newValue = createFreshInstance(value.getClass());
+ if (newValue != null) {
+ copyAuthorizedProperties(value, newValue, action, authTarget, valuePath);
+ result.put(entry.getKey(), newValue);
+ } else {
+ LOG.warn("REST map value [{}] skipped — no no-arg constructor for [{}]",
+ valuePath, value.getClass().getName());
+ }
+ } else {
+ result.put(entry.getKey(), value);
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Authorizes each complex element of an array ({@code Pojo[]}) using indexed-path semantics,
+ * matching {@code ParametersInterceptor} depth counting. Scalar elements are copied directly.
+ */
+ private Object deepCopyAuthorizedArray(
+ Object sourceArray, String arrayPath, Object authTarget, Object action) throws Exception {
+ int length = Array.getLength(sourceArray);
+ Class componentType = sourceArray.getClass().getComponentType();
+ Object result = Array.newInstance(componentType, length);
+ for (int i = 0; i < length; i++) {
+ Object element = Array.get(sourceArray, i);
+ if (element != null && isNestedBeanType(element.getClass())) {
+ String elementPath = arrayPath + "[0]";
+ if (!parameterAuthorizer.isAuthorized(elementPath, authTarget, action)) {
+ LOG.warn("REST array element [{}] rejected by @StrutsParameter authorization", elementPath);
+ continue;
+ }
+ Object newElement = createFreshInstance(element.getClass());
+ if (newElement != null) {
+ copyAuthorizedProperties(element, newElement, action, authTarget, elementPath);
+ Array.set(result, i, newElement);
+ } else {
+ LOG.warn("REST array element [{}] skipped — no no-arg constructor for [{}]",
+ elementPath, element.getClass().getName());
+ }
+ } else {
+ Array.set(result, i, element);
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Determines whether a class represents a nested bean that should be recursively authorized,
+ * as opposed to simple/leaf types (primitives, strings, collections, maps, arrays, enums) that
+ * are handled directly.
+ */
+ private boolean isNestedBeanType(Class clazz) {
+ if (clazz.isPrimitive() || clazz.isEnum() || clazz.isArray()) {
+ return false;
+ }
+ // Exclude standard library value/leaf types that have no meaningful bean properties to recurse into.
+ // java.lang.*, java.math.* — primitives, String, Number subclasses, etc.
+ // java.util.* leaf types — UUID, Currency, Locale, Date, etc. (NOT Collection/Map which are handled separately)
+ if (clazz.getName().startsWith("java.lang.") || clazz.getName().startsWith("java.math.")) {
+ return false;
+ }
+ if (clazz.getName().startsWith("java.util.") && !Collection.class.isAssignableFrom(clazz)
+ && !Map.class.isAssignableFrom(clazz)) {
+ return false;
+ }
+ if (java.time.temporal.Temporal.class.isAssignableFrom(clazz)) {
+ return false;
+ }
+ if (clazz.getName().startsWith("java.time.")) {
+ return false;
+ }
+ if (clazz.getName().startsWith("java.net.") || clazz.getName().startsWith("java.io.")
+ || clazz.getName().startsWith("java.nio.")) {
+ return false;
+ }
+ if (Collection.class.isAssignableFrom(clazz) || Map.class.isAssignableFrom(clazz)) {
+ return false;
+ }
+ return true;
+ }
+
}
diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java
index 2232ccb94c..a14d407eef 100644
--- a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java
+++ b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorTest.java
@@ -32,12 +32,14 @@
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import org.apache.struts2.rest.handler.ContentTypeHandler;
+import org.apache.struts2.interceptor.parameter.ParameterAuthorizer;
import org.springframework.mock.web.MockHttpServletRequest;
public class ContentTypeInterceptorTest extends TestCase {
public void testRequestWithoutEncoding() throws Exception {
ContentTypeInterceptor interceptor = new ContentTypeInterceptor();
+ interceptor.setParameterAuthorizer((parameterName, target, action) -> true);
ActionSupport action = new ActionSupport();
@@ -76,6 +78,7 @@ public void testRequestWithEncodingAscii() throws Exception {
final Charset charset = StandardCharsets.US_ASCII;
ContentTypeInterceptor interceptor = new ContentTypeInterceptor();
+ interceptor.setParameterAuthorizer((parameterName, target, action) -> true);
ActionSupport action = new ActionSupport();
@@ -116,6 +119,7 @@ public void testRequestWithEncodingUtf() throws Exception {
final Charset charset = StandardCharsets.UTF_8;
ContentTypeInterceptor interceptor = new ContentTypeInterceptor();
+ interceptor.setParameterAuthorizer((parameterName, target, action) -> true);
ActionSupport action = new ActionSupport();
@@ -151,4 +155,82 @@ public boolean matches(Object[] args) {
mockActionInvocation.verify();
mockContentTypeHandler.verify();
}
+
+ public void testRequireAnnotationsEnabled_twoPhaseDeserialization() throws Exception {
+ ContentTypeInterceptor interceptor = new ContentTypeInterceptor();
+ interceptor.setParameterAuthorizer((parameterName, target, action) -> false);
+ interceptor.setRequireAnnotations(Boolean.TRUE.toString());
+
+ ActionSupport action = new ActionSupport();
+
+ Mock mockActionInvocation = new Mock(ActionInvocation.class);
+ Mock mockContentTypeHandler = new Mock(ContentTypeHandler.class);
+ mockContentTypeHandler.expect("toObject", new AnyConstraintMatcher() {
+ public boolean matches(Object[] args) {
+ return true;
+ }
+ });
+ mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS);
+ mockActionInvocation.expectAndReturn("getAction", action);
+ mockActionInvocation.expectAndReturn("getAction", action);
+ Mock mockContentTypeHandlerManager = new Mock(ContentTypeHandlerManager.class);
+ mockContentTypeHandlerManager.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() {
+ public boolean matches(Object[] args) {
+ return true;
+ }
+ }, mockContentTypeHandler.proxy());
+ interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockContentTypeHandlerManager.proxy());
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setContent(new byte[] {1});
+
+ ActionContext.of()
+ .withActionMapping(new ActionMapping())
+ .withServletRequest(request)
+ .bind();
+
+ interceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+ mockContentTypeHandlerManager.verify();
+ mockActionInvocation.verify();
+ mockContentTypeHandler.verify();
+ }
+
+ public void testRequireAnnotationsEnabled_selectiveFilter() throws Exception {
+ ContentTypeInterceptor interceptor = new ContentTypeInterceptor();
+ interceptor.setParameterAuthorizer((parameterName, target, action) -> "name".equals(parameterName));
+ interceptor.setRequireAnnotations(Boolean.TRUE.toString());
+
+ ActionSupport action = new ActionSupport();
+
+ Mock mockActionInvocation = new Mock(ActionInvocation.class);
+ Mock mockContentTypeHandler = new Mock(ContentTypeHandler.class);
+ mockContentTypeHandler.expect("toObject", new AnyConstraintMatcher() {
+ public boolean matches(Object[] args) {
+ return true;
+ }
+ });
+ mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS);
+ mockActionInvocation.expectAndReturn("getAction", action);
+ mockActionInvocation.expectAndReturn("getAction", action);
+ Mock mockContentTypeHandlerManager = new Mock(ContentTypeHandlerManager.class);
+ mockContentTypeHandlerManager.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() {
+ public boolean matches(Object[] args) {
+ return true;
+ }
+ }, mockContentTypeHandler.proxy());
+ interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockContentTypeHandlerManager.proxy());
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setContent(new byte[] {1});
+
+ ActionContext.of()
+ .withActionMapping(new ActionMapping())
+ .withServletRequest(request)
+ .bind();
+
+ interceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+ mockContentTypeHandlerManager.verify();
+ mockActionInvocation.verify();
+ mockContentTypeHandler.verify();
+ }
}