Skip to content

Commit 803be76

Browse files
lukaszlenartclaude
andcommitted
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 <noreply@anthropic.com>
1 parent a21c763 commit 803be76

7 files changed

Lines changed: 329 additions & 20 deletions

File tree

core/src/main/java/org/apache/struts2/ActionProxy.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,16 @@ public interface ActionProxy {
9393
String getMethod();
9494

9595
/**
96-
* Gets status of the method value's initialization.
96+
* Returns whether the action method was explicitly specified rather than defaulting to {@code "execute"}.
97+
* <p>
98+
* This returns {@code true} when the method was provided via the URL (DMI), passed as a constructor argument,
99+
* or resolved from the action configuration (including wildcard-substituted values like {@code method="{1}"}).
100+
* It returns {@code false} only when no method was specified anywhere and the framework fell back
101+
* to the default {@code "execute"} method.
102+
* </p>
97103
*
98-
* @return true if the method returned by getMethod() is not a default initializer value.
104+
* @return {@code true} if the method was explicitly provided or resolved from config;
105+
* {@code false} only when defaulting to {@code "execute"}
99106
*/
100107
boolean isMethodSpecified();
101108

core/src/main/java/org/apache/struts2/DefaultActionProxy.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ public class DefaultActionProxy implements ActionProxy, Serializable {
7171
* <p>
7272
* The reason for the builder methods is so that you can use a subclass to create your own DefaultActionProxy instance
7373
* </p>
74-
*
74+
* <p>
7575
* (like a RMIActionProxy).
7676
*
77-
* @param inv the action invocation
78-
* @param namespace the namespace
79-
* @param actionName the action name
80-
* @param methodName the method name
81-
* @param executeResult execute result
77+
* @param inv the action invocation
78+
* @param namespace the namespace
79+
* @param actionName the action name
80+
* @param methodName the method name
81+
* @param executeResult execute result
8282
* @param cleanupContext cleanup context
8383
*/
8484
protected DefaultActionProxy(ActionInvocation inv, String namespace, String actionName, String methodName, boolean executeResult, boolean cleanupContext) {
@@ -171,8 +171,8 @@ private void resolveMethod() {
171171
this.method = config.getMethodName();
172172
if (StringUtils.isEmpty(this.method)) {
173173
this.method = ActionConfig.DEFAULT_METHOD;
174+
methodSpecified = false;
174175
}
175-
methodSpecified = false;
176176
}
177177
}
178178

core/src/test/java/org/apache/struts2/DefaultActionProxyTest.java

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,67 @@
1818
*/
1919
package org.apache.struts2;
2020

21-
import org.apache.struts2.mock.MockActionInvocation;
22-
import org.apache.struts2.StrutsInternalTestCase;
21+
import org.apache.struts2.config.ConfigurationException;
2322
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
24-
import org.junit.Test;
23+
import org.apache.struts2.mock.MockActionInvocation;
2524

2625
public class DefaultActionProxyTest extends StrutsInternalTestCase {
2726

28-
@Test
29-
public void testThorwExceptionOnNotAllowedMethod() throws Exception {
30-
final String filename = "org/apache/struts2/config/providers/xwork-test-allowed-methods.xml";
31-
loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
27+
private static final String CONFIG = "org/apache/struts2/config/providers/xwork-test-allowed-methods.xml";
28+
29+
public void testThrowExceptionOnNotAllowedMethod() {
30+
loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
3231
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "strict", "Default", "notAllowed", true, true);
3332
container.inject(dap);
3433

3534
try {
3635
dap.prepare();
3736
fail("Must throw exception!");
38-
} catch (Exception e) {
39-
assertEquals(e.getMessage(), "Method notAllowed for action Default is not allowed!");
37+
} catch (ConfigurationException e) {
38+
assertEquals("Method notAllowed for action Default is not allowed!", e.getMessage());
4039
}
4140
}
41+
42+
public void testMethodSpecifiedWhenPassedExplicitly() {
43+
loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
44+
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "Default", "input", true, true);
45+
container.inject(dap);
46+
dap.prepare();
47+
48+
assertTrue("Method should be specified when passed as constructor argument", dap.isMethodSpecified());
49+
assertEquals("input", dap.getMethod());
50+
}
51+
52+
public void testMethodSpecifiedWhenResolvedFromConfig() {
53+
loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
54+
// ConfigMethod action has method="onPostOnly" in XML config, no method passed in constructor
55+
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "ConfigMethod", null, true, true);
56+
container.inject(dap);
57+
dap.prepare();
58+
59+
assertTrue("Method should be specified when resolved from action config", dap.isMethodSpecified());
60+
assertEquals("onPostOnly", dap.getMethod());
61+
}
62+
63+
public void testMethodNotSpecifiedWhenDefaultingToExecute() {
64+
loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
65+
// NoMethod action has no method in XML config and no method passed in constructor
66+
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "NoMethod", null, true, true);
67+
container.inject(dap);
68+
dap.prepare();
69+
70+
assertFalse("Method should not be specified when defaulting to execute", dap.isMethodSpecified());
71+
assertEquals("execute", dap.getMethod());
72+
}
73+
74+
public void testMethodSpecifiedWithWildcardAction() {
75+
loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG));
76+
// Wild-onPostOnly matches Wild-* with method="{1}" -> resolves to "onPostOnly"
77+
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "default", "Wild-onPostOnly", null, true, true);
78+
container.inject(dap);
79+
dap.prepare();
80+
81+
assertTrue("Method should be specified when resolved from wildcard config", dap.isMethodSpecified());
82+
assertEquals("onPostOnly", dap.getMethod());
83+
}
4284
}

