WW-5626 cleanup follow-ups for @StrutsParameter JSON/REST enforcement (#1673)

* WW-5626 add ParameterAuthorizer#resolveTarget for centralized ModelDriven resolution

Move the ValueStack peek logic that derives the target object from action+ModelDriven
state out of ParametersInterceptor and into ParameterAuthorizer. Callers that need
both authorization and the resolved target (for downstream OGNL allowlisting) can
now call resolveTarget once and reuse the result.

* WW-5626 delegate ModelDriven target resolution to ParameterAuthorizer

Replace the inline ValueStack peek in ParametersInterceptor#isParameterAnnotatedAndAllowlist
with a call to ParameterAuthorizer#resolveTarget. The ModelDriven import is no longer
needed in this class.

* WW-5626 defensively skip non-String JSON keys in authorization filter

The (String) cast in filterUnauthorizedKeysRecursive threw ClassCastException
for any custom JSONReader producing non-String keys. Replace with an instanceof
pattern that debug-logs and skips entries whose key cannot be converted to a
parameter path.

* WW-5626 add real JacksonJsonHandler integration tests for @StrutsParameter filtering

The existing ContentTypeInterceptorTest uses mock ContentTypeHandlers, so its
requireAnnotations=true tests verify only that intercept() returns SUCCESS — they
assert nothing about which properties were actually filtered. These integration
tests use a real JacksonJsonHandler + a real StrutsParameterAuthorizer to verify
end-to-end property-level filtering for top-level annotated/unannotated properties
and nested properties at varying authorized depths.

The SecureRestAction fixture documents a semantic divergence: REST's recursive
copy authorizes each path level independently, so depth-0 authorization on the
top-level property requires @StrutsParameter on the setter even when nested
field access is the actual goal. ParametersInterceptor only requires the getter
annotation. This divergence is tracked for the Approach C refactor.

* WW-5626 make ParameterAuthorizer#resolveTarget a default method to preserve SAM

Making resolveTarget abstract broke ParameterAuthorizer as a functional interface,
which the existing JSON and REST plugin tests rely on for lambda-based stubs:
  interceptor.setParameterAuthorizer((parameterName, target, action) -> true);

The default returns the action unchanged — adequate for lambda test stubs whose
authorization decisions don't depend on the resolved target. The production
implementation (StrutsParameterAuthorizer) overrides this with the proper
ModelDriven value-stack peek.

core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizer.java

@@ -45,4 +45,24 @@ public interface ParameterAuthorizer {
      * @return {@code true} if the parameter is authorized for injection, {@code false} otherwise
      */
     boolean isAuthorized(String parameterName, Object target, Object action);
+
+    /**
+     * Resolves the target object whose annotations should be checked for authorization.
+     * For {@link org.apache.struts2.ModelDriven} actions, the default implementation returns the action itself;
+     * the production implementation ({@link StrutsParameterAuthorizer}) overrides this to return the model from
+     * the value stack.
+     *
+     * <p>Callers that need both authorization checks AND the resolved target (e.g. for downstream OGNL allowlisting)
+     * should call this once and reuse the result.</p>
+     *
+     * <p>This is a {@code default} method to preserve the interface as a functional interface (SAM) for
+     * lambda-based test stubs.</p>
+     *
+     * @param action the action instance
+     * @return the resolved target — either the action or its model
+     * @since 7.2.0
+     */
+    default Object resolveTarget(Object action) {
+        return action;
+    }
 }

core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java

@@ -23,7 +23,6 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.ActionContext;
 import org.apache.struts2.ActionInvocation;
-import org.apache.struts2.ModelDriven;
 import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.action.NoParameters;
 import org.apache.struts2.action.ParameterNameAware;
@@ -369,14 +368,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
             return true;
         }
 
