diff --git a/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java index 53599638ab..b2088a2690 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.config.entities; +import org.apache.struts2.interceptor.httpmethod.HttpMethod; import org.apache.struts2.util.location.Located; import org.apache.struts2.util.location.Location; import org.apache.commons.lang3.StringUtils; @@ -26,6 +27,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -66,6 +68,7 @@ public class ActionConfig extends Located implements Serializable { protected String name; protected boolean strictMethodInvocation = true; protected AllowedMethods allowedMethods; + private Set httpMethods; protected ActionConfig(String packageName, String name, String className) { this.packageName = packageName; @@ -75,6 +78,7 @@ protected ActionConfig(String packageName, String name, String className) { results = new LinkedHashMap<>(); interceptors = new ArrayList<>(); exceptionMappings = new ArrayList<>(); + httpMethods = EnumSet.noneOf(HttpMethod.class); } /** @@ -92,6 +96,8 @@ protected ActionConfig(ActionConfig orig) { this.results = new LinkedHashMap<>(orig.results); this.exceptionMappings = new ArrayList<>(orig.exceptionMappings); this.allowedMethods = orig.allowedMethods; + this.httpMethods = EnumSet.noneOf(HttpMethod.class); + this.httpMethods.addAll(orig.httpMethods); this.location = orig.location; this.strictMethodInvocation = orig.strictMethodInvocation; } @@ -116,6 +122,14 @@ public Set getAllowedMethods() { return allowedMethods.list(); } + /** + * @return HTTP methods this action accepts, empty when it accepts any + * @since 7.5.0 + */ + public Set getHttpMethods() { + return httpMethods; + } + /** * Returns name of the action method * @@ -186,7 +200,7 @@ public boolean equals(Object o) { return false; } - return true; + return Objects.equals(httpMethods, actionConfig.httpMethods); } @Override @@ -201,6 +215,7 @@ public int hashCode() { result = 31 * result + (packageName != null ? packageName.hashCode() : 0); result = 31 * result + (name != null ? name.hashCode() : 0); result = 31 * result + (allowedMethods != null ? allowedMethods.hashCode() : 0); + result = 31 * result + (httpMethods != null ? httpMethods.hashCode() : 0); return result; } @@ -216,6 +231,9 @@ public String toString() { sb.append(")"); sb.append(" - ").append(location); sb.append(" - ").append(allowedMethods); + if (!httpMethods.isEmpty()) { + sb.append(" - ").append(httpMethods); + } sb.append("}"); return sb.toString(); } @@ -342,6 +360,14 @@ public Builder addAllowedMethod(Collection methods) { return this; } + /** + * @since 7.5.0 + */ + public Builder addHttpMethods(Collection methods) { + target.httpMethods.addAll(methods); + return this; + } + public Builder location(Location loc) { target.location = loc; return this; @@ -362,6 +388,7 @@ public ActionConfig build() { target.results = Collections.unmodifiableMap(target.results); target.interceptors = Collections.unmodifiableList(target.interceptors); target.exceptionMappings = Collections.unmodifiableList(target.exceptionMappings); + target.httpMethods = Collections.unmodifiableSet(target.httpMethods); target.allowedMethods = AllowedMethods.build(target.strictMethodInvocation, allowedMethods, methodRegex != null ? methodRegex : DEFAULT_METHOD_REGEX); ActionConfig result = target; diff --git a/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java b/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java index 0cd42f4f14..cdd7e7fa2c 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java +++ b/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java @@ -145,6 +145,7 @@ public ActionConfig convert(String path, ActionConfig orig, .addResultConfigs(results) .setStrictMethodInvocation(orig.isStrictMethodInvocation()) .addAllowedMethod(orig.getAllowedMethods()) + .addHttpMethods(orig.getHttpMethods()) .addInterceptors(orig.getInterceptors()) .addExceptionMappings(exs) .location(orig.getLocation()) diff --git a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java index 4f908bfb63..7d940275f7 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java @@ -93,6 +93,7 @@ import org.apache.struts2.ognl.accessor.CompoundRootAccessor; import org.apache.struts2.ognl.accessor.RootAccessor; import org.apache.struts2.ognl.accessor.XWorkMethodAccessor; +import org.apache.struts2.interceptor.httpmethod.HttpMethodInterceptor; import org.apache.struts2.interceptor.parameter.OgnlParameterAllowlister; import org.apache.struts2.interceptor.parameter.ParameterAllowlister; import org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer; @@ -559,6 +560,8 @@ private ActionConfig buildFullActionConfig(PackageConfig packageContext, ActionC } } + warnIfHttpMethodNotEnforced(baseConfig, interceptors); + String methodRegex = container.getInstance(String.class, StrutsConstants.STRUTS_SMI_METHOD_REGEX); if (methodRegex == null) { methodRegex = ActionConfig.DEFAULT_METHOD_REGEX; @@ -577,6 +580,13 @@ private ActionConfig buildFullActionConfig(PackageConfig packageContext, ActionC .build(); } + private void warnIfHttpMethodNotEnforced(ActionConfig baseConfig, List interceptors) { + if (!baseConfig.getHttpMethods().isEmpty() + && interceptors.stream().noneMatch(mapping -> mapping.getInterceptor() instanceof HttpMethodInterceptor)) { + LOG.warn("Action [{}] in package [{}] declares http-method {} but its interceptors do not include {}, the restriction will not be enforced", + baseConfig.getName(), baseConfig.getPackageName(), baseConfig.getHttpMethods(), HttpMethodInterceptor.class.getSimpleName()); + } + } private static class RuntimeConfigurationImpl implements RuntimeConfiguration { diff --git a/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java index a5547d9350..e7e7d40663 100644 --- a/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java @@ -39,6 +39,7 @@ import org.apache.struts2.inject.ContainerBuilder; import org.apache.struts2.inject.Inject; import org.apache.struts2.inject.Scope; +import org.apache.struts2.interceptor.httpmethod.HttpMethod; import org.apache.struts2.util.ClassLoaderUtil; import org.apache.struts2.util.DomHelper; import org.apache.struts2.util.location.LocatableProperties; @@ -58,10 +59,12 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.function.Consumer; @@ -499,6 +502,7 @@ protected ActionConfig buildActionConfig(Element actionElement, List interceptorList = buildInterceptorList(actionElement, packageContext); List exceptionMappings = buildExceptionMappings(actionElement); Set allowedMethods = buildAllowedMethods(actionElement, packageContext); + Set httpMethods = buildHttpMethods(actionElement, location); return new ActionConfig.Builder(packageContext.getName(), actionName, className) .methodName(methodName) @@ -508,10 +512,27 @@ protected ActionConfig buildActionConfig(Element actionElement, .addParams(XmlHelper.getParams(actionElement)) .setStrictMethodInvocation(packageContext.isStrictMethodInvocation()) .addAllowedMethod(allowedMethods) + .addHttpMethods(httpMethods) .location(location) .build(); } + /** + * @since 7.5.0 + */ + protected Set buildHttpMethods(Element actionElement, Location location) { + Set httpMethods = EnumSet.noneOf(HttpMethod.class); + for (String value : commaDelimitedStringToSet(actionElement.getAttribute("http-method"))) { + try { + httpMethods.add(HttpMethod.valueOf(value.toUpperCase(Locale.ROOT))); + } catch (IllegalArgumentException e) { + throw new ConfigurationException(format("Invalid http-method [%s] on action [%s], allowed values: %s", + value, actionElement.getAttribute("name"), Arrays.toString(HttpMethod.values())), e, location); + } + } + return httpMethods; + } + protected void verifyAction(String className, Location loc) { if (className.contains("{")) { LOG.debug("Action class [{}] contains a wildcard replacement value, so it can't be verified", className); diff --git a/core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java index 8e732f3260..c6dbfc0d3b 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/httpmethod/HttpMethodInterceptor.java @@ -19,19 +19,28 @@ package org.apache.struts2.interceptor.httpmethod; import org.apache.struts2.ActionInvocation; +import org.apache.struts2.UnknownHandlerManager; +import org.apache.struts2.action.Action; +import org.apache.struts2.config.entities.ActionConfig; import org.apache.struts2.interceptor.AbstractInterceptor; import org.apache.struts2.util.AnnotationUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; /** * Interceptor is used to control with what http methods action can be called, @@ -45,6 +54,12 @@ * {@link HttpPut}, {@link HttpDelete} and {@link HttpGetOrPost} *

* You can combine any of these annotations to achieve required allowed methods' filtering. + *

+ * 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. + *

+ * If no result can be resolved under the bad request result name, the response is + * {@code 405 Method Not Allowed} with an {@code Allow} header listing the accepted methods. * * @see HttpMethodAware * @see HttpMethod @@ -97,21 +112,73 @@ public String intercept(ActionInvocation invocation) throws Exception { return doIntercept(invocation, action.getClass()); } + Set configuredMethods = invocation.getProxy().getConfig().getHttpMethods(); + if (!configuredMethods.isEmpty()) { + LOG.debug("Action: {} declares http-method: {}, checking if request: {} meets allowed methods!", + action, configuredMethods, request.getMethod()); + return checkAllowedMethods(invocation, configuredMethods); + } + return invocation.invoke(); } protected String doIntercept(ActionInvocation invocation, AnnotatedElement element) throws Exception { - List allowedMethods = readAllowedMethods(element); + Set allowedMethods = EnumSet.noneOf(HttpMethod.class); + allowedMethods.addAll(readAllowedMethods(element)); + + Set configuredMethods = invocation.getProxy().getConfig().getHttpMethods(); + if (!configuredMethods.isEmpty()) { + allowedMethods.retainAll(configuredMethods); + } + + return checkAllowedMethods(invocation, allowedMethods); + } + + private String checkAllowedMethods(ActionInvocation invocation, Set allowedMethods) throws Exception { HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); - HttpMethod requestedMethod = HttpMethod.parse(request.getMethod()); + String requestedMethod = request.getMethod(); - if (allowedMethods.contains(requestedMethod)) { + if (isAllowed(requestedMethod, allowedMethods)) { LOG.trace("Request method: {} matches allowed methods: {}, continuing invocation!", requestedMethod, allowedMethods); return invocation.invoke(); } else { - LOG.trace("Request method: {} doesn't match allowed methods: {}, continuing invocation!", requestedMethod, allowedMethods); - return getBadRequestResultName(invocation); + LOG.trace("Request method: {} doesn't match allowed methods: {}, rejecting request!", requestedMethod, allowedMethods); + return rejectRequest(invocation, allowedMethods); + } + } + + private boolean isAllowed(String requestedMethod, Set allowedMethods) { + return allowedMethods.stream().anyMatch(method -> method.name().equalsIgnoreCase(requestedMethod)); + } + + /** + * @return the bad request result name when the action maps that result, otherwise {@link Action#NONE} + * after sending {@code 405 Method Not Allowed} with an {@code Allow} header + * @since 7.5.0 + */ + protected String rejectRequest(ActionInvocation invocation, Collection allowedMethods) throws IOException { + String resultName = getBadRequestResultName(invocation); + if (canResolveResult(invocation, resultName)) { + return resultName; + } + + LOG.debug("No result mapped for: {}, sending: {} with allowed methods: {}", + resultName, HttpServletResponse.SC_METHOD_NOT_ALLOWED, allowedMethods); + HttpServletResponse response = invocation.getInvocationContext().getServletResponse(); + response.setHeader("Allow", allowedMethods.stream().map(HttpMethod::name).collect(Collectors.joining(", "))); + response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED); + return Action.NONE; + } + + private boolean canResolveResult(ActionInvocation invocation, String resultName) { + ActionConfig config = invocation.getProxy().getConfig(); + if (config.getResults().containsKey(resultName) || config.getResults().containsKey("*")) { + return true; } + UnknownHandlerManager unknownHandlerManager = invocation.getInvocationContext().getContainer().getInstance(UnknownHandlerManager.class); + return unknownHandlerManager.hasUnknownHandlers() + && unknownHandlerManager.handleUnknownResult(invocation.getInvocationContext(), + invocation.getProxy().getActionName(), config, resultName) != null; } protected List readAllowedMethods(AnnotatedElement element) { diff --git a/core/src/main/resources/struts-7.5.dtd b/core/src/main/resources/struts-7.5.dtd index 307f5f0e0a..b534fd5c84 100644 --- a/core/src/main/resources/struts-7.5.dtd +++ b/core/src/main/resources/struts-7.5.dtd @@ -105,6 +105,7 @@ class CDATA #IMPLIED method CDATA #IMPLIED converter CDATA #IMPLIED + http-method CDATA #IMPLIED > diff --git a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderHttpMethodTest.java b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderHttpMethodTest.java new file mode 100644 index 0000000000..235ec3ca38 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderHttpMethodTest.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.config.providers; + +import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.config.ConfigurationException; +import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.apache.struts2.config.entities.ActionConfig; +import org.apache.struts2.config.impl.DefaultConfiguration; +import org.apache.struts2.test.LogCapture; + +import static org.apache.struts2.interceptor.httpmethod.HttpMethod.GET; +import static org.apache.struts2.interceptor.httpmethod.HttpMethod.POST; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class XmlConfigurationProviderHttpMethodTest extends StrutsInternalTestCase { + + private static final String CONFIG = "org/apache/struts2/config/providers/struts-http-method.xml"; + private static final String INVALID_CONFIG = "org/apache/struts2/config/providers/struts-http-method-invalid.xml"; + + public void testSingleHttpMethod() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("", "PostOnly").getHttpMethods()).containsExactly(POST); + } + + public void testHttpMethodListIsTrimmedAndCaseInsensitive() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("", "GetOrPost").getHttpMethods()).containsExactlyInAnyOrder(GET, POST); + } + + public void testNoHttpMethodMeansUnrestricted() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("", "Unrestricted").getHttpMethods()).isEmpty(); + } + + public void testWildcardActionKeepsHttpMethod() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + ActionConfig resolved = actionConfig("", "PostWild-execute"); + + assertThat(resolved.getName()).isEqualTo("PostWild-*"); + assertThat(resolved.getMethodName()).isEqualTo("execute"); + assertThat(resolved.getHttpMethods()).containsExactly(POST); + } + + public void testInvalidHttpMethodFailsConfiguration() { + StrutsXmlConfigurationProvider provider = new StrutsXmlConfigurationProvider(INVALID_CONFIG); + assertThatThrownBy(() -> loadConfigurationProviders(provider)) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Invalid http-method [fetch] on action [Fetch]"); + } + + public void testWarnsWhenStackHasNoHttpMethodInterceptor() { + try (LogCapture log = new LogCapture(DefaultConfiguration.class)) { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(log.messages()) + .anyMatch(message -> message.contains("[PostOnlyWithoutInterceptor]") && message.contains("http-method")) + .anyMatch(message -> message.contains("[PostOnlyWithoutStack]") && message.contains("http-method")) + .noneMatch(message -> message.contains("[PostOnly]")) + .noneMatch(message -> message.contains("[Unrestricted]")); + } + } + + private ActionConfig actionConfig(String namespace, String name) { + return configuration.getRuntimeConfiguration().getActionConfig(namespace, name); + } +} 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 0403152ba3..63ad8118f1 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 @@ -18,18 +18,32 @@ */ package org.apache.struts2.interceptor.httpmethod; +import jakarta.servlet.http.HttpServletResponse; +import org.apache.struts2.action.Action; import org.apache.struts2.ActionContext; import org.apache.struts2.ActionProxy; +import org.apache.struts2.result.Result; +import org.apache.struts2.UnknownHandlerManager; import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.apache.struts2.config.entities.ActionConfig; +import org.apache.struts2.config.entities.ResultConfig; +import org.apache.struts2.inject.Container; import org.apache.struts2.mock.MockActionInvocation; import org.apache.struts2.mock.MockActionProxy; import org.apache.struts2.HttpMethodsTestAction; import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.TestAction; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import java.util.List; import java.util.Map; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class HttpMethodInterceptorTest extends StrutsInternalTestCase { private HttpMethodInterceptor interceptor; @@ -270,13 +284,26 @@ public void testWildcardResolvedMethodWithPostAnnotationAllowsPost() throws Exce } private void prepareActionInvocation(Object action) { + prepareActionInvocation(action, actionConfig("bad-request", "custom-bad-request", "action-bad-request")); + } + + private void prepareActionInvocation(Object action, ActionConfig config) { interceptor = new HttpMethodInterceptor(); invocation = new MockActionInvocation(); invocation.setAction(action); actionProxy = new MockActionProxy(); + actionProxy.setConfig(config); invocation.setProxy(actionProxy); } + private ActionConfig actionConfig(String... resultNames) { + ActionConfig.Builder builder = new ActionConfig.Builder("package", "action", TestAction.class.getName()); + for (String resultName : resultNames) { + builder.addResultConfig(new ResultConfig.Builder(resultName, "result").build()); + } + return builder.build(); + } + /** * Regression: wildcard-resolved method with NO method-level annotation on a class @@ -336,8 +363,10 @@ public void testWildcardResolvedExecuteRejectsGetThroughRealProxy() throws Excep "org/apache/struts2/config/providers/xwork-test-allowed-methods.xml")); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/Wild-execute"); + MockHttpServletResponse response = new MockHttpServletResponse(); Map extraContext = ActionContext.of() .withServletRequest(request) + .withServletResponse(response) .getContextMap(); ActionProxy proxy = actionProxyFactory.createActionProxy("", "Wild-execute", null, extraContext); @@ -351,13 +380,184 @@ public void testWildcardResolvedExecuteRejectsGetThroughRealProxy() throws Excep // class-level @AllowedHttpMethod(POST) must still be enforced even though the resolved // method carries no method-level annotation — this is what #1690 fixed + assertEquals(Action.NONE, result); + assertEquals(HttpServletResponse.SC_METHOD_NOT_ALLOWED, response.getStatus()); + assertEquals("POST", response.getHeader("Allow")); + } + + public void testAnnotationWithoutMappedResultSends405() throws Exception { + prepareActionInvocation(new HttpMethodsTestAction(), actionConfig()); + MockHttpServletResponse response = prepareRequest("get"); + + String resultName = interceptor.intercept(invocation); + + assertEquals(Action.NONE, resultName); + assertEquals(HttpServletResponse.SC_METHOD_NOT_ALLOWED, response.getStatus()); + assertEquals("POST", response.getHeader("Allow")); + } + + public void testConfigAndAnnotationMustBothAllowTheMethod() throws Exception { + ActionConfig config = new ActionConfig.Builder(actionConfig("bad-request")) + .addHttpMethods(List.of(HttpMethod.GET)) + .build(); + prepareActionInvocation(new HttpMethodsTestAction(), config); + actionProxy.setMethod("onGetPostOnly"); + actionProxy.setMethodSpecified(true); + invocation.setResultCode("onGetPostOnly"); + + prepareRequest("get"); + assertEquals("onGetPostOnly", interceptor.intercept(invocation)); + + prepareRequest("post"); + assertEquals("bad-request", interceptor.intercept(invocation)); + } + + public void testAnnotationNarrowsConfig() throws Exception { + ActionConfig config = new ActionConfig.Builder(actionConfig("bad-request")) + .addHttpMethods(List.of(HttpMethod.GET, HttpMethod.POST)) + .build(); + prepareActionInvocation(new HttpMethodsTestAction(), config); + invocation.setResultCode("success"); + + prepareRequest("post"); + assertEquals("success", interceptor.intercept(invocation)); + + prepareRequest("get"); + assertEquals("bad-request", interceptor.intercept(invocation)); + } + + public void testUnknownHandlerResultIsUsedInsteadOf405() throws Exception { + prepareActionInvocation(new HttpMethodsTestAction(), actionConfig()); + UnknownHandlerManager unknownHandlerManager = mock(UnknownHandlerManager.class); + when(unknownHandlerManager.hasUnknownHandlers()).thenReturn(true); + Result unknownResult = mock(Result.class); + when(unknownHandlerManager.handleUnknownResult(any(), any(), any(), eq("bad-request"))).thenReturn(unknownResult); + Container mockContainer = mock(Container.class); + when(mockContainer.getInstance(UnknownHandlerManager.class)).thenReturn(unknownHandlerManager); + MockHttpServletResponse response = prepareRequest("get"); + ActionContext.getContext().withContainer(mockContainer); + + String resultName = interceptor.intercept(invocation); + + assertEquals("bad-request", resultName); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + } + + public void testConfigRestrictsMethodWithoutAnnotation() throws Exception { + ActionConfig config = new ActionConfig.Builder(actionConfig("bad-request")) + .addHttpMethods(List.of(HttpMethod.GET)) + .build(); + prepareActionInvocation(new TestAction(), config); + invocation.setResultCode("success"); + + prepareRequest("get"); + assertEquals("success", interceptor.intercept(invocation)); + + prepareRequest("post"); + assertEquals("bad-request", interceptor.intercept(invocation)); + } + + public void testConfigRejectsUnknownRequestMethod() throws Exception { + ActionConfig config = new ActionConfig.Builder(actionConfig()) + .addHttpMethods(List.of(HttpMethod.GET)) + .build(); + prepareActionInvocation(new TestAction(), config); + MockHttpServletResponse response = prepareRequest("PROPFIND"); + + String resultName = interceptor.intercept(invocation); + + assertEquals(Action.NONE, resultName); + assertEquals(HttpServletResponse.SC_METHOD_NOT_ALLOWED, response.getStatus()); + assertEquals("GET", response.getHeader("Allow")); + } + + public void testHttpMethodAttributeRejectsWith405AndAllowHeader() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionProxy proxy = proxyFor("", "PostOnly", "GET", response); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + + assertEquals(Action.NONE, result); + assertEquals(HttpServletResponse.SC_METHOD_NOT_ALLOWED, response.getStatus()); + assertEquals("POST", response.getHeader("Allow")); + } + + public void testHttpMethodAttributeAllowsListedMethod() throws Exception { + ActionProxy proxy = proxyFor("", "PostOnly", "POST", new MockHttpServletResponse()); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + + assertEquals(Action.SUCCESS, result); + } + + public void testHttpMethodAttributeListsAllMethodsInAllowHeader() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionProxy proxy = proxyFor("", "GetOrPost", "PUT", response); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + + assertEquals(Action.NONE, result); + assertEquals("GET, POST", response.getHeader("Allow")); + } + + public void testHttpMethodAttributeUsesMappedActionResult() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionProxy proxy = proxyFor("", "PostOnlyWithResult", "GET", response); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + + assertEquals("bad-request", result); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + } + + public void testHttpMethodAttributeUsesMappedGlobalResult() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionProxy proxy = proxyFor("/global", "PostOnly", "GET", response); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + + assertEquals("bad-request", result); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + } + + public void testHttpMethodAttributeUsesCatchAllResult() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionProxy proxy = proxyFor("", "PostOnlyWithCatchAllResult", "GET", response); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + assertEquals("bad-request", result); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + } + + public void testHttpMethodAttributeOnWildcardAction() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionProxy proxy = proxyFor("", "PostWild-execute", "GET", response); + + String result = new HttpMethodInterceptor().intercept(proxy.getInvocation()); + + assertEquals(Action.NONE, result); + assertEquals(HttpServletResponse.SC_METHOD_NOT_ALLOWED, response.getStatus()); + } + + private ActionProxy proxyFor(String namespace, String actionName, String httpMethod, MockHttpServletResponse response) { + loadConfigurationProviders(new StrutsXmlConfigurationProvider( + "org/apache/struts2/config/providers/struts-http-method.xml")); + + Map extraContext = ActionContext.of() + .withServletRequest(new MockHttpServletRequest(httpMethod, "/" + actionName)) + .withServletResponse(response) + .getContextMap(); + + return actionProxyFactory.createActionProxy(namespace, actionName, null, extraContext, false, true); } - private void prepareRequest(String httpMethod) { + private MockHttpServletResponse prepareRequest(String httpMethod) { MockHttpServletRequest request = new MockHttpServletRequest(httpMethod, "/action"); - ActionContext.getContext().withServletRequest(request); + MockHttpServletResponse response = new MockHttpServletResponse(); + ActionContext.getContext().withServletRequest(request).withServletResponse(response); invocation.setInvocationContext(ActionContext.getContext()); + return response; } } diff --git a/core/src/test/resources/org/apache/struts2/config/providers/struts-http-method-invalid.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-http-method-invalid.xml new file mode 100644 index 0000000000..0b599ac3d9 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-http-method-invalid.xml @@ -0,0 +1,29 @@ + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/providers/struts-http-method.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-http-method.xml new file mode 100644 index 0000000000..cc309d5984 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-http-method.xml @@ -0,0 +1,79 @@ + + + + + + + + + + + + + + + + + + + + + + + + + /bad-request.jsp + + + + + /any.jsp + + + + + regex:.* + + + + + + + + + + + + + + + + /bad-request.jsp + + + + + + +