core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public void testDefaultAllowedMethods() throws ConfigurationException {
4242
Map actionConfigs = pkg.getActionConfigs();
4343

4444
// assertions
45-
assertEquals(5, actionConfigs.size());
45+
assertEquals(8, actionConfigs.size());
4646

4747
ActionConfig action = (ActionConfig) actionConfigs.get("Default");
4848
assertEquals(1, action.getAllowedMethods().size());

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,80 @@ public void testPostOnPutOrPostMethod() throws Exception {
217217
assertEquals(HttpMethod.POST, action.getHttpMethod());
218218
}
219219

220+
/**
221+
* Simulates a wildcard action like {@code <action name="Wild-*" method="{1}">}
222+
* resolving to method "onPostOnly" (annotated with @HttpPost).
223+
* With the fix in DefaultActionProxy.resolveMethod(), config-resolved methods
224+
* set isMethodSpecified()=true, so the interceptor checks method-level annotations.
225+
* A GET request should be rejected because @HttpPost only allows POST.
226+
*/
227+
public void testWildcardResolvedMethodWithPostAnnotationRejectsGet() throws Exception {
228+
// given
229+
HttpMethodsTestAction action = new HttpMethodsTestAction();
230+
prepareActionInvocation(action);
231+
// Simulate wildcard resolution: Wild-onPostOnly -> method="onPostOnly"
232+
actionProxy.setMethod("onPostOnly");
233+
// After the fix, config-resolved methods have methodSpecified=true
234+
actionProxy.setMethodSpecified(true);
235+
236+
invocation.setResultCode("onPostOnly");
237+
238+
prepareRequest("get");
239+
240+
// when
241+
String resultName = interceptor.intercept(invocation);
242+
243+
// then - interceptor checks method-level @HttpPost and rejects GET
244+
assertEquals("bad-request", resultName);
245+
}
246+
247+
/**
248+
* Counterpart: same wildcard scenario but with POST request — should succeed.
249+
*/
250+
public void testWildcardResolvedMethodWithPostAnnotationAllowsPost() throws Exception {
251+
// given
252+
HttpMethodsTestAction action = new HttpMethodsTestAction();
253+
prepareActionInvocation(action);
254+
actionProxy.setMethod("onPostOnly");
255+
actionProxy.setMethodSpecified(true);
256+
257+
invocation.setResultCode("onPostOnly");
258+
259+
prepareRequest("post");
260+
261+
// when
262+
String resultName = interceptor.intercept(invocation);
263+
264+
// then - interceptor checks method-level @HttpPost and allows POST
265+
assertEquals("onPostOnly", resultName);
266+
}
267+
268+
/**
269+
* Before the fix: when methodSpecified=false (bug), the interceptor falls through
270+
* to class-level @AllowedHttpMethod(POST), which also rejects GET — but for the
271+
* wrong reason (checking class-level instead of method-level annotation).
272+
* This test verifies the interceptor uses method-level annotations when methodSpecified=true.
273+
*/
274+
public void testWildcardResolvedMethodGetAnnotationAllowsGet() throws Exception {
275+
// given
276+
HttpMethodsTestAction action = new HttpMethodsTestAction();
277+
prepareActionInvocation(action);
278+
// Simulate wildcard resolution to onGetOnly (annotated with @HttpGet)
279+
actionProxy.setMethod("onGetOnly");
280+
actionProxy.setMethodSpecified(true);
281+
282+
invocation.setResultCode("onGetOnly");
283+
284+
prepareRequest("get");
285+
286+
// when
287+
String resultName = interceptor.intercept(invocation);
288+
289+
// then - method-level @HttpGet allows GET (class-level @AllowedHttpMethod(POST) would reject it)
290+
assertEquals("onGetOnly", resultName);
291+
assertEquals(HttpMethod.GET, action.getHttpMethod());
292+
}
293+
220294
private void prepareActionInvocation(Object action) {
221295
interceptor = new HttpMethodInterceptor();
222296
invocation = new MockActionInvocation();

core/src/test/resources/org/apache/struts2/config/providers/xwork-test-allowed-methods.xml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
</action>
3030

3131
<action name="Boring">
32-
<allowed-methods> </allowed-methods>
32+
<allowed-methods></allowed-methods>
3333
</action>
3434

3535
<action name="Foo">
@@ -43,6 +43,18 @@
4343
<action name="Baz" method="baz">
4444
<allowed-methods>foo,bar</allowed-methods>
4545
</action>
46+
47+
<action name="Wild-*" class="org.apache.struts2.HttpMethodsTestAction" method="{1}">
48+
<allowed-methods>regex:.*</allowed-methods>
49+
</action>
50+
51+
<action name="ConfigMethod" class="org.apache.struts2.HttpMethodsTestAction" method="onPostOnly">
52+
<allowed-methods>regex:.*</allowed-methods>
53+
</action>
54+
55+
<action name="NoMethod" class="org.apache.struts2.HttpMethodsTestAction">
56+
<allowed-methods>regex:.*</allowed-methods>
57+
</action>
4658
</package>
4759

4860
<package name="strict" strict-method-invocation="true">

0 commit comments

Comments
 (0)