diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java index d643458541..779ca7cd1f 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java @@ -30,7 +30,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; -import org.apache.struts2.StrutsException; import org.apache.struts2.views.freemarker.FreemarkerManager; import java.io.IOException; @@ -48,8 +47,11 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { private static final Logger LOG = LogManager.getLogger(DefaultDispatcherErrorHandler.class); + private static final String ERROR_TEMPLATE = "/org/apache/struts2/dispatcher/error.ftl"; + private FreemarkerManager freemarkerManager; private boolean devMode; + private ServletContext servletContext; private Template template; @Inject @@ -63,12 +65,26 @@ public void setDevMode(String devMode) { } public void init(ServletContext ctx) { - try { - freemarker.template.Configuration config = freemarkerManager.getConfiguration(ctx); - template = config.getTemplate("/org/apache/struts2/dispatcher/error.ftl"); - } catch (IOException e) { - throw new StrutsException(e); + this.servletContext = ctx; + } + + /** + * Loads the problem report template on first use rather than at startup: it is only ever + * rendered in devMode, so a production application should never build a FreeMarker + * configuration on its behalf. + *
+ * Synchronised rather than lock-free: this runs only when devMode is on and a request has + * already failed, and {@link FreemarkerManager#getConfiguration(ServletContext)} is itself + * synchronised, so the lock costs nothing that this path was not paying already. + * + * @return the problem report template + * @throws IOException if the template cannot be loaded + */ + protected synchronized Template getTemplate() throws IOException { + if (template == null) { + template = freemarkerManager.getConfiguration(servletContext).getTemplate(ERROR_TEMPLATE); } + return template; } public void handleError(HttpServletRequest request, HttpServletResponse response, int code, Exception e) { @@ -115,7 +131,7 @@ protected void handleErrorInDevMode(HttpServletResponse response, int code, Exce } while ((cur = cur.getCause()) != null); Writer writer = new StringWriter(); - template.process(createReportData(e, chain), writer); + getTemplate().process(createReportData(e, chain), writer); response.setContentType("text/html"); response.getWriter().write(writer.toString()); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java index 6d44c7dac0..f1f5210477 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java @@ -18,10 +18,14 @@ */ package org.apache.struts2.dispatcher; +import freemarker.template.Configuration; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.util.Collections; import org.apache.struts2.StrutsInternalTestCase; +import jakarta.servlet.ServletContext; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.apache.struts2.views.freemarker.FreemarkerManager; @@ -130,6 +134,61 @@ public void testHandleErrorIllegalStateExceptionDevMode() { defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException); } + /** + * The dev-mode problem report is the only thing that needs FreeMarker here, so booting the + * application must not build a FreeMarker configuration just to have the template ready. + */ + public void testInitDoesNotLoadErrorTemplate() { + RecordingFreemarkerManager freemarkerManager = createFreemarkerManager(); + DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler(); + defaultDispatcherErrorHandler.setDevMode("true"); + defaultDispatcherErrorHandler.setFreemarkerManager(freemarkerManager); + + defaultDispatcherErrorHandler.init(dispatcher.servletContext); + + assertFalse("init() must not touch FreeMarker", freemarkerManager.configurationRequested); + } + + /** + * The deferred load must still happen, otherwise the problem report silently stops rendering. + */ + public void testErrorTemplateLoadedOnFirstDevModeError() throws IOException { + RecordingFreemarkerManager freemarkerManager = createFreemarkerManager(); + DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler(); + defaultDispatcherErrorHandler.setDevMode("true"); + defaultDispatcherErrorHandler.setFreemarkerManager(freemarkerManager); + defaultDispatcherErrorHandler.init(dispatcher.servletContext); + Exception fakeException = new Exception("Fake Exception, devMode true"); + responseMock.setContentType("text/html"); + expectLastCall(); + expect(responseMock.getWriter()).andStubReturn(new PrintWriter(new StringWriter())); + replay(responseMock); + + defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException); + + assertTrue("problem report must load the template on first use", freemarkerManager.configurationRequested); + } + + private RecordingFreemarkerManager createFreemarkerManager() { + RecordingFreemarkerManager freemarkerManager = new RecordingFreemarkerManager(); + container.inject(freemarkerManager); + return freemarkerManager; + } + + /** + * Records whether the FreeMarker configuration was ever asked for, while still delegating so the + * template really renders. + */ + private static class RecordingFreemarkerManager extends FreemarkerManager { + private boolean configurationRequested; + + @Override + public Configuration getConfiguration(ServletContext servletContext) { + configurationRequested = true; + return super.getConfiguration(servletContext); + } + } + protected void setUp() { requestMock = (HttpServletRequest) createMock(HttpServletRequest.class); responseMock = (HttpServletResponse) createMock(HttpServletResponse.class);