You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/02/22 16:00:38 UTC
(struts) branch fix/WW-5535-http-method-interceptor-wildcard created (now e6ad2951a)
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a change to branch fix/WW-5535-http-method-interceptor-wildcard
in repository https://gitbox.apache.org/repos/asf/struts.git
at e6ad2951a fix(core): correct isMethodSpecified() for wildcard-resolved methods
This branch includes the following new commits:
new e6ad2951a fix(core): correct isMethodSpecified() for wildcard-resolved methods
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(struts) 01/01: fix(core): correct isMethodSpecified() for wildcard-resolved methods
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5535-http-method-interceptor-wildcard
in repository https://gitbox.apache.org/repos/asf/struts.git
commit e6ad2951a8526e32df890a107d0b273f30546aa5
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Sun Feb 22 17:00:25 2026 +0100
fix(core): correct isMethodSpecified() for wildcard-resolved methods
DefaultActionProxy.resolveMethod() unconditionally set methodSpecified=false
when the method was not passed explicitly, including when it was resolved from
ActionConfig (e.g., wildcard substitution like method="{1}"). This caused
HttpMethodInterceptor to skip method-level annotation checks for wildcard
actions, falling back to class-level annotations instead.
Move methodSpecified=false inside the inner branch that defaults to "execute",
so config-resolved methods (including wildcard-substituted ones) correctly
report isMethodSpecified()=true. Update Javadoc to reflect the corrected
semantics.
Fixes [WW-5535](https://issues.apache.org/jira/browse/WW-5535)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <no...@anthropic.com>
---
.../main/java/org/apache/struts2/ActionProxy.java | 11 +-
.../org/apache/struts2/DefaultActionProxy.java | 14 +-
.../org/apache/struts2/DefaultActionProxyTest.java | 60 +++++--
.../httpmethod/HttpMethodInterceptorTest.java | 74 +++++++++
.../providers/xwork-test-allowed-methods.xml | 14 +-
...-22-WW-5535-http-method-interceptor-wildcard.md | 174 +++++++++++++++++++++
6 files changed, 328 insertions(+), 19 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/ActionProxy.java b/core/src/main/java/org/apache/struts2/ActionProxy.java
index 94b816bf8..e03c73f1f 100644
--- a/core/src/main/java/org/apache/struts2/ActionProxy.java
+++ b/core/src/main/java/org/apache/struts2/ActionProxy.java
@@ -93,9 +93,16 @@ public interface ActionProxy {
String getMethod();
/**
- * Gets status of the method value's initialization.
+ * Returns whether the action method was explicitly specified rather than defaulting to {@code "execute"}.
+ * <p>
+ * This returns {@code true} when the method was provided via the URL (DMI), passed as a constructor argument,
+ * or resolved from the action configuration (including wildcard-substituted values like {@code method="{1}"}).
+ * It returns {@code false} only when no method was specified anywhere and the framework fell back
+ * to the default {@code "execute"} method.
+ * </p>
*
- * @return true if the method returned by getMethod() is not a default initializer value.
+ * @return {@code true} if the method was explicitly provided or resolved from config;
+ * {@code false} only when defaulting to {@code "execute"}
*/
boolean isMethodSpecified();
diff --git a/core/src/main/java/org/apache/struts2/DefaultActionProxy.java b/core/src/main/java/org/apache/struts2/DefaultActionProxy.java
index 43fb695ec..50d273ed0 100644
--- a/core/src/main/java/org/apache/struts2/DefaultActionProxy.java
+++ b/core/src/main/java/org/apache/struts2/DefaultActionProxy.java
@@ -71,14 +71,14 @@ public class DefaultActionProxy implements ActionProxy, Serializable {
* <p>
* The reason for the builder methods is so that you can use a subclass to create your own DefaultActionProxy instance
* </p>
- *
+ * <p>
* (like a RMIActionProxy).
*
- * @param inv the action invocation
- * @param namespace the namespace
- * @param actionName the action name
- * @param methodName the method name
- * @param executeResult execute result
+ * @param inv the action invocation
+ * @param namespace the namespace
+ * @param actionName the action name
+ * @param methodName the method name
+ * @param executeResult execute result
* @param cleanupContext cleanup context
*/
protected DefaultActionProxy(ActionInvocation inv, String namespace, String actionName, String methodName, boolean executeResult, boolean cleanupContext) {
@@ -171,8 +171,8 @@ public class DefaultActionProxy implements ActionProxy, Serializable {
this.method = config.getMethodName();
if (StringUtils.isEmpty(this.method)) {
this.method = ActionConfig.DEFAULT_METHOD;
+ methodSpecified = false;
}
- methodSpecified = false;
}
}
diff --git a/core/src/test/java/org/apache/struts2/DefaultActionProxyTest.java b/core/src/test/java/org/apache/struts2/DefaultActionProxyTest.java
index 8484fac5a..902eaacf1 100644
--- a/core/src/test/java/org/apache/struts2/DefaultActionProxyTest.java
+++ b/core/src/test/java/org/apache/struts2/DefaultActionProxyTest.java
@@ -18,25 +18,67 @@
*/
package org.apache.struts2;
-import org.apache.struts2.mock.MockActionInvocation;
-import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.config.ConfigurationException;
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
-import org.junit.Test;
+import org.apache.struts2.mock.MockActionInvocation;
public class DefaultActionProxyTest extends StrutsInternalTestCase {
- @Test
- public void testThorwExceptionOnNotAllowedMethod() throws Exception {
- final String filename = "org/apache/struts2/config/providers/xwork-test-allowed-methods.xml";
- loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
+ private static final String CONFIG = "org/apache/struts2/config/providers/xwork-test-allowed-methods.xml";
+
+ public void testThrowExceptionOnNotAllowedMethod() {
+ loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "strict", "Default", "notAllowed", true, true);
container.inject(dap);
try {
dap.prepare();
fail("Must throw exception!");
- } catch (Exception e) {
- assertEquals(e.getMessage(), "Method notAllowed for action Default is not allowed!");
+ } catch (ConfigurationException e) {
+ assertEquals("Method notAllowed for action Default is not allowed!", e.getMessage());
}
}
+
+ public void testMethodSpecifiedWhenPassedExplicitly() {
+ loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
+ DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "Default", "input", true, true);
+ container.inject(dap);
+ dap.prepare();
+
+ assertTrue("Method should be specified when passed as constructor argument", dap.isMethodSpecified());
+ assertEquals("input", dap.getMethod());
+ }
+
+ public void testMethodSpecifiedWhenResolvedFromConfig() {
+ loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
+ // ConfigMethod action has method="onPostOnly" in XML config, no method passed in constructor
+ DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "ConfigMethod", null, true, true);
+ container.inject(dap);
+ dap.prepare();
+
+ assertTrue("Method should be specified when resolved from action config", dap.isMethodSpecified());
+ assertEquals("onPostOnly", dap.getMethod());
+ }
+
+ public void testMethodNotSpecifiedWhenDefaultingToExecute() {
+ loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
+ // NoMethod action has no method in XML config and no method passed in constructor
+ DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "NoMethod", null, true, true);
+ container.inject(dap);
+ dap.prepare();
+
+ assertFalse("Method should not be specified when defaulting to execute", dap.isMethodSpecified());
+ assertEquals("execute", dap.getMethod());
+ }
+
+ public void testMethodSpecifiedWithWildcardAction() {
+ loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
+ // Wild-onPostOnly matches Wild-* with method="{1}" -> resolves to "onPostOnly"
+ DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "Wild-onPostOnly", null, true, true);
+ container.inject(dap);
+ dap.prepare();
+
+ assertTrue("Method should be specified when resolved from wildcard config", dap.isMethodSpecified());
+ assertEquals("onPostOnly", dap.getMethod());
+ }
}
\ No newline at end of file
diff --git a/core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptorTest.java
index 8c7aa3cab..5f29681df 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptorTest.java
@@ -217,6 +217,80 @@ public class HttpMethodInterceptorTest extends StrutsInternalTestCase {
assertEquals(HttpMethod.POST, action.getHttpMethod());
}
+ /**
+ * Simulates a wildcard action like {@code <action name="Wild-*" method="{1}">}
+ * resolving to method "onPostOnly" (annotated with @HttpPost).
+ * With the fix in DefaultActionProxy.resolveMethod(), config-resolved methods
+ * set isMethodSpecified()=true, so the interceptor checks method-level annotations.
+ * A GET request should be rejected because @HttpPost only allows POST.
+ */
+ public void testWildcardResolvedMethodWithPostAnnotationRejectsGet() throws Exception {
+ // given
+ HttpMethodsTestAction action = new HttpMethodsTestAction();
+ prepareActionInvocation(action);
+ // Simulate wildcard resolution: Wild-onPostOnly -> method="onPostOnly"
+ actionProxy.setMethod("onPostOnly");
+ // After the fix, config-resolved methods have methodSpecified=true
+ actionProxy.setMethodSpecified(true);
+
+ invocation.setResultCode("onPostOnly");
+
+ prepareRequest("get");
+
+ // when
+ String resultName = interceptor.intercept(invocation);
+
+ // then - interceptor checks method-level @HttpPost and rejects GET
+ assertEquals("bad-request", resultName);
+ }
+
+ /**
+ * Counterpart: same wildcard scenario but with POST request — should succeed.
+ */
+ public void testWildcardResolvedMethodWithPostAnnotationAllowsPost() throws Exception {
+ // given
+ HttpMethodsTestAction action = new HttpMethodsTestAction();
+ prepareActionInvocation(action);
+ actionProxy.setMethod("onPostOnly");
+ actionProxy.setMethodSpecified(true);
+
+ invocation.setResultCode("onPostOnly");
+
+ prepareRequest("post");
+
+ // when
+ String resultName = interceptor.intercept(invocation);
+
+ // then - interceptor checks method-level @HttpPost and allows POST
+ assertEquals("onPostOnly", resultName);
+ }
+
+ /**
+ * Before the fix: when methodSpecified=false (bug), the interceptor falls through
+ * to class-level @AllowedHttpMethod(POST), which also rejects GET — but for the
+ * wrong reason (checking class-level instead of method-level annotation).
+ * This test verifies the interceptor uses method-level annotations when methodSpecified=true.
+ */
+ public void testWildcardResolvedMethodGetAnnotationAllowsGet() throws Exception {
+ // given
+ HttpMethodsTestAction action = new HttpMethodsTestAction();
+ prepareActionInvocation(action);
+ // Simulate wildcard resolution to onGetOnly (annotated with @HttpGet)
+ actionProxy.setMethod("onGetOnly");
+ actionProxy.setMethodSpecified(true);
+
+ invocation.setResultCode("onGetOnly");
+
+ prepareRequest("get");
+
+ // when
+ String resultName = interceptor.intercept(invocation);
+
+ // then - method-level @HttpGet allows GET (class-level @AllowedHttpMethod(POST) would reject it)
+ assertEquals("onGetOnly", resultName);
+ assertEquals(HttpMethod.GET, action.getHttpMethod());
+ }
+
private void prepareActionInvocation(Object action) {
interceptor = new HttpMethodInterceptor();
invocation = new MockActionInvocation();
diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-allowed-methods.xml b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-allowed-methods.xml
index 247b4ef27..0c78ff5cf 100644
--- a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-allowed-methods.xml
+++ b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-allowed-methods.xml
@@ -29,7 +29,7 @@
</action>
<action name="Boring">
- <allowed-methods> </allowed-methods>
+ <allowed-methods></allowed-methods>
</action>
<action name="Foo">
@@ -43,6 +43,18 @@
<action name="Baz" method="baz">
<allowed-methods>foo,bar</allowed-methods>
</action>
+
+ <action name="Wild-*" class="org.apache.struts2.HttpMethodsTestAction" method="{1}">
+ <allowed-methods>regex:.*</allowed-methods>
+ </action>
+
+ <action name="ConfigMethod" class="org.apache.struts2.HttpMethodsTestAction" method="onPostOnly">
+ <allowed-methods>regex:.*</allowed-methods>
+ </action>
+
+ <action name="NoMethod" class="org.apache.struts2.HttpMethodsTestAction">
+ <allowed-methods>regex:.*</allowed-methods>
+ </action>
</package>
<package name="strict" strict-method-invocation="true">
diff --git a/thoughts/shared/research/2026-02-22-WW-5535-http-method-interceptor-wildcard.md b/thoughts/shared/research/2026-02-22-WW-5535-http-method-interceptor-wildcard.md
new file mode 100644
index 000000000..cff2a8928
--- /dev/null
+++ b/thoughts/shared/research/2026-02-22-WW-5535-http-method-interceptor-wildcard.md
@@ -0,0 +1,174 @@
+---
+date: 2026-02-22T12:00:00+01:00
+topic: "HttpMethodInterceptor does not work with action names using wildcards"
+tags: [research, codebase, HttpMethodInterceptor, DefaultActionProxy, wildcard, isMethodSpecified, security]
+status: complete
+git_commit: a21c763d8a8592f1056086134414123f6d8d168d
+---
+
+# Research: WW-5535 — HttpMethodInterceptor does not work with wildcard action names
+
+**Date**: 2026-02-22
+
+## Research Question
+
+Why does `HttpMethodInterceptor` fail to enforce HTTP method validation when actions use wildcard names like
+`example-*`?
+
+## Summary
+
+The root cause is in `DefaultActionProxy.resolveMethod()`. For wildcard-matched actions, the method name is resolved
+from the `ActionConfig` (after wildcard substitution) rather than being passed explicitly from the URL. Because
+`resolveMethod()` treats any method resolved from config as "not specified", `isMethodSpecified()` returns `false`. This
+causes `HttpMethodInterceptor` to check **class-level** annotations instead of **method-level** annotations, potentially
+skipping validation entirely.
+
+## Detailed Findings
+
+### 1. The `methodSpecified` Flag Logic
+
+**File**: [
+`DefaultActionProxy.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/DefaultActionProxy.java)
+
+```java
+private boolean methodSpecified = true; // field default
+
+private void resolveMethod() {
+ if (StringUtils.isEmpty(this.method)) {
+ this.method = config.getMethodName();
+ if (StringUtils.isEmpty(this.method)) {
+ this.method = ActionConfig.DEFAULT_METHOD;
+ }
+ methodSpecified = false; // <-- ONLY path that sets it false
+ }
+}
+```
+
+The flag is set to `false` whenever the method was not passed explicitly to the proxy constructor. This conflates two
+different concepts:
+
+- **"Was the method specified by the user via DMI?"** (security concern — user-controlled method invocation)
+- **"Is a non-default method being invoked?"** (what `HttpMethodInterceptor` needs to know)
+
+### 2. Wildcard Resolution Flow
+
+For a config like `<action name="example-*" class="ExampleAction" method="do{1}">` and URL `example-create`:
+
+| Step | Component | Result |
+|------|----------------------------------------------|-------------------------------------------------------------------------------------------------------------|
+| 1 | `DefaultActionMapper.extractMethodName()` | `ActionMapping.method = null` (exact map lookup fails for wildcards) |
+| 2 | `Dispatcher.serviceAction()` | Passes `null` method to `ActionProxyFactory` |
+| 3 | `DefaultActionProxy` constructor | `this.method = null` |
+| 4 | `RuntimeConfigurationImpl.getActionConfig()` | Triggers `ActionConfigMatcher.match()` → `convert()` substitutes `{1}` → `config.methodName = "docreate"` |
+| 5 | `DefaultActionProxy.resolveMethod()` | `this.method` is empty → takes `config.getMethodName()` = `"docreate"` → **sets `methodSpecified = false`** |
+
+**Key file**: [
+`ActionConfigMatcher.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java) —
+`convert()` performs `{n}` substitution on method name.
+
+**Key file**: [
+`DefaultActionMapper.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java) —
+`extractMethodName()` uses `cfg.getActionConfigs().get(mapping.getName())` which is an exact map lookup that cannot
+match wildcard patterns.
+
+### 3. HttpMethodInterceptor Behavior
+
+**File**: [
+`HttpMethodInterceptor.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java)
+
+```java
+if (invocation.getProxy().isMethodSpecified()) {
+ // Check METHOD-LEVEL annotations (e.g., @HttpGet on the specific method)
+ Method method = action.getClass().getMethod(invocation.getProxy().getMethod());
+ if (AnnotationUtils.isAnnotatedBy(method, HTTP_METHOD_ANNOTATIONS)) {
+ return doIntercept(invocation, method);
+ }
+} else if (AnnotationUtils.isAnnotatedBy(action.getClass(), HTTP_METHOD_ANNOTATIONS)) {
+ // Check CLASS-LEVEL annotations only
+ return doIntercept(invocation, action.getClass());
+}
+// No annotations → allow request through (no validation)
+```
+
+**Impact for wildcard actions**: Since `isMethodSpecified()` returns `false`, the interceptor checks class-level
+annotations. If the action class has no class-level HTTP method annotations (only method-level ones), validation is *
+*skipped entirely**.
+
+### 4. The Semantic Mismatch
+
+The `ActionProxy.isMethodSpecified()` Javadoc says:
+
+> Returns true if the method returned by `getMethod()` is not a default initializer value.
+
+The current implementation interprets "default initializer" as "anything not explicitly passed from the URL/DMI", which
+includes wildcard-configured methods. But for `HttpMethodInterceptor`, what matters is whether a *specific* method (not
+`execute`) is being invoked, regardless of how it was determined.
+
+## Code References
+
+- [
+ `DefaultActionProxy.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/DefaultActionProxy.java) —
+ `resolveMethod()` and `methodSpecified` field
+- [
+ `ActionProxy.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/ActionProxy.java) —
+ `isMethodSpecified()` interface and Javadoc
+- [
+ `HttpMethodInterceptor.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java) —
+ `intercept()` branching on `isMethodSpecified()`
+- [
+ `ActionConfigMatcher.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java) —
+ wildcard `{n}` substitution in `convert()`
+- [
+ `DefaultActionMapper.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java) —
+ `extractMethodName()` exact-match lookup
+- [
+ `DefaultConfiguration.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java) —
+ `findActionConfigInNamespace()` wildcard fallback
+- [
+ `HttpMethodInterceptorTest.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/test/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptorTest.java) —
+ existing tests
+- [
+ `DefaultActionProxyTest.java`](https://github.com/apache/struts/blob/a21c763d8a8592f1056086134414123f6d8d168d/core/src/test/java/org/apache/struts2/DefaultActionProxyTest.java) —
+ only tests disallowed method, not `isMethodSpecified()`
+
+## Architecture Insights
+
+The `isMethodSpecified()` flag was originally introduced (WW-3628) to distinguish user-controlled DMI method invocation
+from default `execute()` dispatch. This was a security measure — DMI-specified methods need stricter validation.
+
+However, the flag now serves double duty:
+
+1. **Security gate** in DMI handling — was the method user-specified via URL?
+2. **Dispatch hint** in `HttpMethodInterceptor` — should we check method-level or class-level annotations?
+
+These two concerns have different semantics for wildcard actions. A wildcard-configured method like `do{1}` is **not
+user-controlled** (it's defined in the config), but it **is a specific method** that should have its annotations
+checked.
+
+## Potential Fix Approaches
+
+1. **Fix in `HttpMethodInterceptor`**: Instead of relying on `isMethodSpecified()`, check method-level annotations
+ first, then fall back to class-level. This avoids changing the `ActionProxy` contract.
+
+2. **Fix in `DefaultActionProxy.resolveMethod()`**: Set `methodSpecified = true` when the method is resolved from
+ config (not just defaulted to `execute`). This changes the semantics of the flag but aligns with the Javadoc ("not a
+ default initializer value").
+
+3. **Add a new flag**: Introduce `isMethodFromConfig()` or similar to distinguish "method from wildcard config" from "
+ method from DMI" from "default execute". This is the most precise but highest-impact change.
+
+## Test Gaps
+
+- No tests for `DefaultActionProxy` with wildcard-resolved action names
+- No tests for `isMethodSpecified()` on a real `DefaultActionProxy` instance (all tests use `MockActionProxy`)
+- No tests for `HttpMethodInterceptor` combined with wildcard-resolved methods
+
+## Historical Context (from thoughts/)
+
+No prior research documents found for WW-5535 or WW-3628.
+
+## Open Questions
+
+1. Are there other interceptors or components that depend on `isMethodSpecified()` semantics?
+2. Would changing `methodSpecified` behavior for config-resolved methods break DMI security checks?
+3. Should `DefaultActionMapper.extractMethodName()` be updated to also resolve wildcard-matched configs?