diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java index c1e049dfa..772e0adb0 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java @@ -24,6 +24,7 @@ import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.interceptor.ConditionalInterceptor; import com.opensymphony.xwork2.interceptor.Interceptor; import com.opensymphony.xwork2.interceptor.PreResultListener; import com.opensymphony.xwork2.interceptor.WithLazyParams; @@ -248,10 +249,10 @@ public class DefaultActionInvocation implements ActionInvocation { if (interceptor instanceof WithLazyParams) { interceptor = lazyParamInjector.injectParams(interceptor, interceptorMapping.getParams(), invocationContext); } - if (interceptor.isDisabled(this)) { - LOG.debug("Interceptor: {} is disabled, skipping to next", interceptor.getClass().getSimpleName()); - resultCode = this.invoke(); + if (interceptor instanceof ConditionalInterceptor) { + resultCode = executeConditional((ConditionalInterceptor) interceptor); } else { + LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName()); resultCode = interceptor.intercept(this); } } else { @@ -292,6 +293,16 @@ public class DefaultActionInvocation implements ActionInvocation { return resultCode; } + protected String executeConditional(ConditionalInterceptor conditionalInterceptor) throws Exception { + if (conditionalInterceptor.shouldIntercept(this)) { + LOG.debug("Executing conditional interceptor: {}", conditionalInterceptor.getClass().getSimpleName()); + return conditionalInterceptor.intercept(this); + } else { + LOG.debug("Interceptor: {} is disabled, skipping to next", conditionalInterceptor.getClass().getSimpleName()); + return this.invoke(); + } + } + public String invokeActionOnly() throws Exception { return invokeAction(getAction(), proxy.getConfig()); } diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java index efa009052..21e459c29 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java @@ -23,7 +23,7 @@ import com.opensymphony.xwork2.ActionInvocation; /** * Provides default implementations of optional lifecycle methods */ -public abstract class AbstractInterceptor implements Interceptor { +public abstract class AbstractInterceptor implements ConditionalInterceptor { private boolean disabled; @@ -44,12 +44,18 @@ public abstract class AbstractInterceptor implements Interceptor { */ public abstract String intercept(ActionInvocation invocation) throws Exception; + /** + * Allows to skip executing a given interceptor, just define {@code true} + * or use other way to override interceptor's parameters, see + * docs. + * @param disable if set to true, execution of a given interceptor will be skipped. + */ public void setDisabled(String disable) { this.disabled = Boolean.parseBoolean(disable); } @Override - public boolean isDisabled(ActionInvocation invocation) { - return this.disabled; + public boolean shouldIntercept(ActionInvocation invocation) { + return !this.disabled; } } diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ConditionalInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConditionalInterceptor.java new file mode 100644 index 000000000..12752fce8 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConditionalInterceptor.java @@ -0,0 +1,39 @@ +/* + * 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 com.opensymphony.xwork2.interceptor; + +import com.opensymphony.xwork2.ActionInvocation; + +/** + * A marking interface, when implemented allows to conditionally execute a given interceptor + * within the current action invocation. + * + * @since Struts 6.1.1 + */ +public interface ConditionalInterceptor extends Interceptor { + + /** + * Determines if a given interceptor should be executed in the current processing of action invocation. + * + * @param invocation current {@link ActionInvocation} to determine if the interceptor should be executed + * @return true if the given interceptor should be included in the current action invocation + * @since 6.1.1 + */ + boolean shouldIntercept(ActionInvocation invocation); +} diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/Interceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/Interceptor.java index 3488314d2..cafa08fc0 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/Interceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/Interceptor.java @@ -219,12 +219,4 @@ public interface Interceptor extends Serializable { */ String intercept(ActionInvocation invocation) throws Exception; - /** - * Allows to disable processing a given interceptor - * - * @param invocation current {@link ActionInvocation} to determine if the interceptor should be executed - * @return true if the given interceptor should be skipped - * @since 6.1.0 - */ - boolean isDisabled(ActionInvocation invocation); } diff --git a/core/src/main/java/org/apache/struts2/interceptor/CoepInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/CoepInterceptor.java index 6d550c19f..850556e32 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/CoepInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/CoepInterceptor.java @@ -51,20 +51,12 @@ public class CoepInterceptor extends AbstractInterceptor implements PreResultLis @Override public String intercept(ActionInvocation invocation) throws Exception { - if (this.isDisabled(invocation)) { - LOG.trace("COEP interceptor has been disabled"); - } else { - invocation.addPreResultListener(this); - } + invocation.addPreResultListener(this); return invocation.invoke(); } @Override public void beforeResult(ActionInvocation invocation, String resultCode) { - if (this.isDisabled(invocation)) { - return; - } - HttpServletRequest req = invocation.getInvocationContext().getServletRequest(); final String path = req.getContextPath(); diff --git a/core/src/main/java/org/apache/struts2/interceptor/CoopInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/CoopInterceptor.java index 9827ceb13..123170baf 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/CoopInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/CoopInterceptor.java @@ -53,19 +53,12 @@ public class CoopInterceptor extends AbstractInterceptor implements PreResultLis @Override public String intercept(ActionInvocation invocation) throws Exception { - if (this.isDisabled(invocation)) { - LOG.trace("COOP interceptor has been disabled"); - } else { - invocation.addPreResultListener(this); - } + invocation.addPreResultListener(this); return invocation.invoke(); } @Override public void beforeResult(ActionInvocation invocation, String resultCode) { - if (this.isDisabled(invocation)) { - return; - } HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); String path = request.getContextPath(); diff --git a/core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java index 9a3583607..0f46206f2 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/FetchMetadataInterceptor.java @@ -62,10 +62,6 @@ public class FetchMetadataInterceptor extends AbstractInterceptor { @Override public String intercept(ActionInvocation invocation) throws Exception { - if (this.isDisabled(invocation)) { - LOG.trace("Fetch Metadata interceptor has been disabled"); - return invocation.invoke(); - } ActionContext context = invocation.getInvocationContext(); HttpServletRequest request = context.getServletRequest(); diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java index 38b196514..5bae4f543 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java @@ -21,8 +21,6 @@ package org.apache.struts2.interceptor.csp; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; import com.opensymphony.xwork2.interceptor.PreResultListener; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -41,24 +39,15 @@ import java.util.Optional; **/ public final class CspInterceptor extends AbstractInterceptor implements PreResultListener { - private static final Logger LOG = LogManager.getLogger(CspInterceptor.class); - private final CspSettings settings = new DefaultCspSettings(); @Override public String intercept(ActionInvocation invocation) throws Exception { - if (this.isDisabled(invocation)) { - LOG.trace("CSP interceptor has been disabled"); - } else { - invocation.addPreResultListener(this); - } + invocation.addPreResultListener(this); return invocation.invoke(); } public void beforeResult(ActionInvocation invocation, String resultCode) { - if (this.isDisabled(invocation)) { - return; - } HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); HttpServletResponse response = invocation.getInvocationContext().getServletResponse(); settings.addCspHeaders(request, response); diff --git a/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java b/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java index 0d2ca8918..6ad6b8422 100644 --- a/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java @@ -22,7 +22,7 @@ import com.opensymphony.xwork2.config.entities.ActionConfig; import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider; -import com.opensymphony.xwork2.interceptor.PreResultListener; +import com.opensymphony.xwork2.interceptor.Interceptor; import com.opensymphony.xwork2.mock.MockActionProxy; import com.opensymphony.xwork2.mock.MockInterceptor; import com.opensymphony.xwork2.mock.MockResult; @@ -65,10 +65,18 @@ public class DefaultActionInvocationTest extends XWorkTestCase { interceptorMappings.add(new InterceptorMapping("test2", mockInterceptor2)); mockInterceptor2.setFoo("test2"); mockInterceptor2.setExpectedFoo("test2"); - MockInterceptor mockInterceptor3 = new MockInterceptor(); + MockInterceptor mockInterceptor3 = new MockInterceptor() { + @Override + public boolean shouldIntercept(ActionInvocation invocation) { + return false; + } + }; interceptorMappings.add(new InterceptorMapping("test3", mockInterceptor3)); - mockInterceptor3.setFoo("test3"); - mockInterceptor3.setExpectedFoo("test3"); + + MockInterceptor mockInterceptor4 = new MockInterceptor(); + interceptorMappings.add(new InterceptorMapping("test4", mockInterceptor4)); + mockInterceptor4.setFoo("test4"); + mockInterceptor4.setExpectedFoo("test4"); DefaultActionInvocation defaultActionInvocation = new DefaultActionInvocationTester(interceptorMappings); container.inject(defaultActionInvocation); @@ -78,7 +86,8 @@ public class DefaultActionInvocationTest extends XWorkTestCase { defaultActionInvocation.invoke(); assertTrue(mockInterceptor1.isExecuted()); assertTrue(mockInterceptor2.isExecuted()); - assertTrue(mockInterceptor3.isExecuted()); + assertFalse(mockInterceptor3.isExecuted()); + assertTrue(mockInterceptor4.isExecuted()); assertTrue(defaultActionInvocation.isExecuted()); try { defaultActionInvocation.setResultCode(""); @@ -92,6 +101,33 @@ public class DefaultActionInvocationTest extends XWorkTestCase { } } + public void testInvokeSimpleInterceptor() throws Exception { + List interceptorMappings = new ArrayList<>(); + Interceptor interceptor1 = new Interceptor() { + @Override + public void destroy() { + } + + @Override + public void init() { + } + + @Override + public String intercept(ActionInvocation invocation) throws Exception { + return "done"; + } + }; + interceptorMappings.add(new InterceptorMapping("test1", interceptor1)); + + DefaultActionInvocation defaultActionInvocation = new DefaultActionInvocationTester(interceptorMappings); + container.inject(defaultActionInvocation); + defaultActionInvocation.stack = container.getInstance(ValueStackFactory.class).createValueStack(); + + String result = defaultActionInvocation.invoke(); + assertTrue(defaultActionInvocation.isExecuted()); + assertEquals("done", result); + } + public void testInvokeWithDisabledInterceptors() throws Exception { // given List interceptorMappings = new ArrayList<>(); @@ -147,7 +183,7 @@ public class DefaultActionInvocationTest extends XWorkTestCase { assertEquals("success", result); } - public void testInvokingMissingMethod() throws Exception { + public void testInvokingMissingMethod() { // given DefaultActionInvocation dai = new DefaultActionInvocation(ActionContext.getContext().getContextMap(), false); container.inject(dai); @@ -348,7 +384,7 @@ public class DefaultActionInvocationTest extends XWorkTestCase { } public void testInvokeWithAsyncManager() throws Exception { - DefaultActionInvocation dai = new DefaultActionInvocation(new HashMap(), false); + DefaultActionInvocation dai = new DefaultActionInvocation(new HashMap<>(), false); dai.stack = container.getInstance(ValueStackFactory.class).createValueStack(); final Semaphore lock = new Semaphore(1); @@ -377,29 +413,14 @@ public class DefaultActionInvocationTest extends XWorkTestCase { } }); - dai.action = new Callable>() { - @Override - public Callable call() throws Exception { - return new Callable() { - @Override - public String call() throws Exception { - return "success"; - } - }; - } - }; + dai.action = (Callable>) () -> (Callable) () -> "success"; MockActionProxy actionProxy = new MockActionProxy(); actionProxy.setMethod("call"); dai.proxy = actionProxy; final boolean[] preResultExecuted = new boolean[1]; - dai.addPreResultListener(new PreResultListener() { - @Override - public void beforeResult(ActionInvocation invocation, String resultCode) { - preResultExecuted[0] = true; - } - }); + dai.addPreResultListener((invocation, resultCode) -> preResultExecuted[0] = true); List interceptorMappings = new ArrayList<>(); MockInterceptor mockInterceptor1 = new MockInterceptor(); @@ -436,7 +457,7 @@ public class DefaultActionInvocationTest extends XWorkTestCase { public void testActionEventListener() throws Exception { ActionProxy actionProxy = actionProxyFactory.createActionProxy("", - "ExceptionFoo", "exceptionMethod", new HashMap()); + "ExceptionFoo", "exceptionMethod", new HashMap<>()); DefaultActionInvocation defaultActionInvocation = (DefaultActionInvocation) actionProxy.getInvocation(); SimpleActionEventListener actionEventListener = new SimpleActionEventListener("prepared", "exceptionHandled"); @@ -461,8 +482,7 @@ public class DefaultActionInvocationTest extends XWorkTestCase { } public void testActionChainResult() throws Exception { - ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "Foo", null, - new HashMap()); + ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "Foo", null, new HashMap<>()); DefaultActionInvocation defaultActionInvocation = (DefaultActionInvocation) actionProxy.getInvocation(); defaultActionInvocation.init(actionProxy); @@ -478,9 +498,8 @@ public class DefaultActionInvocationTest extends XWorkTestCase { assertTrue(result instanceof MockResult); } - public void testNoResultDefined() throws Exception { - ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "Foo", null, - new HashMap()); + public void testNoResultDefined() { + ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "Foo", null, new HashMap<>()); DefaultActionInvocation defaultActionInvocation = (DefaultActionInvocation) actionProxy.getInvocation(); defaultActionInvocation.init(actionProxy); @@ -492,8 +511,7 @@ public class DefaultActionInvocationTest extends XWorkTestCase { } public void testNullResultPossible() throws Exception { - ActionProxy actionProxy = actionProxyFactory.createActionProxy("", - "NullFoo", "nullMethod", new HashMap()); + ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "NullFoo", "nullMethod", new HashMap<>()); DefaultActionInvocation defaultActionInvocation = (DefaultActionInvocation) actionProxy.getInvocation(); defaultActionInvocation.init(actionProxy); diff --git a/core/src/test/java/org/apache/struts2/interceptor/CoepInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CoepInterceptorTest.java index 1401951c2..72ea1a024 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CoepInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CoepInterceptorTest.java @@ -41,15 +41,6 @@ public class CoepInterceptorTest extends StrutsInternalTestCase { private final String HEADER_CONTENT = "require-corp"; - public void testDisabled() throws Exception { - interceptor.setDisabled("true"); - - interceptor.intercept(mai); - - String header = response.getHeader(COEP_ENFORCING_HEADER); - assertTrue("COEP is not disabled", Strings.isEmpty(header)); - } - public void testEnforcingHeader() throws Exception { interceptor.setEnforcingMode("true"); diff --git a/core/src/test/java/org/apache/struts2/interceptor/CoopInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CoopInterceptorTest.java index 8e9e9d4ec..544e7b5d8 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CoopInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CoopInterceptorTest.java @@ -75,15 +75,6 @@ public class CoopInterceptorTest extends StrutsInternalTestCase { } } - public void testDisabled() throws Exception { - interceptor.setDisabled("true"); - - interceptor.intercept(mai); - - String header = response.getHeader(COOP_HEADER); - assertTrue("COOP is not disabled", Strings.isEmpty(header)); - } - @Override protected void setUp() throws Exception { super.setUp(); diff --git a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java index a091b93c7..5727a9dcf 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java @@ -145,17 +145,6 @@ public class CspInterceptorTest extends StrutsInternalTestCase { } } - public void testDisabled() throws Exception { - interceptor.setDisabled("true"); - - interceptor.intercept(mai); - - String header = response.getHeader(CSP_ENFORCE_HEADER); - assertTrue("CSP is not disabled", Strings.isEmpty(header)); - header = response.getHeader(CSP_REPORT_HEADER); - assertTrue("CSP is not disabled", Strings.isEmpty(header)); - } - public void checkHeader(String reportUri, String enforcingMode) { String expectedCspHeader; if (Strings.isEmpty(reportUri)) { diff --git a/core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java index 4b8403c47..6426ebeaa 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java @@ -261,12 +261,4 @@ public class FetchMetadataInterceptorTest extends XWorkTestCase { assertNotEquals("Expected interceptor to accept this request [" + "/" + fetchMetadataExemptedGlobalActionConfig.getName() + "]", SC_FORBIDDEN, configuredFetchMetadataInterceptor.intercept(mai)); } - public void testDisabled() throws Exception { - interceptor.setDisabled("true"); - - interceptor.intercept(mai); - - String header = response.getHeader(VARY_HEADER); - assertTrue("Fetch Metadata is not disabled", Strings.isEmpty(header)); - } }