From 9ddf336e5eeff75964e1fb5cc86d71a85d478877 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 18 Jan 2025 19:27:49 +0100 Subject: [PATCH] WW-5517 Fixes to be compatible with allowlist capability --- .../org/apache/struts2/components/Debug.java | 36 ++- .../ExceptionMappingInterceptor.java | 108 +++++---- .../debugging/DebuggingInterceptor.java | 53 ++-- .../struts2/ognl/SecurityMemberAccess.java | 2 - .../struts2/interceptor/debugging/browser.ftl | 22 +- .../interceptor/debugging/webconsole.html | 28 ++- .../struts2/StrutsJUnit4InternalTestCase.java | 2 - .../ExceptionMappingInterceptorTest.java | 60 ++--- .../debugging/DebuggingInterceptorTest.java | 229 ++++++++++++++++++ .../struts2/views/jsp/ui/DebugTagTest.java | 19 +- 10 files changed, 414 insertions(+), 145 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java diff --git a/core/src/main/java/org/apache/struts2/components/Debug.java b/core/src/main/java/org/apache/struts2/components/Debug.java index ddd228bf24..702f9e32b8 100644 --- a/core/src/main/java/org/apache/struts2/components/Debug.java +++ b/core/src/main/java/org/apache/struts2/components/Debug.java @@ -18,7 +18,10 @@ */ package org.apache.struts2.components; +import org.apache.commons.lang3.ClassUtils; import org.apache.struts2.inject.Inject; +import org.apache.struts2.ognl.ThreadAllowlist; +import org.apache.struts2.util.CompoundRoot; import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.reflection.ReflectionProvider; import jakarta.servlet.http.HttpServletRequest; @@ -40,6 +43,7 @@ public class Debug extends UIBean { protected ReflectionProvider reflectionProvider; + private ThreadAllowlist threadAllowlist; public Debug(ValueStack stack, HttpServletRequest request, HttpServletResponse response) { super(stack, request, response); @@ -50,6 +54,11 @@ public void setReflectionProvider(ReflectionProvider prov) { this.reflectionProvider = prov; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + protected String getDefaultTemplate() { return TEMPLATE; } @@ -59,16 +68,19 @@ public boolean start(Writer writer) { if (showDebug()) { ValueStack stack = getStack(); - Iterator iter = stack.getRoot().iterator(); - List stackValues = new ArrayList(stack.getRoot().size()); + allowList(stack.getRoot()); + + Iterator iter = stack.getRoot().iterator(); + List stackValues = new ArrayList<>(stack.getRoot().size()); while (iter.hasNext()) { Object o = iter.next(); - Map values; + Map values; try { values = reflectionProvider.getBeanMap(o); } catch (Exception e) { throw new StrutsException("Caught an exception while getting the property values of " + o, e); } + allowListClass(o); stackValues.add(new DebugMapEntry(o.getClass().getName(), values)); } @@ -77,6 +89,16 @@ public boolean start(Writer writer) { return result; } + private void allowList(CompoundRoot root) { + root.forEach(this::allowListClass); + } + + private void allowListClass(Object o) { + threadAllowlist.allowClass(o.getClass()); + ClassUtils.getAllSuperclasses(o.getClass()).forEach(threadAllowlist::allowClass); + ClassUtils.getAllInterfaces(o.getClass()).forEach(threadAllowlist::allowClass); + } + @Override public boolean end(Writer writer, String body) { if (showDebug()) { @@ -91,17 +113,17 @@ protected boolean showDebug() { return (devMode || Boolean.TRUE == PrepareOperations.getDevModeOverride()); } - private static class DebugMapEntry implements Map.Entry { - private final Object key; + private static class DebugMapEntry implements Map.Entry { + private final String key; private Object value; - DebugMapEntry(Object key, Object value) { + DebugMapEntry(String key, Object value) { this.key = key; this.value = value; } @Override - public Object getKey() { + public String getKey() { return key; } diff --git a/core/src/main/java/org/apache/struts2/interceptor/ExceptionMappingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ExceptionMappingInterceptor.java index 53570184b0..ab7f9aefe5 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ExceptionMappingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ExceptionMappingInterceptor.java @@ -18,11 +18,14 @@ */ package org.apache.struts2.interceptor; +import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.ActionInvocation; import org.apache.struts2.config.entities.ExceptionMappingConfig; import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.inject.Inject; +import org.apache.struts2.ognl.ThreadAllowlist; import java.util.List; import java.util.Map; @@ -42,11 +45,11 @@ * you make this interceptor the first interceptor on the stack, ensuring that it has full access to catch any * exception, even those caused by other interceptors. *

- * + *

* * *

Interceptor parameters:

- * + *

* * *

    @@ -64,11 +67,11 @@ * The parameters above enables us to log all thrown exceptions with stacktace in our own logfile, * and present a friendly webpage (with no stacktrace) to the end user. *

    - * + *

    * * *

    Extending the interceptor:

    - * + *

    * *

    * If you want to add custom handling for publishing the Exception, you may override @@ -158,11 +161,17 @@ public class ExceptionMappingInterceptor extends AbstractInterceptor { private static final Logger LOG = LogManager.getLogger(ExceptionMappingInterceptor.class); + private ThreadAllowlist threadAllowlist; + protected Logger categoryLogger; protected boolean logEnabled = false; protected String logCategory; protected String logLevel; + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } public boolean isLogEnabled() { return logEnabled; @@ -173,20 +182,20 @@ public void setLogEnabled(boolean logEnabled) { } public String getLogCategory() { - return logCategory; - } + return logCategory; + } - public void setLogCategory(String logCatgory) { - this.logCategory = logCatgory; - } + public void setLogCategory(String logCategory) { + this.logCategory = logCategory; + } - public String getLogLevel() { - return logLevel; - } + public String getLogLevel() { + return logLevel; + } - public void setLogLevel(String logLevel) { - this.logLevel = logLevel; - } + public void setLogLevel(String logLevel) { + this.logLevel = logLevel; + } @Override public String intercept(ActionInvocation invocation) throws Exception { @@ -200,13 +209,16 @@ public String intercept(ActionInvocation invocation) throws Exception { } List exceptionMappings = invocation.getProxy().getConfig().getExceptionMappings(); ExceptionMappingConfig mappingConfig = this.findMappingFromExceptions(exceptionMappings, e); - if (mappingConfig != null && mappingConfig.getResult()!=null) { + if (mappingConfig != null && mappingConfig.getResult() != null) { Map mappingParams = mappingConfig.getParams(); // create a mutable HashMap since some interceptors will remove parameters, and parameterMap is immutable HttpParameters parameters = HttpParameters.create(mappingParams).build(); invocation.getInvocationContext().withParameters(parameters); result = mappingConfig.getResult(); - publishException(invocation, new ExceptionHolder(e)); + ExceptionHolder holder = new ExceptionHolder(e); + threadAllowlist.allowClass(holder.getClass()); + threadAllowlist.allowClass(e.getClass()); + publishException(invocation, holder); } else { throw e; } @@ -221,55 +233,45 @@ public String intercept(ActionInvocation invocation) throws Exception { * @param e the exception to log. */ protected void handleLogging(Exception e) { - if (logCategory != null) { - if (categoryLogger == null) { - // init category logger - categoryLogger = LogManager.getLogger(logCategory); - } - doLog(categoryLogger, e); - } else { - doLog(LOG, e); - } + if (logCategory != null) { + if (categoryLogger == null) { + // init category logger + categoryLogger = LogManager.getLogger(logCategory); + } + doLog(categoryLogger, e); + } else { + doLog(LOG, e); + } } /** * Performs the actual logging. * - * @param logger the provided logger to use. - * @param e the exception to log. + * @param logger the provided logger to use. + * @param e the exception to log. */ protected void doLog(Logger logger, Exception e) { - if (logLevel == null) { - logger.debug(e.getMessage(), e); - return; - } + if (logLevel == null) { + logger.debug(e.getMessage(), e); + return; + } - if ("trace".equalsIgnoreCase(logLevel)) { - logger.trace(e.getMessage(), e); - } else if ("debug".equalsIgnoreCase(logLevel)) { - logger.debug(e.getMessage(), e); - } else if ("info".equalsIgnoreCase(logLevel)) { - logger.info(e.getMessage(), e); - } else if ("warn".equalsIgnoreCase(logLevel)) { - logger.warn(e.getMessage(), e); - } else if ("error".equalsIgnoreCase(logLevel)) { - logger.error(e.getMessage(), e); - } else if ("fatal".equalsIgnoreCase(logLevel)) { - logger.fatal(e.getMessage(), e); - } else { - throw new IllegalArgumentException("LogLevel [" + logLevel + "] is not supported"); - } + Level level = Level.getLevel(logLevel); + if (level == null) { + throw new IllegalArgumentException("LogLevel [" + logLevel + "] is not supported"); + } + logger.log(level, e.getMessage(), e); } /** * Try to find appropriate {@link ExceptionMappingConfig} based on provided Throwable * * @param exceptionMappings list of defined exception mappings - * @param t caught exception + * @param t caught exception * @return appropriate mapping or null */ protected ExceptionMappingConfig findMappingFromExceptions(List exceptionMappings, Throwable t) { - ExceptionMappingConfig config = null; + ExceptionMappingConfig config = null; // Check for specific exception mappings. if (exceptionMappings != null) { int deepest = Integer.MAX_VALUE; @@ -288,15 +290,15 @@ protected ExceptionMappingConfig findMappingFromExceptions(List exceptionClass, int depth) { if (exceptionClass.getName().contains(exceptionMapping)) { // Found it! return depth; @@ -312,7 +314,7 @@ private int getDepth(String exceptionMapping, Class exceptionClass, int depth) { * Default implementation to handle ExceptionHolder publishing. Pushes given ExceptionHolder on the stack. * Subclasses may override this to customize publishing. * - * @param invocation The invocation to publish Exception for. + * @param invocation The invocation to publish Exception for. * @param exceptionHolder The exceptionHolder wrapping the Exception to publish. */ protected void publishException(ActionInvocation invocation, ExceptionHolder exceptionHolder) { diff --git a/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java index 156fcc47a7..4ea98f9b97 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptor.java @@ -18,21 +18,23 @@ */ package org.apache.struts2.interceptor.debugging; -import org.apache.struts2.ActionContext; -import org.apache.struts2.ActionInvocation; -import org.apache.struts2.inject.Inject; -import org.apache.struts2.interceptor.AbstractInterceptor; -import org.apache.struts2.util.ValueStack; -import org.apache.struts2.util.reflection.ReflectionProvider; import jakarta.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.ClassUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.ActionContext; +import org.apache.struts2.ActionInvocation; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsConstants; import org.apache.struts2.dispatcher.DispatcherConstants; import org.apache.struts2.dispatcher.Parameter; import org.apache.struts2.dispatcher.PrepareOperations; import org.apache.struts2.dispatcher.RequestMap; +import org.apache.struts2.inject.Inject; +import org.apache.struts2.interceptor.AbstractInterceptor; +import org.apache.struts2.ognl.ThreadAllowlist; +import org.apache.struts2.util.ValueStack; +import org.apache.struts2.util.reflection.ReflectionProvider; import org.apache.struts2.views.freemarker.FreemarkerManager; import org.apache.struts2.views.freemarker.FreemarkerResult; @@ -101,10 +103,10 @@ public class DebuggingInterceptor extends AbstractInterceptor { private final String[] ignorePrefixes = new String[]{"org.apache.struts.", "org.apache.struts2.", "xwork."}; private final Set ignoreKeys = Set.of( - DispatcherConstants.APPLICATION, - DispatcherConstants.SESSION, - DispatcherConstants.PARAMETERS, - DispatcherConstants.REQUEST + DispatcherConstants.APPLICATION, + DispatcherConstants.SESSION, + DispatcherConstants.PARAMETERS, + DispatcherConstants.REQUEST ); private final static String XML_MODE = "xml"; @@ -126,6 +128,7 @@ public class DebuggingInterceptor extends AbstractInterceptor { private boolean consoleEnabled = false; private ReflectionProvider reflectionProvider; + private ThreadAllowlist threadAllowlist; @Inject(StrutsConstants.STRUTS_DEVMODE) public void setDevMode(String mode) { @@ -142,6 +145,11 @@ public void setReflectionProvider(ReflectionProvider reflectionProvider) { this.reflectionProvider = reflectionProvider; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + /* * (non-Javadoc) * @@ -200,7 +208,7 @@ public String intercept(ActionInvocation inv) throws Exception { res.setContentType("text/plain"); try (PrintWriter writer = - ServletActionContext.getResponse().getWriter()) { + ServletActionContext.getResponse().getWriter()) { writer.print(stack.findValue(cmd)); } catch (IOException ex) { LOG.warn("Interceptor in: {} mode has failed!", COMMAND_MODE, ex); @@ -217,6 +225,7 @@ public String intercept(ActionInvocation inv) throws Exception { String decorate = getParameter(DECORATE_PARAM); ValueStack stack = ctx.getValueStack(); Object rootObject = stack.findValue(rootObjectExpression); + allowListClass(rootObject); try (StringWriter writer = new StringWriter()) { ObjectToHTMLWriter htmlWriter = new ObjectToHTMLWriter(writer); @@ -228,8 +237,9 @@ public String intercept(ActionInvocation inv) throws Exception { //on the first request, response can be decorated //but we need plain text on the other ones - if ("false".equals(decorate)) + if ("false".equals(decorate)) { ServletActionContext.getRequest().setAttribute("decorator", "none"); + } FreemarkerResult result = new FreemarkerResult(); result.setFreemarkerManager(freemarkerManager); @@ -239,7 +249,6 @@ public String intercept(ActionInvocation inv) throws Exception { } catch (Exception ex) { LOG.error("Unable to create debugging console", ex); } - }); } } @@ -262,6 +271,14 @@ public String intercept(ActionInvocation inv) throws Exception { } } + private void allowListClass(Object o) { + if (o != null) { + threadAllowlist.allowClass(o.getClass()); + ClassUtils.getAllSuperclasses(o.getClass()).forEach(threadAllowlist::allowClass); + ClassUtils.getAllInterfaces(o.getClass()).forEach(threadAllowlist::allowClass); + } + } + /** * Gets a single string from the request parameters * @@ -277,12 +294,11 @@ private String getParameter(String key) { * Prints the current context to the response in XML format. */ protected void printContext() { - HttpServletResponse res = ServletActionContext.getResponse(); - res.setContentType("text/xml"); + HttpServletResponse response = ActionContext.getContext().getServletResponse(); + response.setContentType("text/xml"); try { - PrettyPrintWriter writer = new PrettyPrintWriter( - ServletActionContext.getResponse().getWriter()); + PrettyPrintWriter writer = new PrettyPrintWriter(response.getWriter()); printContext(writer); writer.close(); } catch (IOException ex) { @@ -311,6 +327,7 @@ protected void printContext(PrettyPrintWriter writer) { } } if (print) { + allowListClass(ctxMap.get(key)); serializeIt(ctxMap.get(key), key, writer, new ArrayList<>()); } } @@ -426,5 +443,3 @@ private List filterValueStack(Map requestMap) { return filter; } } - - diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java index 45a291d2f8..9c266645ba 100644 --- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java @@ -25,8 +25,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; -import org.apache.struts2.ognl.ProviderAllowlist; -import org.apache.struts2.ognl.ThreadAllowlist; import java.lang.reflect.AccessibleObject; import java.lang.reflect.Constructor; diff --git a/core/src/main/resources/org/apache/struts2/interceptor/debugging/browser.ftl b/core/src/main/resources/org/apache/struts2/interceptor/debugging/browser.ftl index 3f0dbb339f..a398a94bfe 100644 --- a/core/src/main/resources/org/apache/struts2/interceptor/debugging/browser.ftl +++ b/core/src/main/resources/org/apache/struts2/interceptor/debugging/browser.ftl @@ -19,8 +19,8 @@ */ --> - - <@s.style> + + - <@s.script> + ${debugHtml?no_esc} diff --git a/core/src/main/resources/org/apache/struts2/interceptor/debugging/webconsole.html b/core/src/main/resources/org/apache/struts2/interceptor/debugging/webconsole.html index bb9e38958c..3baca58ac1 100644 --- a/core/src/main/resources/org/apache/struts2/interceptor/debugging/webconsole.html +++ b/core/src/main/resources/org/apache/struts2/interceptor/debugging/webconsole.html @@ -23,21 +23,25 @@ - - - OGNL Console + + + OGNL Console
    -
    - Welcome to the OGNL console! -
    - :-> -
    -
    - - -
    +
    + Welcome to the OGNL console! +
    + :-> +
    +
    + + + +
    diff --git a/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java b/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java index 81403bdcee..1a3f0daf48 100644 --- a/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java +++ b/core/src/test/java/org/apache/struts2/StrutsJUnit4InternalTestCase.java @@ -18,8 +18,6 @@ */ package org.apache.struts2; -import org.apache.struts2.ActionProxyFactory; -import org.apache.struts2.XWorkJUnit4TestCase; import org.apache.struts2.dispatcher.Dispatcher; import org.apache.struts2.util.StrutsTestCaseHelper; import org.apache.struts2.views.jsp.StrutsMockServletContext; diff --git a/core/src/test/java/org/apache/struts2/interceptor/ExceptionMappingInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ExceptionMappingInterceptorTest.java index e5f922fdf9..bc085e54bc 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ExceptionMappingInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ExceptionMappingInterceptorTest.java @@ -19,16 +19,17 @@ package org.apache.struts2.interceptor; import com.mockobjects.dynamic.Mock; -import org.apache.struts2.action.Action; import org.apache.struts2.ActionContext; import org.apache.struts2.ActionInvocation; import org.apache.struts2.ActionProxy; +import org.apache.struts2.StrutsException; import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.action.Action; import org.apache.struts2.config.entities.ActionConfig; import org.apache.struts2.config.entities.ExceptionMappingConfig; +import org.apache.struts2.ognl.ThreadAllowlist; import org.apache.struts2.util.ValueStack; import org.apache.struts2.validator.ValidationException; -import org.apache.struts2.StrutsException; /** * Unit test for ExceptionMappingInterceptor. @@ -52,7 +53,7 @@ public void testThrownExceptionMatching() throws Exception { String result = interceptor.intercept(invocation); assertNotNull(stack.findValue("exception")); assertEquals(stack.findValue("exception"), exception); - assertEquals(result, "spooky"); + assertEquals("spooky", result); ExceptionHolder holder = (ExceptionHolder) stack.getRoot().get(0); // is on top of the root assertNotNull(holder.getExceptionStack()); // to invoke the method for unit test } @@ -67,7 +68,7 @@ public void testThrownExceptionMatching2() throws Exception { String result = interceptor.intercept(invocation); assertNotNull(stack.findValue("exception")); assertEquals(stack.findValue("exception"), exception); - assertEquals(result, "throwable"); + assertEquals("throwable", result); } public void testNoThrownException() throws Exception { @@ -77,7 +78,7 @@ public void testNoThrownException() throws Exception { mockInvocation.expectAndReturn("invoke", Action.SUCCESS); mockInvocation.matchAndReturn("getAction", action.proxy()); String result = interceptor.intercept(invocation); - assertEquals(result, Action.SUCCESS); + assertEquals(Action.SUCCESS, result); assertNull(stack.findValue("exception")); } @@ -106,7 +107,7 @@ public void testThrownExceptionNoMatchLogging() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); + interceptor.setLogEnabled(true); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -123,8 +124,8 @@ public void testThrownExceptionNoMatchLoggingCategory() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -141,9 +142,9 @@ public void testThrownExceptionNoMatchLoggingCategoryLevelFatal() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); - interceptor.setLogLevel("fatal"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogLevel("fatal"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -164,9 +165,9 @@ public void testThrownExceptionNoMatchLoggingCategoryLevelError() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); - interceptor.setLogLevel("error"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogLevel("error"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -183,9 +184,9 @@ public void testThrownExceptionNoMatchLoggingCategoryLevelWarn() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); - interceptor.setLogLevel("warn"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogLevel("warn"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -202,9 +203,9 @@ public void testThrownExceptionNoMatchLoggingCategoryLevelInfo() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); - interceptor.setLogLevel("info"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogLevel("info"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -221,9 +222,9 @@ public void testThrownExceptionNoMatchLoggingCategoryLevelDebug() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); - interceptor.setLogLevel("debug"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogLevel("debug"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -240,9 +241,9 @@ public void testThrownExceptionNoMatchLoggingCategoryLevelTrace() { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogCategory("showcase.unhandled"); - interceptor.setLogLevel("trace"); + interceptor.setLogEnabled(true); + interceptor.setLogCategory("showcase.unhandled"); + interceptor.setLogLevel("trace"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (Exception e) { @@ -259,12 +260,12 @@ public void testThrownExceptionNoMatchLoggingUnknownLevel() throws Exception { mockInvocation.matchAndReturn("getAction", action.proxy()); try { - interceptor.setLogEnabled(true); - interceptor.setLogLevel("xxx"); + interceptor.setLogEnabled(true); + interceptor.setLogLevel("xxx"); interceptor.intercept(invocation); fail("Should not have reached this point."); } catch (IllegalArgumentException e) { - // success + // success } } @@ -296,6 +297,7 @@ protected void setUp() throws Exception { mockInvocation.expectAndReturn("getStack", stack); mockInvocation.expectAndReturn("getInvocationContext", ActionContext.of().bind()); interceptor = new ExceptionMappingInterceptor(); + interceptor.setThreadAllowlist(new ThreadAllowlist()); interceptor.init(); } diff --git a/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java new file mode 100644 index 0000000000..e2f5260d29 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java @@ -0,0 +1,229 @@ +package org.apache.struts2.interceptor.debugging; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.StrutsJUnit4InternalTestCase; +import org.apache.struts2.TestAction; +import org.apache.struts2.dispatcher.DispatcherConstants; +import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.RequestMap; +import org.apache.struts2.dispatcher.SessionMap; +import org.apache.struts2.mock.MockActionInvocation; +import org.apache.struts2.util.ValueStack; +import org.assertj.core.util.Maps; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockHttpSession; + +import java.util.HashMap; +import java.util.Map; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +public class DebuggingInterceptorTest extends StrutsJUnit4InternalTestCase { + + private DebuggingInterceptor interceptor; + private MockActionInvocation invocation; + private MockHttpServletRequest request; + private MockHttpServletResponse response; + private ActionContext context; + + @Test + public void noDevMode() throws Exception { + interceptor.intercept(invocation); + assertThat(invocation.getResultCode()).isEqualTo("mock"); + } + + @Test + public void debugXml() throws Exception { + interceptor.setDevMode("true"); + context.withParameters(HttpParameters.create(Maps.newHashMap("debug", "xml")).build()); + + interceptor.intercept(invocation); + + assertThat(response.getContentAsString()).isEqualToIgnoringWhitespace(""" + + + + + + + + class org.apache.struts2.text.DefaultTextProvider + + + + """); + } + + @Test + public void debugConsole() throws Exception { + interceptor.setDevMode("true"); + context.withParameters(HttpParameters.create(Maps.newHashMap("debug", "console")).build()); + + interceptor.intercept(invocation); + + assertThat(response.getContentAsString()).isEqualToIgnoringWhitespace(""" + + + + + + + 
    +                
+
    + + + """); + } + + @Test + public void debugCommand() throws Exception { + interceptor.setDevMode("true"); + Map params = new HashMap<>() {{ + put("debug", "command"); + put("expression", "1+1"); + }}; + context.withParameters(HttpParameters.create(params).build()); + + interceptor.intercept(invocation); + + assertThat(response.getContentAsString()).isEqualToIgnoringWhitespace("2"); + } + + @Test + public void debugBrowser() throws Exception { + interceptor.setDevMode("true"); + context.withParameters(HttpParameters.create(Maps.newHashMap("debug", "browser")).build()); + + interceptor.intercept(invocation); + invocation.invoke(); + + assertThat(response.getContentAsString()).isEqualToIgnoringWhitespace(""" + + + + + + + + + + + + + +
    rootnullunknown
    + + + """); + } + + @Before + public void setUp() throws Exception { + super.setUp(); + request = new MockHttpServletRequest(); + request.setSession(new MockHttpSession()); + response = new MockHttpServletResponse(); + + ValueStack valueStack = dispatcher.getValueStackFactory().createValueStack(); + + context = valueStack.getActionContext() + .withServletContext(servletContext) + .withServletRequest(request) + .withServletResponse(response) + .withSession(new SessionMap(request)) + .with(DispatcherConstants.REQUEST, new RequestMap(request)) + .withActionInvocation(invocation) + .bind(); + + interceptor = container.inject(DebuggingInterceptor.class); + interceptor.init(); + + invocation = new MockActionInvocation(); + invocation.setResultCode("mock"); + invocation.setInvocationContext(context); + invocation.setAction(new TestAction()); + invocation.setStack(valueStack); + } + + @After + public void tearDown() throws Exception { + super.tearDown(); + interceptor.destroy(); + interceptor = null; + invocation = null; + + servletContext = null; + request = null; + response = null; + } + +} \ No newline at end of file diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/DebugTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/DebugTagTest.java index a4ca5a94a5..8e53b1b09e 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/DebugTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/DebugTagTest.java @@ -18,7 +18,6 @@ */ package org.apache.struts2.views.jsp.ui; -import org.apache.struts2.config.ConfigurationException; import org.apache.commons.lang3.StringUtils; import org.apache.struts2.StrutsConstants; import org.apache.struts2.dispatcher.PrepareOperations; @@ -59,7 +58,7 @@ public void testDevModeEnabled() throws Exception { freshTag.setPageContext(pageContext); // DebugTag has no additional state, so it compares as equal with the default tag clear state as well. assertTrue("Tag state after doEndTag() under default tag clear state is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -82,7 +81,7 @@ public void testDevModeEnabled_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -98,7 +97,7 @@ public void testDevModeDisabled() throws Exception { freshTag.setPageContext(pageContext); // DebugTag has no additional state, so it compares as equal with the default tag clear state as well. assertTrue("Tag state after doEndTag() under default tag clear state is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -116,7 +115,7 @@ public void testDevModeDisabled_clearTagStateSet() throws Exception { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -135,7 +134,7 @@ public void testTagAttributeOverrideDevModeTrue() throws Exception { freshTag.setPageContext(pageContext); // DebugTag has no additional state, so it compares as equal with the default tag clear state as well. assertTrue("Tag state after doEndTag() under default tag clear state is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); PrepareOperations.clearDevModeOverride(); // Clear DevMode override. Avoid ThreadLocal side-effects if test thread re-used. @@ -158,7 +157,7 @@ public void testTagAttributeOverrideDevModeTrue_clearTagStateSet() throws Except freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); PrepareOperations.clearDevModeOverride(); // Clear DevMode override. Avoid ThreadLocal side-effects if test thread re-used. @@ -177,7 +176,7 @@ public void testTagAttributeOverrideDevModeFalse() throws Exception { freshTag.setPageContext(pageContext); // DebugTag has no additional state, so it compares as equal with the default tag clear state as well. assertTrue("Tag state after doEndTag() under default tag clear state is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); PrepareOperations.clearDevModeOverride(); // Clear DevMode override. Avoid ThreadLocal side-effects if test thread re-used. @@ -198,14 +197,14 @@ public void testTagAttributeOverrideDevModeFalse_clearTagStateSet() throws Excep freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); PrepareOperations.clearDevModeOverride(); // Clear DevMode override. Avoid ThreadLocal side-effects if test thread re-used. } private void setDevMode(final boolean devMode) { - setStrutsConstant(new HashMap() {{ + setStrutsConstant(new HashMap<>() {{ put(StrutsConstants.STRUTS_DEVMODE, Boolean.toString(devMode)); }}); }