You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2022/11/11 08:25:04 UTC
[struts] 01/01: WW-4173 Introduces a dedicated interface to allow conditionally executing a given interceptor
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-4173-optional
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 3efe12a07ff65bdd7c198197d790e9b4af553268
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Wed Nov 9 16:20:54 2022 +0100
WW-4173 Introduces a dedicated interface to allow conditionally executing a given interceptor
---
.../xwork2/DefaultActionInvocation.java | 7 ++--
.../xwork2/interceptor/AbstractInterceptor.java | 12 +++++--
...nterceptor.java => ConditionalInterceptor.java} | 38 +++++++---------------
.../xwork2/interceptor/Interceptor.java | 8 -----
.../struts2/interceptor/CoepInterceptor.java | 10 +-----
.../struts2/interceptor/CoopInterceptor.java | 9 +----
.../interceptor/FetchMetadataInterceptor.java | 4 ---
.../struts2/interceptor/csp/CspInterceptor.java | 13 +-------
.../struts2/interceptor/CoepInterceptorTest.java | 9 -----
.../struts2/interceptor/CoopInterceptorTest.java | 9 -----
.../struts2/interceptor/CspInterceptorTest.java | 11 -------
.../interceptor/FetchMetadataInterceptorTest.java | 8 -----
12 files changed, 27 insertions(+), 111 deletions(-)
diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java
index c1e049dfa..9634e47ce 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,11 +249,11 @@ public class DefaultActionInvocation implements ActionInvocation {
if (interceptor instanceof WithLazyParams) {
interceptor = lazyParamInjector.injectParams(interceptor, interceptorMapping.getParams(), invocationContext);
}
- if (interceptor.isDisabled(this)) {
+ if (interceptor instanceof ConditionalInterceptor && ((ConditionalInterceptor) interceptor).shouldIntercept(this)) {
+ resultCode = interceptor.intercept(this);
+ } else {
LOG.debug("Interceptor: {} is disabled, skipping to next", interceptor.getClass().getSimpleName());
resultCode = this.invoke();
- } else {
- resultCode = interceptor.intercept(this);
}
} else {
resultCode = invokeActionOnly();
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 <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;
}
}
diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConditionalInterceptor.java
similarity index 59%
copy from core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java
copy to core/src/main/java/com/opensymphony/xwork2/interceptor/ConditionalInterceptor.java
index efa009052..12752fce8 100644
--- a/core/src/main/java/com/opensymphony/xwork2/interceptor/AbstractInterceptor.java
+++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ConditionalInterceptor.java
@@ -21,35 +21,19 @@ package com.opensymphony.xwork2.interceptor;
import com.opensymphony.xwork2.ActionInvocation;
/**
- * Provides default implementations of optional lifecycle methods
+ * A marking interface, when implemented allows to conditionally execute a given interceptor
+ * within the current action invocation.
+ *
+ * @since Struts 6.1.1
*/
-public abstract class AbstractInterceptor implements Interceptor {
-
- private boolean disabled;
-
- /**
- * Does nothing
- */
- public void init() {
- }
+public interface ConditionalInterceptor extends Interceptor {
/**
- * Does nothing
+ * 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
*/
- public void destroy() {
- }
-
- /**
- * Override to handle interception
- */
- public abstract String intercept(ActionInvocation invocation) throws Exception;
-
- public void setDisabled(String disable) {
- this.disabled = Boolean.parseBoolean(disable);
- }
-
- @Override
- public boolean isDisabled(ActionInvocation invocation) {
- return this.disabled;
- }
+ 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/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));
- }
}