This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-4858-json-parameter-filtering in repository https://gitbox.apache.org/repos/asf/struts.git
commit 1c6d592a2f3c92fb369c10dab1b5fec35f031429 Author: Lukasz Lenart <[email protected]> AuthorDate: Wed Jul 8 20:31:08 2026 +0200 WW-4858 feat(json): enforce excluded/accepted name patterns on JSON population Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../org/apache/struts2/json/JSONInterceptor.java | 63 +++++++++++++++------- .../apache/struts2/json/JSONInterceptorTest.java | 44 ++++++++++++++- 2 files changed, 88 insertions(+), 19 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 d0acfd4ce..a4d084e0a 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 @@ -23,6 +23,8 @@ import org.apache.struts2.ActionInvocation; import org.apache.struts2.inject.Inject; import org.apache.struts2.interceptor.AbstractInterceptor; import org.apache.struts2.interceptor.parameter.ParameterAuthorizer; +import org.apache.struts2.security.AcceptedPatternsChecker; +import org.apache.struts2.security.ExcludedPatternsChecker; import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.WildcardUtil; import org.apache.commons.lang3.BooleanUtils; @@ -74,6 +76,8 @@ public class JSONInterceptor extends AbstractInterceptor { private JSONUtil jsonUtil; private ParameterAuthorizer parameterAuthorizer; + private ExcludedPatternsChecker excludedPatterns; + private AcceptedPatternsChecker acceptedPatterns; private int maxElements = JSONReader.DEFAULT_MAX_ELEMENTS; private int maxDepth = JSONReader.DEFAULT_MAX_DEPTH; private int maxLength = 2_097_152; // 2MB @@ -134,7 +138,7 @@ public class JSONInterceptor extends AbstractInterceptor { rootObject = invocation.getStack().peek(); // enforce @StrutsParameter authorization on JSON body keys - filterUnauthorizedKeys(json, rootObject, invocation.getAction()); + filterUnacceptableKeys(json, rootObject, invocation.getAction()); // populate fields populator.populateObject(rootObject, json); @@ -206,57 +210,70 @@ public class JSONInterceptor extends AbstractInterceptor { } @SuppressWarnings("rawtypes") - private void filterUnauthorizedKeys(Map json, Object target, Object action) { - filterUnauthorizedKeysRecursive(json, "", target, action); + private void filterUnacceptableKeys(Map json, Object target, Object action) { + filterUnacceptableKeysRecursive(json, "", target, action); } @SuppressWarnings("rawtypes") - private void filterUnauthorizedKeysRecursive(Map json, String prefix, Object target, Object action) { + private void filterUnacceptableKeysRecursive(Map json, String prefix, Object target, Object action) { Iterator<Map.Entry> it = json.entrySet().iterator(); while (it.hasNext()) { Map.Entry entry = it.next(); if (!(entry.getKey() instanceof String key)) { // Defensive: a custom JSONReader could produce non-String keys. Skip — we cannot - // construct a parameter path for authorization, and JSONPopulator wouldn't bind - // these to bean properties anyway. + // 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); continue; } String fullPath = prefix.isEmpty() ? key : prefix + "." + key; - if (!parameterAuthorizer.isAuthorized(fullPath, target, action)) { - LOG.warn("JSON body parameter [{}] rejected by @StrutsParameter authorization on [{}]", - fullPath, target.getClass().getName()); + if (!isAcceptableKey(fullPath, target, action)) { it.remove(); continue; } - // Recurse into nested Maps (JSON objects) to enforce depth-aware authorization Object value = entry.getValue(); if (value instanceof Map) { - filterUnauthorizedKeysRecursive((Map) value, fullPath, target, action); + filterUnacceptableKeysRecursive((Map) value, fullPath, target, action); } else if (value instanceof java.util.List) { - filterUnauthorizedList((java.util.List) value, fullPath, target, action); + filterUnacceptableList((java.util.List) value, fullPath, target, action); } } } @SuppressWarnings("rawtypes") - private void filterUnauthorizedList(java.util.List list, String prefix, Object target, Object action) { - // Use prefix+"[0]" so that list element properties pick up one extra '[' in their path, - // matching the indexed-path semantics of ParametersInterceptor (e.g. "items[0].key" → depth 2). + 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, + // matching the indexed-path semantics of ParametersInterceptor (e.g. "items[0].key"). String elementPrefix = prefix + "[0]"; for (Object item : list) { if (item instanceof Map) { - filterUnauthorizedKeysRecursive((Map) item, elementPrefix, target, action); + filterUnacceptableKeysRecursive((Map) item, elementPrefix, target, action); } else if (item instanceof java.util.List) { - // Handle nested lists (e.g. List<List<Map>>) by recursing with the same elementPrefix - filterUnauthorizedList((java.util.List) item, elementPrefix, target, action); + filterUnacceptableList((java.util.List) item, elementPrefix, target, action); } } } + @SuppressWarnings("rawtypes") + private boolean isAcceptableKey(String fullPath, Object target, Object action) { + if (excludedPatterns != null && excludedPatterns.isExcluded(fullPath).isExcluded()) { + 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; + } + return true; + } + protected String readContentType(HttpServletRequest request) { String contentType = request.getHeader("Content-Type"); LOG.debug("Content Type from request: {}", contentType); @@ -647,6 +664,16 @@ public class JSONInterceptor extends AbstractInterceptor { this.parameterAuthorizer = parameterAuthorizer; } + @Inject + public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) { + this.excludedPatterns = excludedPatterns; + } + + @Inject + public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) { + this.acceptedPatterns = acceptedPatterns; + } + @Inject(value = JSONConstants.JSON_MAX_ELEMENTS, required = false) public void setMaxElements(String maxElements) { this.maxElements = Integer.parseInt(maxElements); 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 ece2f1272..2986e557f 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 @@ -50,6 +50,8 @@ public class JSONInterceptorTest extends StrutsTestCase { interceptor.setJsonUtil(jsonUtil); // Default: allow all parameters (simulates requireAnnotations=false) interceptor.setParameterAuthorizer((parameterName, target, action) -> true); + interceptor.setExcludedPatterns(new org.apache.struts2.security.DefaultExcludedPatternsChecker()); + interceptor.setAcceptedPatterns(new org.apache.struts2.security.DefaultAcceptedPatternsChecker()); return interceptor; } @@ -598,7 +600,7 @@ public class JSONInterceptorTest extends StrutsTestCase { mixedKeyMap.put(42, "shouldBeSkipped"); // Integer key, not String java.lang.reflect.Method method = JSONInterceptor.class.getDeclaredMethod( - "filterUnauthorizedKeys", java.util.Map.class, Object.class, Object.class); + "filterUnacceptableKeys", java.util.Map.class, Object.class, Object.class); method.setAccessible(true); // Should not throw ClassCastException @@ -707,6 +709,46 @@ public class JSONInterceptorTest extends StrutsTestCase { assertEquals(0, bean.getIntField()); } + public void testExcludedNamePatternRejectsKey() throws Exception { + this.request.setContent("{\"foo\":\"a\", \"bar\":\"b\"}".getBytes()); + this.request.addHeader("Content-Type", "application/json"); + + JSONInterceptor interceptor = createInterceptor(); + org.apache.struts2.security.DefaultExcludedPatternsChecker excluded = + new org.apache.struts2.security.DefaultExcludedPatternsChecker(); + excluded.setExcludedPatterns("bar"); + interceptor.setExcludedPatterns(excluded); + TestAction action = new TestAction(); + + this.invocation.setAction(action); + this.invocation.getStack().push(action); + + interceptor.intercept(this.invocation); + + assertEquals("a", action.getFoo()); + assertNull(action.getBar()); + } + + public void testAcceptedNamePatternRejectsKey() throws Exception { + this.request.setContent("{\"foo\":\"a\", \"bar\":\"b\"}".getBytes()); + this.request.addHeader("Content-Type", "application/json"); + + JSONInterceptor interceptor = createInterceptor(); + org.apache.struts2.security.DefaultAcceptedPatternsChecker accepted = + new org.apache.struts2.security.DefaultAcceptedPatternsChecker(); + accepted.setAcceptedPatterns("foo"); + interceptor.setAcceptedPatterns(accepted); + TestAction action = new TestAction(); + + this.invocation.setAction(action); + this.invocation.getStack().push(action); + + interceptor.intercept(this.invocation); + + assertEquals("a", action.getFoo()); + assertNull(action.getBar()); + } + @Override protected void setUp() throws Exception { super.setUp();
