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 39bef89be WW-5833 fix(rest): authorize nested container members at the
depth ParametersInterceptor counts (#2091)
39bef89be is described below
commit 39bef89be42ba680a8f6f14c53e345c3120732a9
Author: g0w6y <[email protected]>
AuthorDate: Sat Nov 7 13:39:00 2026 +0530
WW-5833 fix(rest): authorize nested container members at the depth
ParametersInterceptor counts (#2091)
AuthorizingValueDeserializer appended one [0] to the path of a container
property, so the members
of List<List<Bean>>, Map<String, List<Bean>> and Bean[][] were authorized
one depth level short
of the same name on the form path. Add one index per container level, and
recurse into nested
containers in the two-phase copy of ContentTypeInterceptor.
---
.../struts2/rest/ContentTypeInterceptor.java | 22 ++++++++++++
.../jackson/AuthorizingValueDeserializer.java | 13 ++++---
.../ContentTypeInterceptorIntegrationTest.java | 40 +++++++++++++++++++++-
.../org/apache/struts2/rest/SecureRestAction.java | 7 ++++
.../jackson/ParameterAuthorizingModuleTest.java | 27 +++++++++++++++
5 files changed, 103 insertions(+), 6 deletions(-)
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
index d98b22544..832f77e14 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java
@@ -328,6 +328,8 @@ public class ContentTypeInterceptor extends
AbstractInterceptor {
LOG.warn("REST collection element [{}] skipped — no no-arg
constructor for [{}]",
elementPath, element.getClass().getName());
}
+ } else if (isContainer(element)) {
+ result.add(deepCopyAuthorizedContainer(element, collectionPath
+ "[0]", authTarget, action));
} else {
result.add(element);
}
@@ -366,6 +368,8 @@ public class ContentTypeInterceptor extends
AbstractInterceptor {
LOG.warn("REST map value [{}] skipped — no no-arg
constructor for [{}]",
valuePath, value.getClass().getName());
}
+ } else if (isContainer(value)) {
+ result.put(entry.getKey(), deepCopyAuthorizedContainer(value,
mapPath + "[0]", authTarget, action));
} else {
result.put(entry.getKey(), value);
}
@@ -398,6 +402,8 @@ public class ContentTypeInterceptor extends
AbstractInterceptor {
LOG.warn("REST array element [{}] skipped — no no-arg
constructor for [{}]",
elementPath, element.getClass().getName());
}
+ } else if (isContainer(element)) {
+ Array.set(result, i, deepCopyAuthorizedContainer(element,
arrayPath + "[0]", authTarget, action));
} else {
Array.set(result, i, element);
}
@@ -405,6 +411,22 @@ public class ContentTypeInterceptor extends
AbstractInterceptor {
return result;
}
+ private static boolean isContainer(Object value) {
+ return value instanceof Collection || value instanceof Map || (value
!= null && value.getClass().isArray());
+ }
+
+ /** A container nested in a container sits one index level deeper, as in
{@code List<List<Bean>>}. */
+ private Object deepCopyAuthorizedContainer(Object value, String path,
Object authTarget, Object action)
+ throws Exception {
+ if (value instanceof Collection) {
+ return deepCopyAuthorizedCollection((Collection<?>) value, path,
authTarget, action);
+ }
+ if (value instanceof Map) {
+ return deepCopyAuthorizedMap((Map<?, ?>) value, path, authTarget,
action);
+ }
+ return deepCopyAuthorizedArray(value, path, authTarget, action);
+ }
+
/**
* 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
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java
index 2676c0e2d..b08f0b924 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java
@@ -168,14 +168,17 @@ final class AuthorizingValueDeserializer extends
DelegatingDeserializer {
/**
* For Collection / Map / Array properties, the path to push for nested
element members is
- * {@code path + "[0]"} -- matching {@code ParametersInterceptor}
bracket-depth semantics. Scalar /
+ * {@code path + "[0]"} per container level -- matching {@code
ParametersInterceptor} bracket-depth
+ * semantics, so {@code List<List<Bean>>} members sit under {@code
path[0][0]}. Scalar /
* bean-valued properties push the path unchanged.
*/
private String prefixForNested(String pathOfThisProperty) {
- if (propertyType != null
- && (propertyType.isCollectionLikeType() ||
propertyType.isMapLikeType() || propertyType.isArrayType())) {
- return pathOfThisProperty + "[0]";
+ StringBuilder prefix = new StringBuilder(pathOfThisProperty);
+ for (JavaType type = propertyType; type != null
+ && (type.isCollectionLikeType() || type.isMapLikeType() ||
type.isArrayType());
+ type = type.getContentType()) {
+ prefix.append("[0]");
}
- return pathOfThisProperty;
+ return prefix.toString();
}
}
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
index 0ac5ce458..48cb779de 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
@@ -20,6 +20,7 @@ package org.apache.struts2.rest;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.ObjectMapper;
import com.mockobjects.dynamic.AnyConstraintMatcher;
import com.mockobjects.dynamic.Mock;
import junit.framework.TestCase;
@@ -47,6 +48,7 @@ import org.apache.struts2.util.StrutsProxyService;
import org.apache.struts2.util.ValueStack;
import org.springframework.mock.web.MockHttpServletRequest;
+import java.io.IOException;
import java.io.Reader;
import java.io.Writer;
import java.util.ArrayList;
@@ -65,6 +67,7 @@ import static
org.apache.struts2.ognl.OgnlCacheFactory.CacheType.LRU;
public class ContentTypeInterceptorIntegrationTest extends TestCase {
private ContentTypeInterceptor interceptor;
+ private StrutsParameterAuthorizer authorizer;
private SecureRestAction action;
private Object configuredAction;
private Mock mockActionInvocation;
@@ -95,7 +98,7 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
new StrutsOgnlGuard());
var proxyService = new StrutsProxyService(new
StrutsProxyCacheFactory<>("1000", "basic"));
- StrutsParameterAuthorizer authorizer = new StrutsParameterAuthorizer();
+ authorizer = new StrutsParameterAuthorizer();
authorizer.setOgnlUtil(ognlUtil);
authorizer.setProxyService(proxyService);
authorizer.setRequireAnnotations(Boolean.TRUE.toString());
@@ -178,6 +181,24 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
action.getShallowAddress().getZip());
}
+ public void
testNestedContainerMembersAreRejectedAtTheDepthTheFormPathRejects() throws
Exception {
+ assertTrue(authorizer.isAuthorized("addressRows[0][0]", action,
action));
+ assertFalse(authorizer.isAuthorized("addressRows[0][0].city", action,
action));
+
+ runWithBody("{\"addressRows\":[[{\"city\":\"Warsaw\"}]]}");
+
+ assertNotNull(action.getAddressRows().get(0).get(0));
+ assertNull(action.getAddressRows().get(0).get(0).getCity());
+ }
+
+ public void testTwoPhaseCopyRejectsNestedContainerMembersAtTheSameDepth()
throws Exception {
+ setupInterceptorWithHandler(action, new PlainJacksonHandler());
+
+ runWithBody("{\"addressRows\":[[{\"city\":\"Warsaw\"}]]}");
+
+ assertNull(action.getAddressRows().get(0).get(0).getCity());
+ }
+
// --- WW-5789: a handler that binds nothing must leave the model as the
params interceptor left it ---
public void testFormUrlEncodedBodyKeepsValuesBoundOnTheModel() throws
Exception {
@@ -373,6 +394,23 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
+ " unannotated setAdmin", merged.admin());
}
+ /** A handler without the authorization-aware marker, so the interceptor
takes the two-phase copy. */
+ public static class PlainJacksonHandler implements ContentTypeHandler {
+ @Override
+ public void toObject(ActionInvocation invocation, Reader in, Object
target) throws IOException {
+ new ObjectMapper().readerForUpdating(target).readValue(in);
+ }
+
+ @Override
+ public String fromObject(ActionInvocation invocation, Object obj,
String resultCode, Writer stream) { return null; }
+
+ @Override
+ public String getContentType() { return "application/json"; }
+
+ @Override
+ public String getExtension() { return "json"; }
+ }
+
/** Stands in for a third-party handler that leaves the module's thread
state behind. */
public static class StatePlantingHandler implements
AuthorizationAwareContentTypeHandler {
@Override
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/SecureRestAction.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/SecureRestAction.java
index 8ca032f50..b7d24edec 100644
--- a/plugins/rest/src/test/java/org/apache/struts2/rest/SecureRestAction.java
+++ b/plugins/rest/src/test/java/org/apache/struts2/rest/SecureRestAction.java
@@ -36,6 +36,7 @@ public class SecureRestAction extends ActionSupport {
private Address shallowAddress;
private List<String> tags;
private List<Address> addresses;
+ private List<List<Address>> addressRows;
private Map<String, String> attributes;
private String[] aliases;
@@ -76,6 +77,12 @@ public class SecureRestAction extends ActionSupport {
@StrutsParameter
public void setAddresses(List<Address> addresses) { this.addresses =
addresses; }
+ @StrutsParameter(depth = 2)
+ public List<List<Address>> getAddressRows() { return addressRows; }
+
+ @StrutsParameter
+ public void setAddressRows(List<List<Address>> addressRows) {
this.addressRows = addressRows; }
+
@StrutsParameter(depth = 1)
public Map<String, String> getAttributes() { return attributes; }
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
index 213b9edee..a6d5840ac 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
@@ -151,6 +151,30 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
assertNull(result.addresses.get(0).zip);
}
+ public void testNestedListUsesOneIndexPerContainerLevel() throws Exception
{
+ bind((path, t, a) -> "addressRows".equals(path) ||
"addressRows[0][0].city".equals(path), new Person());
+ Person result = mapper.readValue(
+
"{\"addressRows\":[[{\"city\":\"Warsaw\",\"zip\":\"00-001\"}]]}", Person.class);
+ assertEquals("Warsaw", result.addressRows.get(0).get(0).city);
+ assertNull(result.addressRows.get(0).get(0).zip);
+ }
+
+ public void testMapOfListUsesOneIndexPerContainerLevel() throws Exception {
+ bind((path, t, a) -> "addressesByKey".equals(path) ||
"addressesByKey[0][0].city".equals(path), new Person());
+ Person result = mapper.readValue(
+
"{\"addressesByKey\":{\"home\":[{\"city\":\"Warsaw\",\"zip\":\"00-001\"}]}}",
Person.class);
+ assertEquals("Warsaw", result.addressesByKey.get("home").get(0).city);
+ assertNull(result.addressesByKey.get("home").get(0).zip);
+ }
+
+ public void testArrayOfArrayUsesOneIndexPerContainerLevel() throws
Exception {
+ bind((path, t, a) -> "addressGrid".equals(path) ||
"addressGrid[0][0].city".equals(path), new Person());
+ Person result = mapper.readValue(
+
"{\"addressGrid\":[[{\"city\":\"Warsaw\",\"zip\":\"00-001\"}]]}", Person.class);
+ assertEquals("Warsaw", result.addressGrid[0][0].city);
+ assertNull(result.addressGrid[0][0].zip);
+ }
+
public void testArrayUsesIndexedPath() throws Exception {
bind((path, t, a) -> "addressArray".equals(path) ||
"addressArray[0].city".equals(path), new Person());
Person result = mapper.readValue(
@@ -1168,6 +1192,9 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
public java.util.List<Address> addresses;
public Address[] addressArray;
public java.util.Map<String, Address> addressMap;
+ public java.util.List<java.util.List<Address>> addressRows;
+ public java.util.Map<String, java.util.List<Address>> addressesByKey;
+ public Address[][] addressGrid;
public RecordAddress recordAddress;
}
