WW-4858 fix(json): evaluate name allowlist at leaf keys only

lukaszlenart · claude · lukaszlenart · commit 743ce88e4dda · 2026-07-14T12:51:53.000+02:00
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 &lt;noreply@anthropic.com&gt;
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
@@ -235,12 +235,19 @@ private void filterUnacceptableKeysRecursive(Map json, String prefix, Object tar
             }
             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) {
@@ -263,15 +270,23 @@ private void filterUnacceptableList(java.util.List list, String prefix, Object t
                 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 leaf binding targets: apply the leaf name-allowlist checks at
+            // the element path (e.g. "items[0]") plus value checks. The container key already passed
+            // the per-node checks in the caller.
+            } else if (!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;
@@ -280,26 +295,38 @@ private boolean isAcceptableKey(String fullPath, Object target, Object action) {
             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;
         }
         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
@@ -770,6 +770,95 @@ public void testExcludedNamePatternRejectsNestedKey() throws Exception {
         assertEquals(0, action.getBean().getIntField());
     }
 
+    public void testAcceptedNamePatternRejectsNestedKey() throws Exception {
+        this.request.setContent("{\"bean\": {\"stringField\": \"keep\", \"intField\": 42}}".getBytes());
+        this.request.addHeader("Content-Type", "application/json");
+
+        JSONInterceptor interceptor = createInterceptor();
+        org.apache.struts2.security.DefaultAcceptedPatternsChecker accepted =
+                new org.apache.struts2.security.DefaultAcceptedPatternsChecker();
+        // 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();
+
+        this.invocation.setAction(action);
+        this.invocation.getStack().push(action);
+
+        interceptor.intercept(this.invocation);
+
+        assertNotNull(action.getBean());
+        assertEquals("keep", action.getBean().getStringField());
+        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 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 testIncludePropertiesAppliedToNestedInputWhenEnabled() throws Exception {
+        this.request.setContent("{\"bean\": {\"stringField\": \"keep\", \"intField\": 42}}".getBytes());
+        this.request.addHeader("Content-Type", "application/json");
+
+        JSONInterceptor interceptor = createInterceptor();
+        interceptor.setApplyPropertyFiltersToInput(true);
+        // Include patterns expand across the hierarchy: "bean.stringField" compiles patterns for
+        // both the intermediate node "bean" and the leaf, so the nested leaf populates while the
+        // excluded sibling "bean.intField" is dropped.
+        interceptor.setIncludeProperties("bean\\.stringField");
+        TestAction action = new TestAction();
+
+        this.invocation.setAction(action);
+        this.invocation.getStack().push(action);
+
+        interceptor.intercept(this.invocation);
+
+        assertNotNull(action.getBean());
+        assertEquals("keep", action.getBean().getStringField());
+        assertEquals(0, 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
@@ -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 void setBaz(String baz) {
         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
