This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch feature/WW-5400-refactor
in repository https://gitbox.apache.org/repos/asf/struts.git
commit eae2c834b3c3a3bfb064f4ba3aab52345d88df1a
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Wed Jun 5 07:37:36 2024 +0200
WW-5400 Simplifies how CspSettings is created
---
.../struts2/interceptor/csp/CspInterceptor.java | 43 +++++++++--------
.../struts2/interceptor/CspInterceptorTest.java | 55 +++++++++++++++-------
2 files changed, 60 insertions(+), 38 deletions(-)
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 54d9eeab1..627825407 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
@@ -19,7 +19,9 @@
package org.apache.struts2.interceptor.csp;
import com.opensymphony.xwork2.ActionInvocation;
+import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
+import com.opensymphony.xwork2.util.ClassLoaderUtil;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.action.CspSettingsAware;
@@ -48,7 +50,7 @@ public final class CspInterceptor extends AbstractInterceptor
{
private String reportUri;
private String reportTo;
- private String defaultCspSettingsClassName =
DefaultCspSettings.class.getName();
+ private String cspSettingsClassName = DefaultCspSettings.class.getName();
@Override
public String intercept(ActionInvocation invocation) throws Exception {
@@ -57,26 +59,28 @@ public final class CspInterceptor extends
AbstractInterceptor {
LOG.trace("Using CspSettings provided by the action: {}", action);
applySettings(invocation, ((CspSettingsAware)
action).getCspSettings());
} else {
- LOG.trace("Using {} with action: {}", defaultCspSettingsClassName,
action);
+ LOG.trace("Using {} with action: {}", cspSettingsClassName,
action);
+ CspSettings cspSettings = createCspSettings(invocation);
+ applySettings(invocation, cspSettings);
+ }
+ return invocation.invoke();
+ }
- // if the defaultCspSettingsClassName is not a real class, throw
an exception
- try {
- Class.forName(defaultCspSettingsClassName, false,
Thread.currentThread().getContextClassLoader());
- }
- catch (ClassNotFoundException e) {
- throw new IllegalArgumentException("The
defaultCspSettingsClassName must be a real class.");
- }
+ private CspSettings createCspSettings(ActionInvocation invocation) throws
ClassNotFoundException {
+ Class<?> cspSettingsClass;
- // if defaultCspSettingsClassName does not implement CspSettings,
throw an exception
- if
(!CspSettings.class.isAssignableFrom(Class.forName(defaultCspSettingsClassName)))
{
- throw new IllegalArgumentException("The
defaultCspSettingsClassName must implement CspSettings.");
- }
+ try {
+ cspSettingsClass = ClassLoaderUtil.loadClass(cspSettingsClassName,
getClass());
+ } catch (ClassNotFoundException e) {
+ throw new ConfigurationException(String.format("The class %s
doesn't exist!", cspSettingsClassName));
+ }
- CspSettings cspSettings = (CspSettings)
Class.forName(defaultCspSettingsClassName)
- .getDeclaredConstructor().newInstance();
- applySettings(invocation, cspSettings);
+ if
(!CspSettings.class.isAssignableFrom(Class.forName(cspSettingsClassName))) {
+ throw new ConfigurationException(String.format("The class %s
doesn't implement %s!",
+ cspSettingsClassName, CspSettings.class.getName()));
}
- return invocation.invoke();
+
+ return (CspSettings)
invocation.getInvocationContext().getContainer().inject(cspSettingsClass);
}
private void applySettings(ActionInvocation invocation, CspSettings
cspSettings) {
@@ -127,7 +131,6 @@ public final class CspInterceptor extends
AbstractInterceptor {
* only be used if the reportUri is set.
*
* @param reportTo the report group where csp violation reports will be
sent
- *
* @since Struts 6.5.0
*/
public void setReportTo(String reportTo) {
@@ -167,7 +170,7 @@ public final class CspInterceptor extends
AbstractInterceptor {
*
* @since Struts 6.5.0
*/
- public void setDefaultCspSettingsClassName(String
defaultCspSettingsClassName) {
- this.defaultCspSettingsClassName = defaultCspSettingsClassName;
+ public void setCspSettingsClassName(String cspSettingsClassName) {
+ this.cspSettingsClassName = cspSettingsClassName;
}
}
\ No newline at end of file
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 cd59c347d..221e725db 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
@@ -19,6 +19,7 @@
package org.apache.struts2.interceptor;
import com.opensymphony.xwork2.ActionContext;
+import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import org.apache.logging.log4j.util.Strings;
import org.apache.struts2.StrutsInternalTestCase;
@@ -75,7 +76,7 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
public void testEnforcingCspHeadersSet() throws Exception {
String reportUri = "/csp-reports";
- String reportTo = "csp-group";
+ String reportTo = "csp-group";
boolean enforcingMode = true;
interceptor.setReportUri(reportUri);
interceptor.setReportTo(reportTo);
@@ -92,7 +93,7 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
public void testReportingCspHeadersSet() throws Exception {
String reportUri = "/csp-reports";
- String reportTo = "csp-group";
+ String reportTo = "csp-group";
boolean enforcingMode = false;
interceptor.setReportUri(reportUri);
interceptor.setReportTo(reportTo);
@@ -179,7 +180,7 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
checkHeader("/report-uri", enforcingMode);
}
- public void testInvalidDefaultCspSettingsClassName() throws Exception {
+ public void testNonExistingCspSettingsClassName() throws Exception {
boolean enforcingMode = true;
mai.setAction(new TestAction());
request.setContextPath("/app");
@@ -189,15 +190,15 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
interceptor.setPrependServletContext(false);
try {
- interceptor.setDefaultCspSettingsClassName("foo");
+ interceptor.setCspSettingsClassName("foo");
interceptor.intercept(mai);
- assert (false);
- } catch (IllegalArgumentException e) {
- assert (true);
+ fail("Expected exception");
+ } catch (ConfigurationException e) {
+ assertEquals("The class foo doesn't exist!", e.getMessage());
}
}
- public void testCustomDefaultCspSettingsClassName() throws Exception {
+ public void testInvalidCspSettingsClassName() throws Exception {
boolean enforcingMode = true;
mai.setAction(new TestAction());
request.setContextPath("/app");
@@ -205,7 +206,25 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
interceptor.setEnforcingMode(enforcingMode);
interceptor.setReportUri("/report-uri");
interceptor.setPrependServletContext(false);
-
interceptor.setDefaultCspSettingsClassName(CustomDefaultCspSettings.class.getName());
+
+ try {
+ interceptor.setCspSettingsClassName(Integer.class.getName());
+ interceptor.intercept(mai);
+ fail("Expected exception");
+ } catch (ConfigurationException e) {
+ assertEquals("The class java.lang.Integer doesn't implement
org.apache.struts2.interceptor.csp.CspSettings!", e.getMessage());
+ }
+ }
+
+ public void testCustomCspSettingsClassName() throws Exception {
+ boolean enforcingMode = true;
+ mai.setAction(new TestAction());
+ request.setContextPath("/app");
+
+ interceptor.setEnforcingMode(enforcingMode);
+ interceptor.setReportUri("/report-uri");
+ interceptor.setPrependServletContext(false);
+
interceptor.setCspSettingsClassName(CustomDefaultCspSettings.class.getName());
interceptor.intercept(mai);
@@ -223,9 +242,9 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
String expectedCspHeader;
if (Strings.isEmpty(reportUri)) {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s
%s; %s '%s'; ",
- CspSettings.OBJECT_SRC, CspSettings.NONE,
- CspSettings.SCRIPT_SRC, session.getAttribute("nonce"),
CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS,
- CspSettings.BASE_URI, CspSettings.NONE
+ CspSettings.OBJECT_SRC, CspSettings.NONE,
+ CspSettings.SCRIPT_SRC, session.getAttribute("nonce"),
CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS,
+ CspSettings.BASE_URI, CspSettings.NONE
);
} else {
if (Strings.isEmpty(reportTo)) {
@@ -235,8 +254,7 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
CspSettings.BASE_URI, CspSettings.NONE,
CspSettings.REPORT_URI, reportUri
);
- }
- else {
+ } else {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s'
%s %s; %s '%s'; %s %s; %s %s; ",
CspSettings.OBJECT_SRC, CspSettings.NONE,
CspSettings.SCRIPT_SRC, session.getAttribute("nonce"),
CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS,
@@ -263,10 +281,11 @@ public class CspInterceptorTest extends
StrutsInternalTestCase {
super.setUp();
container.inject(interceptor);
ActionContext context = ActionContext.getContext()
- .withServletRequest(request)
- .withServletResponse(response)
- .withSession(new SessionMap(request))
- .bind();
+ .withContainer(container)
+ .withServletRequest(request)
+ .withServletResponse(response)
+ .withSession(new SessionMap(request))
+ .bind();
mai.setInvocationContext(context);
session = request.getSession();
}
