This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new e83487f0e WW-5627 Gate CookieInterceptor through ParameterAuthorizer
(#1681)
e83487f0e is described below
commit e83487f0e19f4000cf46a88fc6f06bd549ad9d7b
Author: Lukasz Lenart <[email protected]>
AuthorDate: Mon May 18 13:52:17 2026 +0200
WW-5627 Gate CookieInterceptor through ParameterAuthorizer (#1681)
* WW-5627 add ParameterAllowlister interface and
STRUTS_PARAMETER_ALLOWLISTER constant
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* WW-5627 add OgnlParameterAllowlister default implementation
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* WW-5627 register ParameterAllowlister bean in struts-default DI
* WW-5627 delegate ParametersInterceptor OGNL allowlisting to
OgnlParameterAllowlister
Also register ParameterAllowlister in DefaultConfiguration bootstrap
factories so it is available in test containers (parallel to how
ParameterAuthorizer was already registered there).
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* WW-5627 test(cookie): failing test for unannotated setter skip
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* WW-5627 gate CookieInterceptor cookie injection through
ParameterAuthorizer
Adds a 5-arg `populateCookieValueIntoStack(name, value, map, stack,
action)` hook
that runs cookie writes through `ParameterAuthorizer.isAuthorized` and
primes
`ThreadAllowlist` via `ParameterAllowlister` for nested paths, then
delegates
to the legacy 4-arg form. The 4-arg form is `@Deprecated(since="7.2.0")` but
its body is unchanged, so existing subclass overrides automatically receive
only authorized cookies. Default-config behavior is preserved because the
authorizer short-circuits when `requireAnnotations=false`.
Existing `CookieInterceptorTest` instantiates `new CookieInterceptor()`
rather
than going through the container, leaving the new injected fields null.
Wires
explicit pass-through lambdas through a `disableAuthorizationGate(...)`
helper
so those tests continue to exercise default-config behavior.
* WW-5627 cover CookieInterceptor authorization matrix in
CookieInterceptorAnnotationTest
* WW-5627 docs(cookie): document new 5-arg extension hook and deprecation
* WW-5627 wire OgnlParameterAllowlister in StrutsParameterAnnotationTest
fixture
* WW-5627 address SonarCloud findings on PR #1681
- S1948: mark transient on the new ParameterAuthorizer/ParameterAllowlister
fields in CookieInterceptor and ParametersInterceptor (the host classes
are Serializable; the injected services are not).
- S1874: suppress the deprecation warning on the new 5-arg
populateCookieValueIntoStack — the delegation to the deprecated 4-arg
form is the contract that lets existing subclass overrides participate.
- S3776: extract `allowlistViaPropertyDescriptor` and
`allowlistViaPublicField` from
`OgnlParameterAllowlister.allowlistAuthorizedPath`
to drop cognitive complexity below the threshold.
- S1068: remove the unused `mapping` test fixture field.
* WW-5627 clarify ParameterAllowlister contract and tidy
ParametersInterceptor
Rename `ParameterAllowlister#allowlistAuthorizedPath` to
`primeAllowlistForPath`
to make the contract explicit: the SAM is a side-effect-only priming hook
that
runs after `ParameterAuthorizer#isAuthorized` has already decided. A no-op
return means "no priming needed or possible", never "rejected". The
interface
name stays channel-agnostic; only the impl class
(`OgnlParameterAllowlister`)
binds the priming to OGNL's `ThreadAllowlist`.
Add a `LOG.debug` in `OgnlParameterAllowlister` for the case where
authorization
passed but no `@StrutsParameter` could be located on the root property
(e.g. `ModelDriven` models without per-property annotations) so the
authorize-vs-prime gap is observable instead of surfacing later as an opaque
OGNL traversal failure.
Drop the dead `performOgnlAllowlisting` pass-through and its unused
`paramDepth`
parameter from `ParametersInterceptor` — the depth check is already enforced
inside `OgnlParameterAllowlister.primeAllowlistForPath`, so the outer guard
was
a redundant computation.
No behavior change.
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
---
.../java/org/apache/struts2/StrutsConstants.java | 8 +
.../config/StrutsBeanSelectionProvider.java | 2 +
.../struts2/config/impl/DefaultConfiguration.java | 3 +
.../struts2/interceptor/CookieInterceptor.java | 63 +++++-
.../parameter/OgnlParameterAllowlister.java | 192 ++++++++++++++++++
.../parameter/ParameterAllowlister.java | 44 +++++
.../parameter/ParametersInterceptor.java | 56 +-----
core/src/main/resources/struts-beans.xml | 3 +
.../CookieInterceptorAnnotationTest.java | 219 +++++++++++++++++++++
.../struts2/interceptor/CookieInterceptorTest.java | 18 ++
.../parameter/OgnlParameterAllowlisterTest.java | 154 +++++++++++++++
.../parameter/StrutsParameterAnnotationTest.java | 6 +
12 files changed, 714 insertions(+), 54 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index eb925b422..13fc2d3d2 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -558,6 +558,14 @@ public final class StrutsConstants {
*/
public static final String STRUTS_PARAMETER_AUTHORIZER =
"struts.parameterAuthorizer";
+ /**
+ * The {@link
org.apache.struts2.interceptor.parameter.ParameterAllowlister} implementation
class.
+ * Override to provide a custom allowlister for non-OGNL parameter targets.
+ *
+ * @since 7.2.0
+ */
+ public static final String STRUTS_PARAMETER_ALLOWLISTER =
"struts.parameterAllowlister";
+
/**
* Enables evaluation of OGNL expressions
*
diff --git
a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java
b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java
index f169b67f1..e3f632eda 100644
---
a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java
+++
b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java
@@ -73,6 +73,7 @@ import org.apache.struts2.url.UrlDecoder;
import org.apache.struts2.url.UrlEncoder;
import org.apache.struts2.util.ContentTypeMatcher;
import org.apache.struts2.util.PatternMatcher;
+import org.apache.struts2.interceptor.parameter.ParameterAllowlister;
import org.apache.struts2.interceptor.parameter.ParameterAuthorizer;
import org.apache.struts2.util.ProxyService;
import org.apache.struts2.util.TextParser;
@@ -448,6 +449,7 @@ public class StrutsBeanSelectionProvider extends
AbstractBeanSelectionProvider {
alias(ProxyCacheFactory.class,
StrutsConstants.STRUTS_PROXY_CACHE_FACTORY, builder, props, Scope.SINGLETON);
alias(ProxyService.class, StrutsConstants.STRUTS_PROXYSERVICE,
builder, props, Scope.SINGLETON);
alias(ParameterAuthorizer.class,
StrutsConstants.STRUTS_PARAMETER_AUTHORIZER, builder, props, Scope.SINGLETON);
+ alias(ParameterAllowlister.class,
StrutsConstants.STRUTS_PARAMETER_ALLOWLISTER, builder, props, Scope.SINGLETON);
alias(SecurityMemberAccess.class,
StrutsConstants.STRUTS_MEMBER_ACCESS, builder, props, Scope.PROTOTYPE);
alias(OgnlGuard.class, StrutsConstants.STRUTS_OGNL_GUARD, builder,
props, Scope.SINGLETON);
diff --git
a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
index 9eb009592..dcf4f1602 100644
---
a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
+++
b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
@@ -92,6 +92,8 @@ import org.apache.struts2.ognl.SecurityMemberAccess;
import org.apache.struts2.ognl.accessor.CompoundRootAccessor;
import org.apache.struts2.ognl.accessor.RootAccessor;
import org.apache.struts2.ognl.accessor.XWorkMethodAccessor;
+import org.apache.struts2.interceptor.parameter.OgnlParameterAllowlister;
+import org.apache.struts2.interceptor.parameter.ParameterAllowlister;
import org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer;
import org.apache.struts2.interceptor.parameter.ParameterAuthorizer;
import org.apache.struts2.util.StrutsProxyService;
@@ -409,6 +411,7 @@ public class DefaultConfiguration implements Configuration {
.factory(ProxyCacheFactory.class,
StrutsProxyCacheFactory.class, Scope.SINGLETON)
.factory(ProxyService.class, StrutsProxyService.class,
Scope.SINGLETON)
.factory(ParameterAuthorizer.class,
StrutsParameterAuthorizer.class, Scope.SINGLETON)
+ .factory(ParameterAllowlister.class,
OgnlParameterAllowlister.class, Scope.SINGLETON)
.factory(OgnlUtil.class, Scope.SINGLETON)
.factory(SecurityMemberAccess.class, Scope.PROTOTYPE)
.factory(OgnlGuard.class, StrutsOgnlGuard.class,
Scope.SINGLETON)
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java
b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java
index 6ca7ecb0a..4d9685543 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java
@@ -26,6 +26,8 @@ import org.apache.struts2.ActionInvocation;
import org.apache.struts2.ServletActionContext;
import org.apache.struts2.action.CookiesAware;
import org.apache.struts2.inject.Inject;
+import org.apache.struts2.interceptor.parameter.ParameterAllowlister;
+import org.apache.struts2.interceptor.parameter.ParameterAuthorizer;
import org.apache.struts2.security.AcceptedPatternsChecker;
import org.apache.struts2.security.ExcludedPatternsChecker;
import org.apache.struts2.util.TextParseUtil;
@@ -99,8 +101,16 @@ import java.util.Set;
*
* <ul>
* <li>
- * populateCookieValueIntoStack - this method will decide if this
cookie value is qualified
- * to be populated into the value stack (hence into the action itself)
+ * populateCookieValueIntoStack(name, value, map, stack, action) - the
preferred extension point
+ * since 7.2.0. The default implementation gates the cookie write
through
+ * {@link
org.apache.struts2.interceptor.parameter.ParameterAuthorizer} and primes the
OGNL allowlist via
+ * {@link
org.apache.struts2.interceptor.parameter.ParameterAllowlister} before
delegating to the legacy
+ * 4-arg {@code populateCookieValueIntoStack}. Override here to
customize the authorization behavior itself.
+ * </li>
+ * <li>
+ * populateCookieValueIntoStack(name, value, map, stack) -
<em>deprecated since 7.2.0</em>. The legacy
+ * hook that performs the actual {@code stack.setValue}. Existing
overrides continue to work and
+ * automatically receive only authorized cookies via the 5-arg default.
* </li>
* <li>
* injectIntoCookiesAwareAction - this method will inject selected
cookies (as a java.util.Map)
@@ -187,6 +197,8 @@ public class CookieInterceptor extends AbstractInterceptor {
private ExcludedPatternsChecker excludedPatternsChecker;
private AcceptedPatternsChecker acceptedPatternsChecker;
+ private transient ParameterAuthorizer parameterAuthorizer;
+ private transient ParameterAllowlister parameterAllowlister;
@Inject
public void setExcludedPatternsChecker(ExcludedPatternsChecker
excludedPatternsChecker) {
@@ -199,6 +211,16 @@ public class CookieInterceptor extends AbstractInterceptor
{
this.acceptedPatternsChecker.setAcceptedPatterns(ACCEPTED_PATTERN);
}
+ @Inject
+ public void setParameterAuthorizer(ParameterAuthorizer
parameterAuthorizer) {
+ this.parameterAuthorizer = parameterAuthorizer;
+ }
+
+ @Inject
+ public void setParameterAllowlister(ParameterAllowlister
parameterAllowlister) {
+ this.parameterAllowlister = parameterAllowlister;
+ }
+
/**
* @param cookiesName the <code>cookiesName</code> which if matched will
allow the cookie
* to be injected into action, could be comma-separated string.
@@ -234,6 +256,8 @@ public class CookieInterceptor extends AbstractInterceptor {
public String intercept(ActionInvocation invocation) throws Exception {
LOG.debug("start interception");
+ final Object action = invocation.getAction();
+
// contains selected cookies
final Map<String, String> cookiesMap = new LinkedHashMap<>();
@@ -248,9 +272,9 @@ public class CookieInterceptor extends AbstractInterceptor {
if (isAcceptableName(name)) {
if (cookiesNameSet.contains("*")) {
LOG.debug("Contains cookie name [*] in configured
cookies name set, cookie with name [{}] with value [{}] will be injected",
name, value);
- populateCookieValueIntoStack(name, value, cookiesMap,
stack);
+ populateCookieValueIntoStack(name, value, cookiesMap,
stack, action);
} else if (cookiesNameSet.contains(cookie.getName())) {
- populateCookieValueIntoStack(name, value, cookiesMap,
stack);
+ populateCookieValueIntoStack(name, value, cookiesMap,
stack, action);
}
} else {
LOG.warn("Cookie name [{}] with value [{}] was rejected!",
name, value);
@@ -259,7 +283,7 @@ public class CookieInterceptor extends AbstractInterceptor {
}
// inject the cookiesMap, even if we don't have any cookies
- injectIntoCookiesAwareAction(invocation.getAction(), cookiesMap);
+ injectIntoCookiesAwareAction(action, cookiesMap);
return invocation.invoke();
}
@@ -314,6 +338,30 @@ public class CookieInterceptor extends AbstractInterceptor
{
return false;
}
+ /**
+ * Authorizes the cookie against {@link ParameterAuthorizer}, primes OGNL
allowlist for any nested path via
+ * {@link ParameterAllowlister}, then delegates to the legacy {@link
#populateCookieValueIntoStack(String, String,
+ * Map, ValueStack)} hook so existing subclass overrides continue to
participate. Override this method to customize
+ * the authorization behavior itself.
+ *
+ * @param cookieName cookie name (potentially an OGNL path; {@code
ACCEPTED_PATTERN} restricts the character set)
+ * @param cookieValue cookie value
+ * @param cookiesMap map of cookies populated for {@link
org.apache.struts2.action.CookiesAware}
+ * @param stack current request value stack
+ * @param action the action instance from {@link
ActionInvocation#getAction()}; used for {@code @StrutsParameter} target
resolution
+ * @since 7.2.0
+ */
+ @SuppressWarnings("deprecation") // intentional: delegating to the
deprecated 4-arg form is the contract that lets existing subclass overrides
participate
+ protected void populateCookieValueIntoStack(String cookieName, String
cookieValue, Map<String, String> cookiesMap, ValueStack stack, Object action) {
+ Object target = parameterAuthorizer.resolveTarget(action);
+ if (!parameterAuthorizer.isAuthorized(cookieName, target, action)) {
+ LOG.debug("Cookie [{}] rejected by @StrutsParameter authorization
on target [{}]", cookieName, target.getClass().getSimpleName());
+ return;
+ }
+ parameterAllowlister.primeAllowlistForPath(cookieName, target);
+ populateCookieValueIntoStack(cookieName, cookieValue, cookiesMap,
stack);
+ }
+
/**
* Hook that populate cookie value into value stack (hence the action)
* if the criteria is satisfied (if the cookie value matches with those
configured).
@@ -322,7 +370,12 @@ public class CookieInterceptor extends AbstractInterceptor
{
* @param cookieValue cookie value
* @param cookiesMap map of cookies
* @param stack value stack
+ * @deprecated since 7.2.0. Override
+ * {@link #populateCookieValueIntoStack(String, String, Map, ValueStack,
Object)} instead so cookie writes are
+ * authorized by {@link ParameterAuthorizer}. The default 5-arg
implementation calls this method after the
+ * authorization gate, so existing overrides continue to receive only
authorized cookies.
*/
+ @Deprecated(since = "7.2.0")
protected void populateCookieValueIntoStack(String cookieName, String
cookieValue, Map<String, String> cookiesMap, ValueStack stack) {
if (cookiesValueSet.isEmpty() || cookiesValueSet.contains("*")) {
// If the interceptor is configured to accept any cookie value
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlister.java
b/core/src/main/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlister.java
new file mode 100644
index 000000000..0a24d8577
--- /dev/null
+++
b/core/src/main/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlister.java
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.interceptor.parameter;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.inject.Inject;
+import org.apache.struts2.ognl.OgnlUtil;
+import org.apache.struts2.ognl.ThreadAllowlist;
+import org.apache.struts2.util.ProxyService;
+
+import java.beans.BeanInfo;
+import java.beans.IntrospectionException;
+import java.beans.PropertyDescriptor;
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Arrays;
+import java.util.Optional;
+
+import static org.apache.commons.lang3.StringUtils.indexOfAny;
+import static
org.apache.struts2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS;
+import static
org.apache.struts2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS_STR;
+
+/**
+ * Default {@link ParameterAllowlister}. Registers the root property's class
(and generic type args for {@code depth >= 2})
+ * into the OGNL {@link ThreadAllowlist} so OGNL may introspect and traverse a
nested path on the value stack. Logic is
+ * extracted verbatim from {@code
ParametersInterceptor.performOgnlAllowlisting} so the OGNL parameter and cookie
+ * channels share a single implementation.
+ *
+ * <p>No-ops when:
+ * <ul>
+ * <li>{@code paramDepth == 0} — shallow setter; OGNL does not need to
traverse</li>
+ * <li>the root property has no {@code @StrutsParameter} annotation
reachable via {@link java.beans.PropertyDescriptor}
+ * or as a public field (e.g. a {@code ModelDriven} model whose
properties are not individually annotated). A
+ * {@code LOG.debug} surfaces this case so the gap between authorization
and OGNL traversal is observable.</li>
+ * </ul>
+ *
+ * @since 7.2.0
+ */
+public class OgnlParameterAllowlister implements ParameterAllowlister {
+
+ private static final Logger LOG =
LogManager.getLogger(OgnlParameterAllowlister.class);
+
+ private OgnlUtil ognlUtil;
+ private ProxyService proxyService;
+ private ThreadAllowlist threadAllowlist;
+
+ @Inject
+ public void setOgnlUtil(OgnlUtil ognlUtil) {
+ this.ognlUtil = ognlUtil;
+ }
+
+ @Inject
+ public void setProxyService(ProxyService proxyService) {
+ this.proxyService = proxyService;
+ }
+
+ @Inject
+ public void setThreadAllowlist(ThreadAllowlist threadAllowlist) {
+ this.threadAllowlist = threadAllowlist;
+ }
+
+ @Override
+ public void primeAllowlistForPath(String parameterName, Object target) {
+ if (parameterName == null || parameterName.isEmpty() || target ==
null) {
+ return;
+ }
+ long paramDepth = parameterName.codePoints().mapToObj(c -> (char)
c).filter(NESTING_CHARS::contains).count();
+ if (paramDepth == 0) {
+ return;
+ }
+
+ int nestingIndex = indexOfAny(parameterName, NESTING_CHARS_STR);
+ String rootProperty = nestingIndex == -1 ? parameterName :
parameterName.substring(0, nestingIndex);
+ String normalisedRootProperty =
Character.toLowerCase(rootProperty.charAt(0)) + rootProperty.substring(1);
+
+ if (allowlistViaPropertyDescriptor(target, normalisedRootProperty,
paramDepth)) {
+ return;
+ }
+ if (allowlistViaPublicField(target, normalisedRootProperty,
paramDepth)) {
+ return;
+ }
+ // Authorization passed but no @StrutsParameter on the root property —
e.g. ModelDriven model with no
+ // per-property annotations. OGNL won't be able to walk this nested
path; surface the gap in logs.
+ LOG.debug("Parameter [{}] authorized but no @StrutsParameter on root
property [{}] of [{}]; "
+ + "OGNL allowlist not primed and nested traversal may be
blocked",
+ parameterName, normalisedRootProperty,
ultimateClass(target).getSimpleName());
+ }
+
+ private boolean allowlistViaPropertyDescriptor(Object target, String
rootProperty, long paramDepth) {
+ BeanInfo beanInfo = getBeanInfo(target);
+ if (beanInfo == null) {
+ return false;
+ }
+ Optional<PropertyDescriptor> propDescOpt =
Arrays.stream(beanInfo.getPropertyDescriptors())
+ .filter(desc ->
desc.getName().equals(rootProperty)).findFirst();
+ if (propDescOpt.isEmpty()) {
+ return false;
+ }
+ PropertyDescriptor propDesc = propDescOpt.get();
+ Method relevantMethod = propDesc.getReadMethod();
+ if (relevantMethod == null ||
getPermittedInjectionDepth(relevantMethod) < paramDepth) {
+ return false;
+ }
+ allowlistClass(propDesc.getPropertyType());
+ if (paramDepth >= 2) {
+
allowlistParameterizedTypeArg(relevantMethod.getGenericReturnType());
+ }
+ return true;
+ }
+
+ private boolean allowlistViaPublicField(Object target, String
rootProperty, long paramDepth) {
+ Class<?> targetClass = ultimateClass(target);
+ Field field;
+ try {
+ field = targetClass.getDeclaredField(rootProperty);
+ } catch (NoSuchFieldException e) {
+ return false;
+ }
+ if (!Modifier.isPublic(field.getModifiers()) ||
getPermittedInjectionDepth(field) < paramDepth) {
+ return false;
+ }
+ allowlistClass(field.getType());
+ if (paramDepth >= 2) {
+ allowlistParameterizedTypeArg(field.getGenericType());
+ }
+ return true;
+ }
+
+ private void allowlistClass(Class<?> clazz) {
+ threadAllowlist.allowClassHierarchy(clazz);
+ }
+
+ private void allowlistParameterizedTypeArg(Type genericType) {
+ if (!(genericType instanceof ParameterizedType pType)) {
+ return;
+ }
+ Type[] paramTypes = pType.getActualTypeArguments();
+ allowlistParamType(paramTypes[0]);
+ if (paramTypes.length > 1) {
+ allowlistParamType(paramTypes[1]);
+ }
+ }
+
+ private void allowlistParamType(Type paramType) {
+ if (paramType instanceof Class<?> clazz) {
+ allowlistClass(clazz);
+ }
+ }
+
+ private int getPermittedInjectionDepth(AnnotatedElement element) {
+ StrutsParameter annotation =
element.getAnnotation(StrutsParameter.class);
+ return annotation == null ? -1 : annotation.depth();
+ }
+
+ private Class<?> ultimateClass(Object target) {
+ if (proxyService.isProxy(target)) {
+ return proxyService.ultimateTargetClass(target);
+ }
+ return target.getClass();
+ }
+
+ private BeanInfo getBeanInfo(Object target) {
+ Class<?> targetClass = ultimateClass(target);
+ try {
+ return ognlUtil.getBeanInfo(targetClass);
+ } catch (IntrospectionException e) {
+ LOG.warn("Error introspecting target {} for OGNL allowlisting",
targetClass, e);
+ return null;
+ }
+ }
+}
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAllowlister.java
b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAllowlister.java
new file mode 100644
index 000000000..846498c71
--- /dev/null
+++
b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAllowlister.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.interceptor.parameter;
+
+/**
+ * Primes channel-specific runtime state required for an already-authorized
parameter path to be walked by the
+ * value-stack — for example, registering the path's classes into the OGNL
{@link org.apache.struts2.ognl.ThreadAllowlist}
+ * so OGNL may traverse them. Separated from {@link ParameterAuthorizer} so
the authorization decision can remain
+ * side-effect-free and reusable from non-OGNL channels (Jackson, Juneau).
+ *
+ * <p>Implementations MUST NOT repeat the authorization decision — that is
owned by
+ * {@link ParameterAuthorizer#isAuthorized}. A no-op return (e.g. shallow
paths, unannotated root) means "no priming
+ * needed or possible" and never "rejected": callers must not treat the
absence of priming as a negative authorization
+ * signal.</p>
+ *
+ * @since 7.2.0
+ */
+public interface ParameterAllowlister {
+
+ /**
+ * Primes the channel-specific allowlist for an authorized parameter path.
Side-effect-only; no return value
+ * because a no-op is a valid outcome (see class-level javadoc).
+ *
+ * @param parameterName the parameter name (e.g. {@code "user.role"},
{@code "items[0].name"})
+ * @param target the object receiving the parameter value (the
action, or the model for ModelDriven actions)
+ */
+ void primeAllowlistForPath(String parameterName, Object target);
+}
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 5491b585f..365868a2b 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
@@ -65,10 +65,7 @@ import java.util.regex.Pattern;
import static java.lang.String.format;
import static java.util.Collections.unmodifiableSet;
import static java.util.stream.Collectors.joining;
-import static org.apache.commons.lang3.StringUtils.indexOfAny;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
-import static
org.apache.struts2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS;
-import static
org.apache.struts2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS_STR;
import static org.apache.struts2.util.DebugUtils.logWarningForFirstOccurrence;
import static org.apache.struts2.util.DebugUtils.notifyDeveloperOfError;
@@ -100,6 +97,7 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
private Set<Pattern> excludedValuePatterns = null;
private Set<Pattern> acceptedValuePatterns = null;
private ParameterAuthorizer parameterAuthorizer;
+ private transient ParameterAllowlister parameterAllowlister;
@Inject
public void setValueStackFactory(ValueStackFactory valueStackFactory) {
@@ -126,6 +124,11 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
this.parameterAuthorizer = parameterAuthorizer;
}
+ @Inject
+ public void setParameterAllowlister(ParameterAllowlister
parameterAllowlister) {
+ this.parameterAllowlister = parameterAllowlister;
+ }
+
@Inject(StrutsConstants.STRUTS_DEVMODE)
public void setDevMode(String mode) {
this.devMode = BooleanUtils.toBoolean(mode);
@@ -375,55 +378,10 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
return false;
}
- // 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);
- }
+ parameterAllowlister.primeAllowlistForPath(name, target);
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);
-
- BeanInfo beanInfo = getBeanInfo(target);
- if (beanInfo != null) {
- Optional<PropertyDescriptor> propDescOpt =
Arrays.stream(beanInfo.getPropertyDescriptors())
- .filter(desc ->
desc.getName().equals(normalisedRootProperty)).findFirst();
- if (propDescOpt.isPresent()) {
- PropertyDescriptor propDesc = propDescOpt.get();
- Method relevantMethod = paramDepth == 0 ?
propDesc.getWriteMethod() : propDesc.getReadMethod();
- if (relevantMethod != null &&
getPermittedInjectionDepth(relevantMethod) >= paramDepth) {
- allowlistClass(propDesc.getPropertyType());
- if (paramDepth >= 2) {
- allowlistReturnTypeIfParameterized(relevantMethod);
- }
- return;
- }
- }
- }
-
- // Fallback: check public field
- Class<?> targetClass = ultimateClass(target);
- try {
- Field field = targetClass.getDeclaredField(normalisedRootProperty);
- if (Modifier.isPublic(field.getModifiers()) &&
getPermittedInjectionDepth(field) >= paramDepth) {
- allowlistClass(field.getType());
- if (paramDepth >= 2) {
- allowlistFieldIfParameterized(field);
- }
- }
- } catch (NoSuchFieldException e) {
- // No field to allowlist
- }
- }
-
/**
* 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
diff --git a/core/src/main/resources/struts-beans.xml
b/core/src/main/resources/struts-beans.xml
index 232f0f4a4..21c4ec7e6 100644
--- a/core/src/main/resources/struts-beans.xml
+++ b/core/src/main/resources/struts-beans.xml
@@ -248,6 +248,9 @@
<bean type="org.apache.struts2.interceptor.parameter.ParameterAuthorizer"
name="struts"
class="org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer"
scope="singleton"/>
+ <bean type="org.apache.struts2.interceptor.parameter.ParameterAllowlister"
name="struts"
+
class="org.apache.struts2.interceptor.parameter.OgnlParameterAllowlister"
scope="singleton"/>
+
<bean type="org.apache.struts2.url.QueryStringBuilder"
name="strutsQueryStringBuilder"
class="org.apache.struts2.url.StrutsQueryStringBuilder"
scope="singleton"/>
<bean type="org.apache.struts2.url.QueryStringParser"
name="strutsQueryStringParser"
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorAnnotationTest.java
b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorAnnotationTest.java
new file mode 100644
index 000000000..4d794ecbf
--- /dev/null
+++
b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorAnnotationTest.java
@@ -0,0 +1,219 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.interceptor;
+
+import jakarta.servlet.http.Cookie;
+import org.apache.struts2.ActionContext;
+import org.apache.struts2.ActionSupport;
+import org.apache.struts2.ModelDriven;
+import org.apache.struts2.ServletActionContext;
+import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.action.Action;
+import org.apache.struts2.interceptor.parameter.ParameterAuthorizer;
+import org.apache.struts2.interceptor.parameter.StrutsParameter;
+import org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer;
+import org.apache.struts2.mock.MockActionInvocation;
+import org.apache.struts2.util.ValueStack;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class CookieInterceptorAnnotationTest extends StrutsInternalTestCase {
+
+ private CookieInterceptor interceptor;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ interceptor = container.inject(CookieInterceptor.class);
+ interceptor.setCookiesName("*");
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ // Reset shared singleton state — flags flipped on the container's
StrutsParameterAuthorizer
+ // would otherwise leak across tests in the same JVM run.
+ configureRequireAnnotations(false, false);
+ super.tearDown();
+ }
+
+ public void testRequireAnnotations_unannotatedSetter_isSkipped() throws
Exception {
+ configureRequireAnnotations(true, false);
+ AnnotatedAction action = new AnnotatedAction();
+ invokeWithCookies(action, new Cookie("unannotated", "v"));
+
+ assertNull("unannotated setter must not be populated",
action.getUnannotated());
+
assertNull(ActionContext.getContext().getValueStack().findValue("unannotated"));
+ }
+
+ public void testRequireAnnotations_annotatedSetter_isInjected() throws
Exception {
+ configureRequireAnnotations(true, false);
+ AnnotatedAction action = new AnnotatedAction();
+ invokeWithCookies(action, new Cookie("annotated", "v"));
+
+ assertEquals("v", action.getAnnotated());
+ assertEquals("v",
ActionContext.getContext().getValueStack().findValue("annotated"));
+ }
+
+ public void testRequireAnnotations_annotatedNestedPath_isInjected() throws
Exception {
+ configureRequireAnnotations(true, false);
+ AnnotatedAction action = new AnnotatedAction();
+ action.setNested(new NestedBean());
+ invokeWithCookies(action, new Cookie("nested.field", "v"));
+
+ assertEquals("v", action.getNested().getField());
+ }
+
+ public void testRequireAnnotations_unannotatedNestedPath_isSkipped()
throws Exception {
+ configureRequireAnnotations(true, false);
+ AnnotatedAction action = new AnnotatedAction();
+ action.setUnannotatedNested(new NestedBean());
+ invokeWithCookies(action, new Cookie("unannotatedNested.field", "v"));
+
+ assertNull(action.getUnannotatedNested().getField());
+ }
+
+ public void testRequireAnnotations_transitionMode_exemptsDepthZero()
throws Exception {
+ configureRequireAnnotations(true, true);
+ AnnotatedAction action = new AnnotatedAction();
+ invokeWithCookies(action, new Cookie("unannotated", "v"));
+
+ assertEquals("v", action.getUnannotated());
+ }
+
+ public void testDefaultConfig_unannotatedSetter_stillInjected() throws
Exception {
+ configureRequireAnnotations(false, false);
+ AnnotatedAction action = new AnnotatedAction();
+ invokeWithCookies(action, new Cookie("unannotated", "v"));
+
+ assertEquals("v", action.getUnannotated());
+ }
+
+ public void testRequireAnnotations_modelDriven_exemptsModel() throws
Exception {
+ configureRequireAnnotations(true, false);
+ ModelDrivenAction action = new ModelDrivenAction();
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setCookies(new Cookie("name", "v"));
+ ServletActionContext.setRequest(request);
+ // ModelDriven contract: the model is pushed on top of the action.
+ ActionContext.getContext().getValueStack().push(action);
+ ActionContext.getContext().getValueStack().push(action.getModel());
+
+ MockActionInvocation invocation = new MockActionInvocation();
+ invocation.setAction(action);
+ invocation.setInvocationContext(ActionContext.getContext());
+ invocation.setResultCode(Action.SUCCESS);
+
+ interceptor.intercept(invocation);
+
+ assertEquals("v", action.getModel().getName());
+ }
+
+ public void
testSubclassOverridingDeprecatedHook_stillSeesAuthorizationGate() throws
Exception {
+ configureRequireAnnotations(true, false);
+ AtomicInteger calls = new AtomicInteger();
+ @SuppressWarnings("deprecation")
+ CookieInterceptor subclass = new CookieInterceptor() {
+ @Override
+ protected void populateCookieValueIntoStack(String name, String
value, Map<String, String> map, ValueStack stack) {
+ calls.incrementAndGet();
+ super.populateCookieValueIntoStack(name, value, map, stack);
+ }
+ };
+ container.inject(subclass);
+ subclass.setCookiesName("*");
+
+ AnnotatedAction action = new AnnotatedAction();
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setCookies(new Cookie("annotated", "ok"), new
Cookie("unannotated", "blocked"));
+ ServletActionContext.setRequest(req);
+ ActionContext.getContext().getValueStack().push(action);
+
+ MockActionInvocation invocation = new MockActionInvocation();
+ invocation.setAction(action);
+ invocation.setInvocationContext(ActionContext.getContext());
+ invocation.setResultCode(Action.SUCCESS);
+ subclass.intercept(invocation);
+
+ assertEquals("ok", action.getAnnotated());
+ assertNull(action.getUnannotated());
+ assertEquals("4-arg hook should be invoked exactly once (only for the
authorized cookie)", 1, calls.get());
+ }
+
+ private void configureRequireAnnotations(boolean require, boolean
transitionMode) {
+ StrutsParameterAuthorizer authorizer = (StrutsParameterAuthorizer)
container.getInstance(ParameterAuthorizer.class);
+ authorizer.setRequireAnnotations(Boolean.toString(require));
+
authorizer.setRequireAnnotationsTransitionMode(Boolean.toString(transitionMode));
+ }
+
+ private void invokeWithCookies(Object action, Cookie... cookies) throws
Exception {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setCookies(cookies);
+ ServletActionContext.setRequest(request);
+ ActionContext.getContext().getValueStack().push(action);
+
+ MockActionInvocation invocation = new MockActionInvocation();
+ invocation.setAction(action);
+ invocation.setInvocationContext(ActionContext.getContext());
+ invocation.setResultCode(Action.SUCCESS);
+
+ interceptor.intercept(invocation);
+ }
+
+ public static class AnnotatedAction extends ActionSupport {
+ private String annotated;
+ private String unannotated;
+ private NestedBean nested;
+ private NestedBean unannotatedNested;
+
+ @StrutsParameter
+ public void setAnnotated(String v) { this.annotated = v; }
+ public String getAnnotated() { return annotated; }
+
+ public void setUnannotated(String v) { this.unannotated = v; }
+ public String getUnannotated() { return unannotated; }
+
+ @StrutsParameter(depth = 1)
+ public NestedBean getNested() { return nested; }
+ public void setNested(NestedBean nested) { this.nested = nested; }
+
+ public NestedBean getUnannotatedNested() { return unannotatedNested; }
+ public void setUnannotatedNested(NestedBean v) {
this.unannotatedNested = v; }
+ }
+
+ public static class NestedBean {
+ private String field;
+ public String getField() { return field; }
+ public void setField(String f) { this.field = f; }
+ }
+
+ public static class ModelDrivenAction extends ActionSupport implements
ModelDriven<Model> {
+ private final Model model = new Model();
+ @Override
+ public Model getModel() { return model; }
+ }
+
+ public static class Model {
+ private String name;
+ public String getName() { return name; }
+ public void setName(String n) { this.name = n; }
+ }
+}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java
index 8189f5ce8..8bcfe704c 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java
@@ -43,6 +43,15 @@ import static org.easymock.EasyMock.verify;
public class CookieInterceptorTest extends StrutsInternalTestCase {
+ /**
+ * These tests construct {@link CookieInterceptor} via {@code new} rather
than the DI container, so the
+ * {@code @StrutsParameter} authorization gate added in WW-5627 has no
injected services. We supply explicit
+ * pass-through lambdas to mirror the default-config behavior these tests
assume ({@code requireAnnotations=false}).
+ */
+ private static void disableAuthorizationGate(CookieInterceptor
interceptor) {
+ interceptor.setParameterAuthorizer((name, target, action) -> true);
+ interceptor.setParameterAllowlister((name, target) -> {});
+ }
public void testIntercepDefault() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest();
@@ -68,6 +77,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.intercept(invocation);
@@ -105,6 +115,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("*");
interceptor.setCookiesValue("*");
interceptor.intercept(invocation);
@@ -147,6 +158,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("cookie1, cookie2, cookie3");
interceptor.setCookiesValue("cookie1value, cookie2value,
cookie3value");
interceptor.intercept(invocation);
@@ -188,6 +200,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("cookie1, cookie3");
interceptor.setCookiesValue("cookie1value, cookie2value,
cookie3value");
interceptor.intercept(invocation);
@@ -230,6 +243,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("cookie1, cookie3");
interceptor.setCookiesValue("*");
interceptor.intercept(invocation);
@@ -271,6 +285,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("cookie1, cookie3");
interceptor.setCookiesValue("");
interceptor.intercept(invocation);
@@ -313,6 +328,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
CookieInterceptor interceptor = new CookieInterceptor();
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("cookie1, cookie3");
interceptor.setCookiesValue("cookie1value");
interceptor.intercept(invocation);
@@ -395,6 +411,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
excludedPatternsChecker.setAdditionalExcludePatterns(".*(^|\\.|\\[|'|\")class(\\.|\\[|'|\").*");
interceptor.setExcludedPatternsChecker(excludedPatternsChecker);
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("*");
MockActionInvocation invocation = new MockActionInvocation();
@@ -441,6 +458,7 @@ public class CookieInterceptorTest extends
StrutsInternalTestCase {
};
interceptor.setExcludedPatternsChecker(new
DefaultExcludedPatternsChecker());
interceptor.setAcceptedPatternsChecker(new
DefaultAcceptedPatternsChecker());
+ disableAuthorizationGate(interceptor);
interceptor.setCookiesName("*");
MockActionInvocation invocation = new MockActionInvocation();
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlisterTest.java
b/core/src/test/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlisterTest.java
new file mode 100644
index 000000000..7066a9bee
--- /dev/null
+++
b/core/src/test/java/org/apache/struts2/interceptor/parameter/OgnlParameterAllowlisterTest.java
@@ -0,0 +1,154 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.interceptor.parameter;
+
+import org.apache.struts2.ognl.DefaultOgnlBeanInfoCacheFactory;
+import org.apache.struts2.ognl.DefaultOgnlExpressionCacheFactory;
+import org.apache.struts2.ognl.OgnlUtil;
+import org.apache.struts2.ognl.StrutsOgnlGuard;
+import org.apache.struts2.ognl.StrutsProxyCacheFactory;
+import org.apache.struts2.ognl.ThreadAllowlist;
+import org.apache.struts2.util.StrutsProxyService;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.apache.struts2.ognl.OgnlCacheFactory.CacheType.LRU;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class OgnlParameterAllowlisterTest {
+
+ private OgnlParameterAllowlister allowlister;
+ private RecordingThreadAllowlist threadAllowlist;
+
+ @Before
+ public void setUp() {
+ threadAllowlist = new RecordingThreadAllowlist();
+ allowlister = new OgnlParameterAllowlister();
+ var ognlUtil = new OgnlUtil(
+ new DefaultOgnlExpressionCacheFactory<>(String.valueOf(1000),
LRU.toString()),
+ new DefaultOgnlBeanInfoCacheFactory<>(String.valueOf(1000),
LRU.toString()),
+ new StrutsOgnlGuard());
+ allowlister.setOgnlUtil(ognlUtil);
+ allowlister.setProxyService(new StrutsProxyService(new
StrutsProxyCacheFactory<>("1000", "basic")));
+ allowlister.setThreadAllowlist(threadAllowlist);
+ }
+
+ @After
+ public void tearDown() {
+ threadAllowlist.clear();
+ }
+
+ @Test
+ public void depthZero_isNoOp() {
+ var target = new TargetWithAnnotatedNestedBean();
+ allowlister.primeAllowlistForPath("simple", target);
+ assertThat(threadAllowlist.classes).isEmpty();
+ }
+
+ @Test
+ public void nestedProperty_allowlistsPropertyType() {
+ var target = new TargetWithAnnotatedNestedBean();
+ allowlister.primeAllowlistForPath("nested.field", target);
+ assertThat(threadAllowlist.classes).contains(NestedBean.class);
+ }
+
+ @Test
+ public void parameterizedReturn_allowlistsTypeArguments() {
+ var target = new TargetWithAnnotatedNestedBean();
+ allowlister.primeAllowlistForPath("things[0].field", target);
+ assertThat(threadAllowlist.classes).contains(List.class,
NestedBean.class);
+ }
+
+ @Test
+ public void publicField_isAllowlistedWhenNoGetter() {
+ var target = new TargetWithAnnotatedPublicField();
+ allowlister.primeAllowlistForPath("publicNested.field", target);
+ assertThat(threadAllowlist.classes).contains(NestedBean.class);
+ }
+
+ @Test
+ public void unmatchedRoot_isNoOp() {
+ var target = new TargetWithAnnotatedNestedBean();
+ allowlister.primeAllowlistForPath("unknownRoot.field", target);
+ assertThat(threadAllowlist.classes).isEmpty();
+ }
+
+ @Test
+ public void unannotatedNested_isNoOp() {
+ var target = new TargetWithUnannotatedNested();
+ allowlister.primeAllowlistForPath("unannotated.field", target);
+ assertThat(threadAllowlist.classes).isEmpty();
+ }
+
+ public static class TargetWithAnnotatedNestedBean {
+ private NestedBean nested;
+ private List<NestedBean> things;
+
+ @StrutsParameter(depth = 1)
+ public NestedBean getNested() { return nested; }
+ public void setNested(NestedBean nested) { this.nested = nested; }
+
+ @StrutsParameter(depth = 2)
+ public List<NestedBean> getThings() { return things; }
+ public void setThings(List<NestedBean> things) { this.things = things;
}
+ }
+
+ public static class TargetWithAnnotatedPublicField {
+ @StrutsParameter(depth = 1)
+ public NestedBean publicNested;
+ }
+
+ public static class TargetWithUnannotatedNested {
+ private NestedBean unannotated;
+ public NestedBean getUnannotated() { return unannotated; }
+ public void setUnannotated(NestedBean v) { this.unannotated = v; }
+ }
+
+ public static class NestedBean {
+ private String field;
+ public String getField() { return field; }
+ public void setField(String f) { this.field = f; }
+ }
+
+ private static final class RecordingThreadAllowlist extends
ThreadAllowlist {
+ final Set<Class<?>> classes = new HashSet<>();
+
+ @Override
+ public void allowClass(Class<?> clazz) {
+ classes.add(clazz);
+ super.allowClass(clazz);
+ }
+
+ @Override
+ public void allowClassHierarchy(Class<?> clazz) {
+ classes.add(clazz);
+ super.allowClassHierarchy(clazz);
+ }
+
+ void clear() {
+ classes.clear();
+ clearAllowlist();
+ }
+ }
+}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
index 803e0dad5..8ec445253 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
@@ -84,6 +84,12 @@ public class StrutsParameterAnnotationTest {
this.parameterAuthorizer = parameterAuthorizer;
parametersInterceptor.setParameterAuthorizer(parameterAuthorizer);
+ var parameterAllowlister = new OgnlParameterAllowlister();
+ parameterAllowlister.setOgnlUtil(ognlUtil);
+ parameterAllowlister.setProxyService(proxyService);
+ parameterAllowlister.setThreadAllowlist(threadAllowlist);
+ parametersInterceptor.setParameterAllowlister(parameterAllowlister);
+
NotExcludedAcceptedPatternsChecker checker =
mock(NotExcludedAcceptedPatternsChecker.class);
when(checker.isAccepted(anyString())).thenReturn(IsAccepted.yes(""));
when(checker.isExcluded(anyString())).thenReturn(IsExcluded.no(Set.of()));
