WW-5844 fix(core)!: enforce HTTP method annotations across action hierarchies and plugin stacks

core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java

@@ -18,6 +18,7 @@
  */
 package org.apache.struts2.interceptor.httpmethod;
 
+import org.apache.commons.lang3.ClassUtils;
 import org.apache.struts2.ActionInvocation;
 import org.apache.struts2.action.Action;
 import org.apache.struts2.interceptor.AbstractInterceptor;
@@ -32,11 +33,13 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -54,6 +57,11 @@
  * <p>
  * You can combine any of these annotations to achieve required allowed methods' filtering.
  * <p>
+ * Method annotations apply to the invoked method, including the implicit {@code execute()} method.
+ * Its annotation takes precedence; otherwise annotated declarations are looked up through the class
+ * and interface hierarchy. Class annotations are looked up only when no method annotation is found.
+ * Restrictions apply to {@link HttpServletRequest#getMethod()}, not a {@code _method} parameter.
+ * <p>
  * The allowed methods can also be declared on the action in XML with the {@code http-method} attribute.
  * When both the attribute and annotations are present, the request method must satisfy both.
  * <p>
@@ -97,18 +105,20 @@ public String intercept(ActionInvocation invocation) throws Exception {
             ((HttpMethodAware) (action)).setMethod(HttpMethod.parse(request.getMethod()));
         }
 
-        if (invocation.getProxy().isMethodSpecified()) {
-            Method method = action.getClass().getMethod(invocation.getProxy().getMethod());
-            if (AnnotationUtils.isAnnotatedBy(method, HTTP_METHOD_ANNOTATIONS)) {
+        Method method = resolveInvokedMethod(invocation);
+        if (method != null) {
+            AnnotatedElement methodElement = findNearestAnnotatedOverride(action.getClass(), method);
+            if (methodElement != null) {
                 LOG.debug("Action's method: {} annotated with: {}, checking if request: {} meets allowed methods!",
                     invocation.getProxy().getMethod(), AllowedHttpMethod.class.getSimpleName(), request.getMethod());
-                return doIntercept(invocation, method);
+                return doIntercept(invocation, methodElement);
             }
         }
-        if (AnnotationUtils.isAnnotatedBy(action.getClass(), HTTP_METHOD_ANNOTATIONS)) {
+        AnnotatedElement classElement = findNearestAnnotatedType(action.getClass());
+        if (classElement != null) {
             LOG.debug("Action: {} annotated with: {}, checking if request: {} meets allowed methods!",
                 action, AllowedHttpMethod.class.getSimpleName(), request.getMethod());
-            return doIntercept(invocation, action.getClass());
+            return doIntercept(invocation, classElement);
         }
 
         Set<HttpMethod> configuredMethods = invocation.getProxy().getConfig().getHttpMethods();
@@ -133,6 +143,83 @@ protected String doIntercept(ActionInvocation invocation, AnnotatedElement eleme
         return checkAllowedMethods(invocation, allowedMethods);
     }
 
+    private Method resolveInvokedMethod(ActionInvocation invocation) throws NoSuchMethodException {
+        String methodName = invocation.getProxy().getMethod();
+        if (methodName == null || methodName.isEmpty()) {
+            return null;
+        }
+        try {
+            return invocation.getAction().getClass().getMethod(methodName);
+        } catch (NoSuchMethodException e) {
+            if (invocation.getProxy().isMethodSpecified()) {
+                throw e;
+            }
+            // An absent implicit execute() can be handled by an UnknownHandler during invocation.
+            return null;
+        }
+    }
+
+    /** Resolves the invoked method first, then annotated declarations at each class and its interfaces. */
+    private AnnotatedElement findNearestAnnotatedOverride(Class<?> actionClass, Method method) {
+        if (!isPublicInstanceMethod(method)) {
+            return null;
+        }
+        if (AnnotationUtils.isAnnotatedBy(method, HTTP_METHOD_ANNOTATIONS)) {
+            return method;
+        }
+        for (Class<?> type = actionClass; type != null; type = type.getSuperclass()) {
+            AnnotatedElement found = declaredOverrideAnnotatedIn(type, method);
+            if (found != null) {
+                return found;
+            }
+            for (Class<?> iface : interfacesAtLevel(type)) {
+                found = declaredOverrideAnnotatedIn(iface, method);
+                if (found != null) {
+                    return found;
+                }
+            }
+        }
+        return null;
+    }
+
+    private AnnotatedElement declaredOverrideAnnotatedIn(Class<?> type, Method method) {
+        try {
+            Method candidate = type.getDeclaredMethod(method.getName(), method.getParameterTypes());
+            return isPublicInstanceMethod(candidate) && AnnotationUtils.isAnnotatedBy(candidate, HTTP_METHOD_ANNOTATIONS)
+                ? candidate : null;
+        } catch (NoSuchMethodException e) {
+            return null;
+        }
+    }
+
+    private boolean isPublicInstanceMethod(Method method) {
+        return Modifier.isPublic(method.getModifiers()) && !Modifier.isStatic(method.getModifiers());
+    }
+
+    /** Checks each class before its interfaces, including their superinterfaces, then its superclass. */
+    private AnnotatedElement findNearestAnnotatedType(Class<?> actionClass) {
+        for (Class<?> type = actionClass; type != null; type = type.getSuperclass()) {
+            if (AnnotationUtils.isAnnotatedBy(type, HTTP_METHOD_ANNOTATIONS)) {
+                return type;
+            }
+            for (Class<?> iface : interfacesAtLevel(type)) {
+                if (AnnotationUtils.isAnnotatedBy(iface, HTTP_METHOD_ANNOTATIONS)) {
+                    return iface;
+                }
+            }
+        }
+        return null;
+    }
+
+    private Set<Class<?>> interfacesAtLevel(Class<?> type) {
+        Set<Class<?>> interfaces = new LinkedHashSet<>();
+        for (Class<?> iface : type.getInterfaces()) {
+            interfaces.add(iface);
+            interfaces.addAll(ClassUtils.getAllInterfaces(iface));
+        }
+        return interfaces;
+    }
+
     private String checkAllowedMethods(ActionInvocation invocation, Set<HttpMethod> allowedMethods) throws Exception {
         HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
         String requestedMethod = request.getMethod();

core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodDefaultStacksTest.java

@@ -0,0 +1,59 @@
+/*
+ * 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.interceptor.httpmethod;
+
+import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.config.entities.InterceptorMapping;
+import org.apache.struts2.config.entities.InterceptorStackConfig;
+import org.apache.struts2.config.entities.PackageConfig;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/** Every bundled core stack that binds parameters also enforces HTTP method restrictions. */
+public class HttpMethodDefaultStacksTest extends StrutsInternalTestCase {
+
+    public void testEveryStackWithParamsAlsoHasHttpMethod() {
+        PackageConfig pc = configuration.getPackageConfig("struts-default");
+        Map<String, InterceptorStackConfig> stacks = allStacks(pc);
+        assertTrue("expected to find bundled interceptor-stacks", stacks.size() > 5);
+
+        List<String> violations = new ArrayList<>();
+        for (Map.Entry<String, InterceptorStackConfig> e : stacks.entrySet()) {
+            String name = e.getKey();
+            List<String> names = e.getValue().getInterceptors().stream().map(InterceptorMapping::getName).toList();
+            if (names.contains("params") && !names.contains("httpMethod")) {
+                violations.add(name);
+            }
+        }
+        assertTrue("stacks binding params but not enforcing httpMethod: " + violations, violations.isEmpty());
+    }
+
+    private Map<String, InterceptorStackConfig> allStacks(PackageConfig pc) {
+        Map<String, Object> all = pc.getAllInterceptorConfigs();
+        Map<String, InterceptorStackConfig> out = new java.util.LinkedHashMap<>();
+        for (Map.Entry<String, Object> e : all.entrySet()) {
+            if (e.getValue() instanceof InterceptorStackConfig stackConfig) {
+                out.put(e.getKey(), stackConfig);
+            }
+        }
+        return out;
+    }
+}

core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodHierarchyActions.java

@@ -0,0 +1,65 @@
+/*
+ * 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.interceptor.httpmethod;
+
+import org.apache.struts2.ActionSupport;
+import org.apache.struts2.interceptor.httpmethod.HttpPost;
+
+public class HttpMethodHierarchyActions {
+    public static final java.util.concurrent.atomic.AtomicInteger RAN = new java.util.concurrent.atomic.AtomicInteger();
+
+    public static class ExecutePost extends ActionSupport {
+        @HttpPost
+        @Override
+        public String execute() { RAN.incrementAndGet(); return SUCCESS; }
+    }
+    public static class NamedPost extends ActionSupport {
+        @HttpPost
+        public String save() { RAN.incrementAndGet(); return SUCCESS; }
+    }
+    @HttpPost
+    public static class ClassPost extends ActionSupport {
+        @Override
+        public String execute() { RAN.incrementAndGet(); return SUCCESS; }
+    }
+    public static class SubOfClassPost extends ClassPost { }
+    public static class BaseMethodPost extends ActionSupport {
+        @HttpPost
+        public String save() { RAN.incrementAndGet(); return SUCCESS; }
+    }
+    public static class SubOverride extends BaseMethodPost {
+        @Override
+        public String save() { RAN.incrementAndGet(); return SUCCESS; }
+    }
+
+    public interface MethodPostMarker {
+        @HttpPost
+        default String save() { HttpMethodHierarchyActions.RAN.incrementAndGet(); return ActionSupport.SUCCESS; }
+    }
+    public static class ImplementsMethodMarker extends ActionSupport implements MethodPostMarker { }
+    public static class SubOfImplementsMethodMarker extends ImplementsMethodMarker { }
+
+    @HttpPost
+    public interface ClassPostMarker { }
+    public static class ImplementsClassMarker extends ActionSupport implements ClassPostMarker {
+        @Override
+        public String execute() { RAN.incrementAndGet(); return SUCCESS; }
+    }
+    public static class SubOfImplementsClassMarker extends ImplementsClassMarker { }
+}

core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodHierarchyTest.java

@@ -0,0 +1,88 @@
+/*
+ * 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.interceptor.httpmethod;
+
+import org.apache.struts2.ActionContext;
+import org.apache.struts2.ActionProxy;
+import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.config.StrutsXmlConfigurationProvider;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+
+import java.util.Map;
+
+public class HttpMethodHierarchyTest extends StrutsInternalTestCase {
+
+    private MockHttpServletResponse run(String action, String verb) throws Exception {
+        loadConfigurationProviders(new StrutsXmlConfigurationProvider("struts-http-method-hierarchy.xml"));
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        Map<String, Object> context = ActionContext.of()
+            .withServletRequest(new MockHttpServletRequest(verb, "/" + action))
+            .withServletResponse(response).getContextMap();
+        HttpMethodHierarchyActions.RAN.set(0);
+        ActionProxy proxy = actionProxyFactory.createActionProxy("", action, null, context, false, true);
+        proxy.execute();
+        return response;
+    }
+
+    private void assertGetRejected(String action) throws Exception {
+        MockHttpServletResponse response = run(action, "GET");
+        assertEquals(405, response.getStatus());
+        assertEquals("POST", response.getHeader("Allow"));
+        assertEquals("action must not run", 0, HttpMethodHierarchyActions.RAN.get());
+    }
+
+    public void testControlNamedMethodGetRejected() throws Exception {
+        assertGetRejected("NamedPost");
+    }
+
+    public void testControlClassLevelGetRejected() throws Exception {
+        assertGetRejected("ClassPost");
+    }
+
+    public void testExecuteMethodAnnotationGetRejected() throws Exception {
+        assertGetRejected("ExecutePost");
+    }
+
+    public void testSubclassOfClassLevelGetRejected() throws Exception {
+        assertGetRejected("SubOfClassPost");
+    }
+
+    public void testOverriddenMethodRetainsAnnotation() throws Exception {
+        assertGetRejected("SubOverride");
+    }
+
+    public void testSubclassOfInterfaceDefaultMethodGetRejected() throws Exception {
+        assertGetRejected("SubOfImplementsMethodMarker");
+    }
+
+    public void testSubclassOfImplementedInterfaceClassLevelGetRejected() throws Exception {
+        assertGetRejected("SubOfImplementsClassMarker");
+    }
+
+    public void testPostAllowedForEveryAnnotatedActionShape() throws Exception {
+        for (String action : new String[]{"NamedPost", "ClassPost", "ExecutePost", "SubOfClassPost",
+                "SubOverride", "SubOfImplementsMethodMarker", "SubOfImplementsClassMarker"}) {
+            MockHttpServletResponse response = run(action, "POST");
+            assertEquals(action, 200, response.getStatus());
+            assertNull(action, response.getHeader("Allow"));
+            assertEquals(action, 1, HttpMethodHierarchyActions.RAN.get());
+        }
+    }
+}