WW-4173 Introduces a dedicated interface to allow conditionally executing a given interceptor

This commit is contained in:
Lukasz Lenart
2022-11-09 16:20:54 +01:00
parent 1af3e31548
commit bcfa8113ac
13 changed files with 115 additions and 116 deletions
@@ -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());
}
@@ -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 <param name="disabled">true</param>}
* or use other way to override interceptor's parameters, see
* <a href="https://struts.apache.org/core-developers/interceptors#interceptor-parameter-overriding">docs</a>.
* @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;
}
}
@@ -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);
}
@@ -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);
}
@@ -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();
@@ -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();
@@ -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();
@@ -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);
@@ -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<InterceptorMapping> 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<InterceptorMapping> 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<String, Object>(), 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<Callable<String>>() {
@Override
public Callable<String> call() throws Exception {
return new Callable<String>() {
@Override
public String call() throws Exception {
return "success";
}
};
}
};
dai.action = (Callable<Callable<String>>) () -> (Callable<String>) () -> "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<InterceptorMapping> 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<String, Object>());
"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<String, Object>());
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<String, Object>());
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<String, Object>());
ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "NullFoo", "nullMethod", new HashMap<>());
DefaultActionInvocation defaultActionInvocation = (DefaultActionInvocation) actionProxy.getInvocation();
defaultActionInvocation.init(actionProxy);
@@ -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");
@@ -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();
@@ -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)) {
@@ -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));
}
}