-        // Resolve target for ModelDriven: if the ValueStack peek is different from the action, it's the model
-        Object target = action;
-        if (action instanceof ModelDriven<?>) {
-            Object stackTop = ActionContext.getContext().getValueStack().peek();
-            if (!stackTop.equals(action)) {
-                target = stackTop;
-            }
-        }
+        Object target = parameterAuthorizer.resolveTarget(action);
 
         // Delegate authorization check to shared ParameterAuthorizer (no OGNL side effects)
         if (!parameterAuthorizer.isAuthorized(name, target, action)) {

core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameterAuthorizer.java

@@ -21,6 +21,7 @@ package org.apache.struts2.interceptor.parameter;
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.struts2.ActionContext;
 import org.apache.struts2.ModelDriven;
 import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.inject.Inject;
@@ -91,6 +92,17 @@ public class StrutsParameterAuthorizer implements ParameterAuthorizer {
         this.requireAnnotationsTransitionMode = BooleanUtils.toBoolean(transitionMode);
     }
 
+    @Override
+    public Object resolveTarget(Object action) {
+        if (action instanceof ModelDriven<?>) {
+            Object stackTop = ActionContext.getContext().getValueStack().peek();
+            if (!stackTop.equals(action)) {
+                return stackTop;
+            }
+        }
+        return action;
+    }
+
     @Override
     public boolean isAuthorized(String parameterName, Object target, Object action) {
         if (parameterName == null || parameterName.isEmpty()) {

core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizerTest.java

@@ -18,13 +18,16 @@
  */
 package org.apache.struts2.interceptor.parameter;
 
+import org.apache.struts2.ActionContext;
 import org.apache.struts2.ModelDriven;
+import org.apache.struts2.StubValueStack;
 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.util.StrutsProxyService;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -56,6 +59,11 @@ public class ParameterAuthorizerTest {
         authorizer.setProxyService(proxyService);
     }
 
+    @After
+    public void tearDown() {
+        ActionContext.clear();
+    }
+
     // --- requireAnnotations=false (backward compat) ---
 
     @Test
@@ -182,6 +190,37 @@ public class ParameterAuthorizerTest {
         assertThat(authorizer.isAuthorized(null, action, action)).isFalse();
     }
 
+    // --- resolveTarget ---
+
+    @Test
+    public void resolveTarget_nonModelDriven_returnsAction() {
+        var action = new SecureAction();
+        assertThat(authorizer.resolveTarget(action)).isSameAs(action);
+    }
+
+    @Test
+    public void resolveTarget_modelDriven_returnsModelFromValueStack() {
+        var action = new ModelAction();
+        var model = action.getModel();
+        var valueStack = new StubValueStack();
+        valueStack.push(model);
+        ActionContext.of().withValueStack(valueStack).bind();
+
+        assertThat(authorizer.resolveTarget(action)).isSameAs(model);
+    }
+
+    @Test
+    public void resolveTarget_modelDriven_stackTopEqualsAction_returnsAction() {
+        // Edge case: ModelDriven action where stack top equals the action itself.
+        // No exemption applies — target stays as action.
+        var action = new ModelAction();
+        var valueStack = new StubValueStack();
+        valueStack.push(action);
+        ActionContext.of().withValueStack(valueStack).bind();
+
+        assertThat(authorizer.resolveTarget(action)).isSameAs(action);
+    }
+
     // --- Inner test classes ---
 
     public static class SecureAction {

plugins/json/src/main/java/org/apache/struts2/json/JSONInterceptor.java

@@ -215,7 +215,14 @@ public class JSONInterceptor extends AbstractInterceptor {
         Iterator<Map.Entry> it = json.entrySet().iterator();
         while (it.hasNext()) {
             Map.Entry entry = it.next();
-            String key = (String) entry.getKey();
+            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.
+                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)) {

plugins/json/src/test/java/org/apache/struts2/json/JSONInterceptorTest.java

@@ -583,6 +583,32 @@ public class JSONInterceptorTest extends StrutsTestCase {
         assertNull(action.getBar());
     }
 
+    public void testNonStringKeysAreSkippedByAuthorizationFilter() throws Exception {
+        // Simulate a custom JSON reader producing a Map with a non-String key.
+        // The authorizer should skip the entry rather than throw ClassCastException.
+        JSONInterceptor interceptor = new JSONInterceptor();
+        JSONUtil jsonUtil = new JSONUtil();
+        jsonUtil.setReader(new StrutsJSONReader());
+        jsonUtil.setWriter(new StrutsJSONWriter());
+        interceptor.setJsonUtil(jsonUtil);
+        interceptor.setParameterAuthorizer((parameterName, target, action) -> true);
+
+        java.util.Map<Object, Object> mixedKeyMap = new java.util.LinkedHashMap<>();
+        mixedKeyMap.put("validKey", "ok");
+        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);
+        method.setAccessible(true);
+
+        // Should not throw ClassCastException
+        method.invoke(interceptor, mixedKeyMap, new TestAction(), new TestAction());
+
+        // The non-String key entry should still be present (skipped, not removed)
+        assertTrue("non-String-key entry should remain (skipped, not removed)", mixedKeyMap.containsKey(42));
+        assertTrue("String-key entry should remain", mixedKeyMap.containsKey("validKey"));
+    }
+
     public void testParameterAuthorizerAllowsAllWhenPermissive() throws Exception {
         // Same JSON body, but authorizer allows all
         this.request.setContent("{\"foo\":\"value1\", \"bar\":\"value2\"}".getBytes());

plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java

@@ -0,0 +1,134 @@
+/*
+ * 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.rest;
+
+import com.mockobjects.dynamic.AnyConstraintMatcher;
+import com.mockobjects.dynamic.Mock;
+import junit.framework.TestCase;
+import org.apache.struts2.ActionContext;
+import org.apache.struts2.ActionInvocation;
+import org.apache.struts2.action.Action;
+import org.apache.struts2.dispatcher.mapper.ActionMapping;
+import org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer;
+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.rest.handler.JacksonJsonHandler;
+import org.apache.struts2.util.StrutsProxyService;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import static org.apache.struts2.ognl.OgnlCacheFactory.CacheType.LRU;
+
+/**
+ * Integration tests for ContentTypeInterceptor that use a real {@link JacksonJsonHandler}
+ * and a real {@link StrutsParameterAuthorizer}, end-to-end. Verifies that property
+ * filtering actually occurs on the deserialized object — not merely that the wiring runs.
+ */
+public class ContentTypeInterceptorIntegrationTest extends TestCase {
+
+    private ContentTypeInterceptor interceptor;
+    private SecureRestAction action;
+    private Mock mockActionInvocation;
+    private Mock mockSelector;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        action = new SecureRestAction();
+
+        var ognlUtil = new OgnlUtil(
+                new DefaultOgnlExpressionCacheFactory<>("1000", LRU.toString()),
+                new DefaultOgnlBeanInfoCacheFactory<>("1000", LRU.toString()),
+                new StrutsOgnlGuard());
+        var proxyService = new StrutsProxyService(new StrutsProxyCacheFactory<>("1000", "basic"));
+
+        StrutsParameterAuthorizer authorizer = new StrutsParameterAuthorizer();
+        authorizer.setOgnlUtil(ognlUtil);
+        authorizer.setProxyService(proxyService);
+        authorizer.setRequireAnnotations(Boolean.TRUE.toString());
+
+        interceptor = new ContentTypeInterceptor();
+        interceptor.setParameterAuthorizer(authorizer);
+        interceptor.setRequireAnnotations(Boolean.TRUE.toString());
+
+        mockActionInvocation = new Mock(ActionInvocation.class);
+        mockSelector = new Mock(ContentTypeHandlerManager.class);
+        // ContentTypeInterceptor calls getAction() twice when requireAnnotations=true
+        mockActionInvocation.expectAndReturn("getAction", action);
+        mockActionInvocation.expectAndReturn("getAction", action);
+        mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS);
+        mockSelector.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() {
+            public boolean matches(Object[] args) { return true; }
+        }, new JacksonJsonHandler());
+        interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockSelector.proxy());
+    }
+
+    private void runWithBody(String body) throws Exception {
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setContent(body.getBytes());
+        request.setContentType("application/json");
+
+        ActionContext.of()
+                .withActionMapping(new ActionMapping())
+                .withServletRequest(request)
+                .bind();
+
+        interceptor.intercept((ActionInvocation) mockActionInvocation.proxy());
+        mockSelector.verify();
+        mockActionInvocation.verify();
+    }
+
+    public void testAnnotatedTopLevelPropertyIsApplied() throws Exception {
+        runWithBody("{\"name\":\"alice\"}");
+        assertEquals("alice", action.getName());
+    }
+
+    public void testUnannotatedTopLevelPropertyIsRejected() throws Exception {
+        runWithBody("{\"role\":\"admin\"}");
+        assertNull("unannotated 'role' must not be set", action.getRole());
+    }
+
+    public void testMixedPropertiesFilteredCorrectly() throws Exception {
+        runWithBody("{\"name\":\"alice\",\"role\":\"admin\"}");
+        assertEquals("alice", action.getName());
+        assertNull(action.getRole());
+    }
+
+    public void testNestedPropertyAuthorizedWhenDepthAllows() throws Exception {
+        runWithBody("{\"address\":{\"city\":\"Warsaw\",\"zip\":\"00-001\"}}");
+        assertNotNull("address should be set", action.getAddress());
+        assertEquals("Warsaw", action.getAddress().getCity());
+        assertEquals("00-001", action.getAddress().getZip());
+    }
+
+    public void testNestedPropertyRejectedWhenDepthInsufficient() throws Exception {
+        // shallowAddress has @StrutsParameter (depth=0) — its nested fields should NOT be set
+        runWithBody("{\"shallowAddress\":{\"city\":\"Warsaw\",\"zip\":\"00-001\"}}");
+        // The top-level shallowAddress reference itself may be created (Jackson behavior)
+        // but its nested city/zip must remain null because depth-1 isn't allowed.
+        if (action.getShallowAddress() != null) {
+            assertNull("nested city should be rejected (depth insufficient)",
+                    action.getShallowAddress().getCity());
+            assertNull("nested zip should be rejected (depth insufficient)",
+                    action.getShallowAddress().getZip());
+        }
+    }
+}

plugins/rest/src/test/java/org/apache/struts2/rest/SecureRestAction.java

@@ -0,0 +1,70 @@
+/*
+ * 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.rest;
+
+import org.apache.struts2.ActionSupport;
+import org.apache.struts2.interceptor.parameter.StrutsParameter;
+
+/**
+ * Test fixture for ContentTypeInterceptor integration tests.
+ * Has annotated and unannotated properties to exercise authorization filtering.
+ */
+public class SecureRestAction extends ActionSupport {
+
+    private String name;
+    private String role;
+    private Address address;
+    private Address shallowAddress;
+
+    public String getName() { return name; }
+
+    @StrutsParameter
+    public void setName(String name) { this.name = name; }
+
+    public String getRole() { return role; }
+    public void setRole(String role) { this.role = role; }
+
+    // Both annotations needed for REST: setter authorizes the top-level "address" (depth 0),
+    // getter(depth=1) authorizes nested "address.city" (depth 1). Note: ParametersInterceptor
+    // only requires the getter annotation — REST's recursive copy authorizes each path level
+    // independently. This divergence is tracked for the Approach C refactor.
+    @StrutsParameter(depth = 1)
+    public Address getAddress() { return address; }
+
+    @StrutsParameter
+    public void setAddress(Address address) { this.address = address; }
+
+    // shallowAddress: depth-0 authorized (setter annotated), but nested fields rejected
+    // because the getter has no depth>=1 annotation.
+    public Address getShallowAddress() { return shallowAddress; }
+
+    @StrutsParameter
+    public void setShallowAddress(Address shallowAddress) { this.shallowAddress = shallowAddress; }
+
+    public static class Address {
+        private String city;
+        private String zip;
+
+        public String getCity() { return city; }
+        public void setCity(String city) { this.city = city; }
+
+        public String getZip() { return zip; }
+        public void setZip(String zip) { this.zip = zip; }
+    }
+}