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 6d2713292 WW-4858 Evaluate JSON name allowlist at leaf keys only
(#1784)
6d2713292 is described below
commit 6d271329262bd82db732ee3b4facd053b8407dc0
Author: Lukasz Lenart <[email protected]>
AuthorDate: Tue Jul 21 10:59:38 2026 +0200
WW-4858 Evaluate JSON name allowlist at leaf keys only (#1784)
* WW-4858 fix(json): evaluate name allowlist at leaf keys only
The JSON population filter walked the object tree and applied every name
check at every node before recursing. Accepted name patterns and the
ParameterNameAware callback target the full dotted binding path, so gating
an intermediate node (e.g. "bean") against a leaf-specific rule dropped the
entire subtree before the leaf ("bean.stringField") was ever evaluated —
diverging from ParametersInterceptor, which only evaluates complete leaf
names. For arrays it also meant the accepted allowlist judged the container
name instead of the element path.
Split the per-key gate: length, excluded patterns, @StrutsParameter
authorization and property filters stay per-node (exclusion is prefix-safe
and authorization is intentionally hierarchical); accepted patterns and
ParameterNameAware move to leaf keys only, including scalar array elements
at their indexed path ("items[0]"). This reproduces the flat-path semantics
exactly. Excluded/include-property behavior is unchanged.
Tests: nested-object leaf populates under a leaf-targeting accepted pattern
and a ParameterNameAware action that rejects the intermediate node; accepted
patterns now apply to the array element path; nested include-property
filtering still works.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* WW-4858 fix(json): apply per-node checks to scalar array elements
Scalar list elements were gated only by the leaf name-allowlist and value
checks, skipping the per-node checks (length, excluded patterns,
@StrutsParameter authorization, property filters). That left the JSON path
more permissive than ParametersInterceptor, which evaluates all of these
against the full indexed name "items[0]".
Apply isAcceptableNode(elementPrefix, ...) to scalar list elements so an
element is gated exactly as the flat path gates "items[0]". Note this makes
scalar-list @StrutsParameter authorization use the element path (depth 1,
read method) rather than only the container (depth 0), matching the flat
path.
Tests: excluded name pattern and @StrutsParameter authorization now apply at
the list element path.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* WW-4858 refactor(json): extract keyTypeName helper to lower cognitive
complexity
Move the non-String-key logging ternary out of
filterUnacceptableKeysRecursive
into a keyTypeName helper. Pure extraction, no behavior change; drops the
method's cognitive complexity from 17 to 14, under Sonar's S3776 threshold.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
---------
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../org/apache/struts2/json/JSONInterceptor.java | 63 +++++++++++----
.../apache/struts2/json/JSONInterceptorTest.java | 94 ++++++++++++++++++++--
.../struts2/json/ParameterAwareTestAction.java | 13 ++-
3 files changed, 149 insertions(+), 21 deletions(-)
diff --git
a/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
b/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
index 804542399..9386a618d 100644
--- a/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
+++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java
@@ -233,17 +233,24 @@ public class JSONInterceptor extends AbstractInterceptor {
// Defensive: a custom JSONReader could produce non-String
keys. Skip — we cannot
// construct a parameter path for filtering, and JSONPopulator
wouldn't bind these anyway.
LOG.debug("Skipping JSON entry with non-String key [{}] of
type [{}] under prefix [{}]",
- entry.getKey(), entry.getKey() == null ? "null" :
entry.getKey().getClass().getName(), prefix);
+ entry.getKey(), keyTypeName(entry.getKey()), prefix);
continue;
}
String fullPath = prefix.isEmpty() ? key : prefix + "." + key;
- if (!isAcceptableKey(fullPath, target, action)) {
+ Object value = entry.getValue();
+ boolean leaf = !(value instanceof Map) && !(value instanceof
java.util.List);
+
+ // Per-node checks (length, excluded patterns, @StrutsParameter
authorization, property
+ // filters) apply to every node. Name-allowlist checks (accepted
patterns,
+ // ParameterNameAware) apply only at leaf keys, so an intermediate
node is never gated by
+ // a pattern written for a leaf path — mirroring
ParametersInterceptor, which only ever
+ // evaluates complete leaf names.
+ if (!isAcceptableNode(fullPath, target, action) || (leaf &&
!isAcceptableLeafName(fullPath, action))) {
it.remove();
continue;
}
- Object value = entry.getValue();
if (value instanceof Map) {
filterUnacceptableKeysRecursive((Map) value, fullPath, target,
action);
} else if (value instanceof java.util.List) {
@@ -254,6 +261,10 @@ public class JSONInterceptor extends AbstractInterceptor {
}
}
+ private static String keyTypeName(Object key) {
+ return key == null ? "null" : key.getClass().getName();
+ }
+
@SuppressWarnings("rawtypes")
private void filterUnacceptableList(java.util.List list, String prefix,
Object target, Object action) {
// Use prefix+"[0]" so list element properties pick up one extra '['
in their path,
@@ -266,15 +277,25 @@ public class JSONInterceptor extends AbstractInterceptor {
filterUnacceptableKeysRecursive((Map) item, elementPrefix,
target, action);
} else if (item instanceof java.util.List) {
filterUnacceptableList((java.util.List) item, elementPrefix,
target, action);
- // Scalar list elements are value-checked only; their parent key
already passed name/authorization checks.
- } else if (!isAcceptableValue(elementPrefix, item, action)) {
+ // Scalar list elements are full leaf binding targets: apply the
per-node checks, the leaf
+ // name-allowlist checks and the value checks at the element path
(e.g. "items[0]"), so a
+ // scalar element is gated exactly as ParametersInterceptor gates
the flat name "items[0]".
+ } else if (!isAcceptableNode(elementPrefix, target, action)
+ || !isAcceptableLeafName(elementPrefix, action)
+ || !isAcceptableValue(elementPrefix, item, action)) {
it.remove();
}
}
}
- @SuppressWarnings("rawtypes")
- private boolean isAcceptableKey(String fullPath, Object target, Object
action) {
+ /**
+ * Checks applied to every node of the JSON tree (both intermediate
objects/arrays and leaves):
+ * key length, excluded name patterns, {@code @StrutsParameter}
authorization, and — when
+ * enabled — the interceptor's own property filters. Excluded patterns are
prefix-safe and
+ * authorization is intentionally hierarchical (a parent must be
authorized for a child to be
+ * reachable), so these are correct to evaluate at intermediate nodes.
+ */
+ private boolean isAcceptableNode(String fullPath, Object target, Object
action) {
if (fullPath.length() > paramNameMaxLength) {
LOG.warn("JSON body parameter [{}] is too long, allowed length is
[{}]; rejected", fullPath, paramNameMaxLength);
return false;
@@ -283,19 +304,11 @@ public class JSONInterceptor extends AbstractInterceptor {
LOG.warn("JSON body parameter [{}] matches an excluded pattern;
rejected", fullPath);
return false;
}
- if (acceptedPatterns != null &&
!acceptedPatterns.isAccepted(fullPath).isAccepted()) {
- LOG.warn("JSON body parameter [{}] does not match any accepted
pattern; rejected", fullPath);
- return false;
- }
if (!parameterAuthorizer.isAuthorized(fullPath, target, action)) {
LOG.warn("JSON body parameter [{}] rejected by @StrutsParameter
authorization on [{}]",
fullPath, target.getClass().getName());
return false;
}
- if (action instanceof ParameterNameAware nameAware &&
!nameAware.acceptableParameterName(fullPath)) {
- LOG.debug("JSON body parameter [{}] rejected by ParameterNameAware
action", fullPath);
- return false;
- }
if (applyPropertyFiltersToInput &&
!isAcceptedByPropertyFilters(fullPath)) {
LOG.debug("JSON body parameter [{}] rejected by
excludeProperties/includeProperties on input", fullPath);
return false;
@@ -303,6 +316,26 @@ public class JSONInterceptor extends AbstractInterceptor {
return true;
}
+ /**
+ * Name-allowlist checks applied only at leaf keys (scalar values,
including scalar array
+ * elements): accepted name patterns and the {@link ParameterNameAware}
action callback. These
+ * are deliberately not applied to intermediate nodes: their
patterns/decisions target the full
+ * dotted binding path, and gating an intermediate node against a
leaf-specific rule would drop
+ * the whole subtree before the leaf is ever evaluated. This matches
ParametersInterceptor, which
+ * only ever evaluates complete leaf names.
+ */
+ private boolean isAcceptableLeafName(String fullPath, Object action) {
+ if (acceptedPatterns != null &&
!acceptedPatterns.isAccepted(fullPath).isAccepted()) {
+ LOG.warn("JSON body parameter [{}] does not match any accepted
pattern; rejected", fullPath);
+ return false;
+ }
+ if (action instanceof ParameterNameAware nameAware &&
!nameAware.acceptableParameterName(fullPath)) {
+ LOG.debug("JSON body parameter [{}] rejected by ParameterNameAware
action", fullPath);
+ return false;
+ }
+ return true;
+ }
+
private boolean isAcceptedByPropertyFilters(String fullPath) {
if (excludeProperties != null) {
for (Pattern pattern : excludeProperties) {
diff --git
a/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
b/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
index 4aa27563a..32853534a 100644
---
a/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
+++
b/plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java
@@ -762,11 +762,11 @@ public class JSONInterceptorTest extends StrutsTestCase {
JSONInterceptor interceptor = createInterceptor();
org.apache.struts2.security.DefaultAcceptedPatternsChecker accepted =
new
org.apache.struts2.security.DefaultAcceptedPatternsChecker();
- // Accept the intermediate node "bean" and the nested leaf
"bean.stringField", but not
- // "bean.intField". The intermediate node must itself match an
accepted pattern, otherwise
- // the whole subtree is dropped before the leaf is ever visited
(accepted name patterns are
- // raw full-match regexes with no hierarchy expansion, unlike include
patterns).
- accepted.setAcceptedPatterns("bean(\\.stringField)?");
+ // Leaf-targeting pattern: matches only the nested leaf
"bean.stringField", not the
+ // intermediate node "bean". Accepted patterns are evaluated at leaf
keys, so the
+ // intermediate node is not gated and the matching leaf still
populates while the
+ // non-matching sibling "bean.intField" is dropped.
+ accepted.setAcceptedPatterns("bean\\.stringField");
interceptor.setAcceptedPatterns(accepted);
TestAction action = new TestAction();
@@ -780,6 +780,90 @@ public class JSONInterceptorTest extends StrutsTestCase {
assertEquals(0, action.getBean().getIntField());
}
+ public void testAcceptedNamePatternAppliesToListElementPath() throws
Exception {
+ this.request.setContent("{\"list\": [\"x\", \"y\"]}".getBytes());
+ this.request.addHeader("Content-Type", "application/json");
+
+ JSONInterceptor interceptor = createInterceptor();
+ org.apache.struts2.security.DefaultAcceptedPatternsChecker accepted =
+ new
org.apache.struts2.security.DefaultAcceptedPatternsChecker();
+ // Matches the container name "list" but not the element path
"list[0]". Because accepted
+ // patterns are evaluated at the leaf (the scalar element path), the
elements are rejected
+ // even though the container name matches — mirroring the flat
ParametersInterceptor path,
+ // which would evaluate "list[0]" and reject it.
+ accepted.setAcceptedPatterns("list");
+ interceptor.setAcceptedPatterns(accepted);
+ TestAction action = new TestAction();
+
+ this.invocation.setAction(action);
+ this.invocation.getStack().push(action);
+
+ interceptor.intercept(this.invocation);
+
+ assertTrue(action.getList() == null || action.getList().isEmpty());
+ }
+
+ public void testExcludedNamePatternAppliesToListElementPath() throws
Exception {
+ this.request.setContent("{\"list\": [\"x\", \"y\"]}".getBytes());
+ this.request.addHeader("Content-Type", "application/json");
+
+ JSONInterceptor interceptor = createInterceptor();
+ org.apache.struts2.security.DefaultExcludedPatternsChecker excluded =
+ new
org.apache.struts2.security.DefaultExcludedPatternsChecker();
+ // Excludes the element path "list[0]" but not the container "list".
Scalar list elements are
+ // full leaf binding targets, so the per-node checks (here, excluded
patterns) are evaluated
+ // at the element path — mirroring the flat ParametersInterceptor
path, which would exclude
+ // "list[0]".
+ excluded.setExcludedPatterns("list\\[0\\]");
+ interceptor.setExcludedPatterns(excluded);
+ TestAction action = new TestAction();
+
+ this.invocation.setAction(action);
+ this.invocation.getStack().push(action);
+
+ interceptor.intercept(this.invocation);
+
+ assertTrue(action.getList() == null || action.getList().isEmpty());
+ }
+
+ public void testAuthorizationAppliesToListElementPath() throws Exception {
+ this.request.setContent("{\"list\": [\"x\", \"y\"]}".getBytes());
+ this.request.addHeader("Content-Type", "application/json");
+
+ JSONInterceptor interceptor = createInterceptor();
+ // Authorize the container "list" but reject the element path
"list[0]". Scalar list elements
+ // now pass through @StrutsParameter authorization at their element
path, so the elements are
+ // rejected even though the container is authorized.
+ interceptor.setParameterAuthorizer((parameterName, target, action) ->
!"list[0]".equals(parameterName));
+ TestAction action = new TestAction();
+
+ this.invocation.setAction(action);
+ this.invocation.getStack().push(action);
+
+ interceptor.intercept(this.invocation);
+
+ assertTrue(action.getList() == null || action.getList().isEmpty());
+ }
+
+ public void testParameterNameAwareDoesNotRejectIntermediateNode() throws
Exception {
+ this.request.setContent("{\"bean\": {\"stringField\": \"keep\",
\"intField\": 42}}".getBytes());
+ this.request.addHeader("Content-Type", "application/json");
+
+ JSONInterceptor interceptor = createInterceptor();
+ // The action's acceptableParameterName rejects the intermediate node
"bean" but accepts the
+ // nested leaves. ParameterNameAware is evaluated at leaf keys, so the
subtree is not dropped.
+ ParameterAwareTestAction action = new ParameterAwareTestAction();
+
+ this.invocation.setAction(action);
+ this.invocation.getStack().push(action);
+
+ interceptor.intercept(this.invocation);
+
+ assertNotNull(action.getBean());
+ assertEquals("keep", action.getBean().getStringField());
+ assertEquals(42, action.getBean().getIntField());
+ }
+
public void testExcludedValuePatternRejectsListElement() throws Exception {
this.request.setContent("{\"list\": [\"good\",
\"badvalue\"]}".getBytes());
this.request.addHeader("Content-Type", "application/json");
diff --git
a/plugins/json/src/test/java/org/apache/struts2/json/ParameterAwareTestAction.java
b/plugins/json/src/test/java/org/apache/struts2/json/ParameterAwareTestAction.java
index d71287cd9..eeb953ac2 100644
---
a/plugins/json/src/test/java/org/apache/struts2/json/ParameterAwareTestAction.java
+++
b/plugins/json/src/test/java/org/apache/struts2/json/ParameterAwareTestAction.java
@@ -26,6 +26,7 @@ public class ParameterAwareTestAction implements
ParameterNameAware, ParameterVa
private String foo;
private String bar;
private String baz;
+ private Bean bean;
public String getFoo() {
return foo;
@@ -51,9 +52,19 @@ public class ParameterAwareTestAction implements
ParameterNameAware, ParameterVa
this.baz = baz;
}
+ public Bean getBean() {
+ return bean;
+ }
+
+ public void setBean(Bean bean) {
+ this.bean = bean;
+ }
+
@Override
public boolean acceptableParameterName(String parameterName) {
- return !"bar".equals(parameterName);
+ // Reject the flat key "bar" and the intermediate node "bean"; nested
leaves such as
+ // "bean.stringField" are accepted so tests can assert intermediate
nodes are not gated.
+ return !"bar".equals(parameterName) && !"bean".equals(parameterName);
}
@Override
