From f3997f705eccaf42feb949b7300e3adaef1e8cb4 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 | 552 ++++++++++++++++++ .../struts2/views/jsp/ui/DebugTagTest.java | 19 +- 10 files changed, 737 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 ddd228bf2..702f9e32b 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 class Debug extends UIBean { this.reflectionProvider = prov; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + protected String getDefaultTemplate() { return TEMPLATE; } @@ -59,16 +68,19 @@ public class Debug extends UIBean { 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 class Debug extends UIBean { 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 @@ public class Debug extends UIBean { 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 53570184b..b64bd81e1 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 @@ import java.util.Map; * 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 @@ import java.util.Map; * 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 transient 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 class ExceptionMappingInterceptor extends AbstractInterceptor { } 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 class ExceptionMappingInterceptor extends AbstractInterceptor { } 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 class ExceptionMappingInterceptor extends AbstractInterceptor { * @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 @@ public class ExceptionMappingInterceptor extends AbstractInterceptor { * Return the depth to the superclass matching. 0 means ex matches exactly. Returns -1 if there's no match. * Otherwise, returns depth. Lowest depth wins. * - * @param exceptionMapping the mapping classname - * @param t the cause + * @param exceptionMapping the mapping classname + * @param t the cause * @return the depth, if not found -1 is returned. */ public int getDepth(String exceptionMapping, Throwable t) { return getDepth(exceptionMapping, t.getClass(), 0); } - private int getDepth(String exceptionMapping, Class exceptionClass, int depth) { + private int getDepth(String exceptionMapping, Class exceptionClass, int depth) { if (exceptionClass.getName().contains(exceptionMapping)) { // Found it! return depth; @@ -312,7 +314,7 @@ public class ExceptionMappingInterceptor extends AbstractInterceptor { * 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 156fcc47a..1fa9f7b18 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 transient ThreadAllowlist threadAllowlist; @Inject(StrutsConstants.STRUTS_DEVMODE) public void setDevMode(String mode) { @@ -142,6 +145,11 @@ public class DebuggingInterceptor extends AbstractInterceptor { this.reflectionProvider = reflectionProvider; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + /* * (non-Javadoc) * @@ -200,7 +208,7 @@ public class DebuggingInterceptor extends AbstractInterceptor { 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 class DebuggingInterceptor extends AbstractInterceptor { 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 class DebuggingInterceptor extends AbstractInterceptor { //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 class DebuggingInterceptor extends AbstractInterceptor { } catch (Exception ex) { LOG.error("Unable to create debugging console", ex); } - }); } } @@ -262,6 +271,14 @@ public class DebuggingInterceptor extends AbstractInterceptor { } } + 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 @@ public class DebuggingInterceptor extends AbstractInterceptor { * 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 @@ public class DebuggingInterceptor extends AbstractInterceptor { } } if (print) { + allowListClass(ctxMap.get(key)); serializeIt(ctxMap.get(key), key, writer, new ArrayList<>()); } } @@ -426,5 +443,3 @@ public class DebuggingInterceptor extends AbstractInterceptor { 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 45a291d2f..9c266645b 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.commons.lang3.BooleanUtils; 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 3f0dbb339..a398a94bf 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 bb9e38958..3baca58ac 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 81403bdce..1a3f0daf4 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 e5f922fdf..bc085e54b 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 @@ public class ExceptionMappingInterceptorTest extends XWorkTestCase { 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 000000000..ec3bd4ad4 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java @@ -0,0 +1,552 @@ +/* + * 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 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"); + assertThat(response.getContentAsString()).isEmpty(); + } + + @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.TestAction + + + USA + eng + class java.util.Locale + US + United States + English + English (United States) + + + + en + + + + + + + class org.apache.struts2.SomeEnum + class org.apache.struts2.SomeEnum + completed + COMPLETED + + + + class org.apache.struts2.SomeEnum + class org.apache.struts2.SomeEnum + init + INIT + + + class org.apache.struts2.SomeEnum + class org.apache.struts2.SomeEnum + completed + COMPLETED + + + + org.apache.struts2.TestAction + class java.util.PropertyResourceBundle + + class sun.util.ResourceBundleEnumeration + + + + + class java.util.Locale + + + + + + + + + + + + + + + + + + + class org.apache.struts2.text.DefaultTextProvider + + + + """); + } + + @Test + public void debugXmlWithConsole() throws Exception { + interceptor.setDevMode("true"); + context.withParameters(HttpParameters.create(Maps.newHashMap("debug", "console")).build()); + interceptor.setEnableXmlWithConsole(true); + + interceptor.intercept(invocation); + + assertThat(response.getContentAsString()).isEqualToIgnoringWhitespace(""" + + + + + + + 
    +                    &lt;debug&gt;
+                  &lt;parameters/&gt;
+                  &lt;context/&gt;
+                  &lt;request/&gt;
+                  &lt;session/&gt;
+                  &lt;valueStack&gt;
+                    &lt;value&gt;
+                      &lt;action&gt;
+                        &lt;actionErrors/&gt;
+                        &lt;actionMessages/&gt;
+                        &lt;class&gt;class org.apache.struts2.TestAction&lt;/class&gt;
+                        &lt;fieldErrors/&gt;
+                        &lt;locale&gt;
+                          &lt;ISO3Country&gt;USA&lt;/ISO3Country&gt;
+                          &lt;ISO3Language&gt;eng&lt;/ISO3Language&gt;
+                          &lt;class&gt;class java.util.Locale&lt;/class&gt;
+                          &lt;country&gt;US&lt;/country&gt;
+                          &lt;displayCountry&gt;United States&lt;/displayCountry&gt;
+                          &lt;displayLanguage&gt;English&lt;/displayLanguage&gt;
+                          &lt;displayName&gt;English (United States)&lt;/displayName&gt;
+                          &lt;displayScript&gt;&lt;/displayScript&gt;
+                          &lt;displayVariant&gt;&lt;/displayVariant&gt;
+                          &lt;extensionKeys/&gt;
+                          &lt;language&gt;en&lt;/language&gt;
+                          &lt;script&gt;&lt;/script&gt;
+                          &lt;unicodeLocaleAttributes/&gt;
+                          &lt;unicodeLocaleKeys/&gt;
+                          &lt;variant&gt;&lt;/variant&gt;
+                        &lt;/locale&gt;
+                        &lt;status&gt;
+                          &lt;class&gt;class org.apache.struts2.SomeEnum&lt;/class&gt;
+                          &lt;declaringClass&gt;class org.apache.struts2.SomeEnum&lt;/declaringClass&gt;
+                          &lt;displayName&gt;completed&lt;/displayName&gt;
+                          &lt;name&gt;COMPLETED&lt;/name&gt;
+                        &lt;/status&gt;
+                        &lt;statusList&gt;
+                          &lt;value&gt;
+                            &lt;class&gt;class org.apache.struts2.SomeEnum&lt;/class&gt;
+                            &lt;declaringClass&gt;class org.apache.struts2.SomeEnum&lt;/declaringClass&gt;
+                            &lt;displayName&gt;init&lt;/displayName&gt;
+                            &lt;name&gt;INIT&lt;/name&gt;
+                          &lt;/value&gt;
+                          &lt;value&gt;
+                            &lt;class&gt;class org.apache.struts2.SomeEnum&lt;/class&gt;
+                            &lt;declaringClass&gt;class org.apache.struts2.SomeEnum&lt;/declaringClass&gt;
+                            &lt;displayName&gt;completed&lt;/displayName&gt;
+                            &lt;name&gt;COMPLETED&lt;/name&gt;
+                          &lt;/value&gt;
+                        &lt;/statusList&gt;
+                        &lt;texts&gt;
+                          &lt;baseBundleName&gt;org.apache.struts2.TestAction&lt;/baseBundleName&gt;
+                          &lt;class&gt;class java.util.PropertyResourceBundle&lt;/class&gt;
+                          &lt;keys&gt;
+                            &lt;class&gt;class sun.util.ResourceBundleEnumeration&lt;/class&gt;
+                          &lt;/keys&gt;
+                          &lt;locale&gt;
+                            &lt;ISO3Country&gt;&lt;/ISO3Country&gt;
+                            &lt;ISO3Language&gt;&lt;/ISO3Language&gt;
+                            &lt;class&gt;class java.util.Locale&lt;/class&gt;
+                            &lt;country&gt;&lt;/country&gt;
+                            &lt;displayCountry&gt;&lt;/displayCountry&gt;
+                            &lt;displayLanguage&gt;&lt;/displayLanguage&gt;
+                            &lt;displayName&gt;&lt;/displayName&gt;
+                            &lt;displayScript&gt;&lt;/displayScript&gt;
+                            &lt;displayVariant&gt;&lt;/displayVariant&gt;
+                            &lt;extensionKeys/&gt;
+                            &lt;language&gt;&lt;/language&gt;
+                            &lt;script&gt;&lt;/script&gt;
+                            &lt;unicodeLocaleAttributes/&gt;
+                            &lt;unicodeLocaleKeys/&gt;
+                            &lt;variant&gt;&lt;/variant&gt;
+                          &lt;/locale&gt;
+                        &lt;/texts&gt;
+                      &lt;/action&gt;
+                      &lt;org.apache.struts2.util.OgnlValueStack.MAP_IDENTIFIER_KEY&gt;&lt;/org.apache.struts2.util.OgnlValueStack.MAP_IDENTIFIER_KEY&gt;
+                    &lt;/value&gt;
+                    &lt;value&gt;
+                      &lt;class&gt;class org.apache.struts2.text.DefaultTextProvider&lt;/class&gt;
+                    &lt;/value&gt;
+                  &lt;/valueStack&gt;
+                &lt;/debug&gt;
+
    + + + """); + } + + @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(""" + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    containerThere is no read method for containerjava.lang.String
    foonullunknown
    intListnullunknown
    locale + Expand + java.util.Locale
    resultnullunknown
    collection2nullunknown
    someBoolnullunknown
    arraynullunknown
    fooIntnullunknown
    idnullunknown
    mapnullunknown
    actionErrorsemptyjava.util.LinkedList
    objectArraynullunknown
    fieldErrorsemptyjava.util.LinkedHashMap
    collectionnullunknown
    floatNumbernullunknown
    listnullunknown
    enumListnullunknown
    actionMessagesemptyjava.util.LinkedList
    statusList + Expand + java.util.Arrays$ArrayList
    texts + Expand + java.util.PropertyResourceBundle
    list3nullunknown
    list2nullunknown
    usernullunknown
    status + Expand + org.apache.struts2.SomeEnum
    + + + """); + } + + @Before + public void before() { + 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)); + + interceptor = container.inject(DebuggingInterceptor.class); + interceptor.init(); + + invocation = new MockActionInvocation(); + invocation.setResultCode("mock"); + invocation.setInvocationContext(context); + invocation.setAction(new TestAction()); + invocation.setStack(valueStack); + + valueStack.set("action", invocation.getAction()); + + context = context.withActionInvocation(invocation).bind(); + } + + @After + public void after() { + 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 a4ca5a94a..8e53b1b09 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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 class DebugTagTest extends AbstractUITagTest { 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)); }}); }