mirror of
https://github.com/apache/struts.git
synced 2026-08-05 22:56:59 +00:00
fix(core): WW-5535 enforce HTTP method annotations for wildcard actions (#1593)
DefaultActionProxy.resolveMethod() incorrectly set methodSpecified=false for config-resolved methods (including wildcard-substituted ones), causing HttpMethodInterceptor to skip method-level @HttpPost/@HttpGet annotation checks. Move methodSpecified=false inside the inner if block so it only applies when truly defaulting to "execute". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -168,8 +168,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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -93,9 +93,11 @@ public interface ActionProxy {
|
||||
String getMethod();
|
||||
|
||||
/**
|
||||
* Gets status of the method value's initialization.
|
||||
* Gets status of the method value's initialization. Returns {@code true} when the method was explicitly provided
|
||||
* (e.g. via URL parameter, wildcard substitution, or action configuration), and {@code false} only when the
|
||||
* framework defaults to {@code "execute"} because no method was specified anywhere.
|
||||
*
|
||||
* @return true if the method returned by getMethod() is not a default initializer value.
|
||||
* @return true if the method returned by getMethod() is not the default "execute" fallback.
|
||||
*/
|
||||
boolean isMethodSpecified();
|
||||
|
||||
|
||||
@@ -18,15 +18,14 @@
|
||||
*/
|
||||
package com.opensymphony.xwork2;
|
||||
|
||||
import com.opensymphony.xwork2.config.ConfigurationException;
|
||||
import com.opensymphony.xwork2.mock.MockActionInvocation;
|
||||
import org.apache.struts2.StrutsInternalTestCase;
|
||||
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
|
||||
import org.junit.Test;
|
||||
|
||||
public class DefaultActionProxyTest extends StrutsInternalTestCase {
|
||||
|
||||
@Test
|
||||
public void testThorwExceptionOnNotAllowedMethod() throws Exception {
|
||||
public void testThrowExceptionOnNotAllowedMethod() {
|
||||
final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml";
|
||||
loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
|
||||
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "strict", "Default", "notAllowed", true, true);
|
||||
@@ -35,8 +34,52 @@ public class DefaultActionProxyTest extends StrutsInternalTestCase {
|
||||
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() {
|
||||
final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml";
|
||||
loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
|
||||
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "", "NoMethod", "onPostOnly", true, true);
|
||||
container.inject(dap);
|
||||
dap.prepare();
|
||||
|
||||
assertTrue("Method passed explicitly should be marked as specified", dap.isMethodSpecified());
|
||||
assertEquals("onPostOnly", dap.getMethod());
|
||||
}
|
||||
|
||||
public void testMethodSpecifiedWhenResolvedFromConfig() {
|
||||
final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml";
|
||||
loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
|
||||
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "", "ConfigMethod", null, true, true);
|
||||
container.inject(dap);
|
||||
dap.prepare();
|
||||
|
||||
assertTrue("Method resolved from action config should be marked as specified", dap.isMethodSpecified());
|
||||
assertEquals("onPostOnly", dap.getMethod());
|
||||
}
|
||||
|
||||
public void testMethodNotSpecifiedWhenDefaultingToExecute() {
|
||||
final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml";
|
||||
loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
|
||||
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "", "NoMethod", null, true, true);
|
||||
container.inject(dap);
|
||||
dap.prepare();
|
||||
|
||||
assertFalse("Method defaulting to execute should not be marked as specified", dap.isMethodSpecified());
|
||||
assertEquals("execute", dap.getMethod());
|
||||
}
|
||||
|
||||
public void testMethodSpecifiedWithWildcardAction() {
|
||||
final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml";
|
||||
loadConfigurationProviders(new StrutsXmlConfigurationProvider(filename));
|
||||
DefaultActionProxy dap = new DefaultActionProxy(new MockActionInvocation(), "", "Wild-onPostOnly", null, true, true);
|
||||
container.inject(dap);
|
||||
dap.prepare();
|
||||
|
||||
assertTrue("Method resolved from wildcard should be marked as specified", dap.isMethodSpecified());
|
||||
assertEquals("onPostOnly", dap.getMethod());
|
||||
}
|
||||
}
|
||||
+1
-1
@@ -42,7 +42,7 @@ public class XmlConfigurationProviderAllowedMethodsTest extends ConfigurationTes
|
||||
Map actionConfigs = pkg.getActionConfigs();
|
||||
|
||||
// assertions
|
||||
assertEquals(5, actionConfigs.size());
|
||||
assertEquals(8, actionConfigs.size());
|
||||
|
||||
ActionConfig action = (ActionConfig) actionConfigs.get("Default");
|
||||
assertEquals(1, action.getAllowedMethods().size());
|
||||
|
||||
+37
@@ -217,6 +217,43 @@ public class HttpMethodInterceptorTest extends StrutsInternalTestCase {
|
||||
assertEquals(HttpMethod.POST, action.getHttpMethod());
|
||||
}
|
||||
|
||||
public void testWildcardResolvedMethodWithPostAnnotationRejectsGet() throws Exception {
|
||||
// given
|
||||
HttpMethodsTestAction action = new HttpMethodsTestAction();
|
||||
prepareActionInvocation(action);
|
||||
actionProxy.setMethod("onPostOnly");
|
||||
actionProxy.setMethodSpecified(true);
|
||||
|
||||
invocation.setResultCode("onPostOnly");
|
||||
|
||||
prepareRequest("GET");
|
||||
|
||||
// when
|
||||
String resultName = interceptor.intercept(invocation);
|
||||
|
||||
// then
|
||||
assertEquals("bad-request", resultName);
|
||||
}
|
||||
|
||||
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
|
||||
assertEquals("onPostOnly", resultName);
|
||||
assertEquals(HttpMethod.POST, action.getHttpMethod());
|
||||
}
|
||||
|
||||
private void prepareActionInvocation(Object action) {
|
||||
interceptor = new HttpMethodInterceptor();
|
||||
invocation = new MockActionInvocation();
|
||||
|
||||
+13
-1
@@ -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">
|
||||
|
||||
Reference in New Issue
Block a user