This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch fix/WW-5690-lazy-error-template in repository https://gitbox.apache.org/repos/asf/struts.git
commit 7497fe0f5d72934c0d3af0b069783545112d89c9 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Aug 24 11:56:00 2026 +0200 WW-5690 perf(dispatcher): load the dev-mode error template on first use DefaultDispatcherErrorHandler.init() built a FreeMarker configuration and loaded /org/apache/struts2/dispatcher/error.ftl on every startup, including in production where the problem report is never rendered - handleError() delegates to the container's error page unless devMode is on. Defer the load to the first dev-mode error instead. Two threads racing there may both load the template, which is harmless: FreeMarker caches templates in its own configuration, and that is cheaper than locking a path taken once per application. One behavioural change: a missing or unparsable error.ftl used to fail the application at boot. It now surfaces on the first dev-mode error, where the existing catch in handleErrorInDevMode() degrades to sendError() with the cause. A dev-only template should not stop a production application starting. This removes the startup cost only; the FreeMarker dependency itself stays. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../dispatcher/DefaultDispatcherErrorHandler.java | 34 ++++++++++--- .../DefaultDispatcherErrorHandlerTest.java | 59 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 8 deletions(-) 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 d64345854..290db7753 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.commons.lang3.BooleanUtils; 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,9 +47,12 @@ 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 Template template; + private ServletContext servletContext; + private volatile Template template; @Inject public void setFreemarkerManager(FreemarkerManager freemarkerManager) { @@ -63,12 +65,28 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { } 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. + * <p> + * Two threads racing on the very first error may both load the template. That is harmless - + * FreeMarker caches templates in its own configuration - and cheaper than locking a path taken + * once per application. + * + * @return the problem report template + * @throws IOException if the template cannot be loaded + */ + protected Template getTemplate() throws IOException { + Template result = template; + if (result == null) { + result = freemarkerManager.getConfiguration(servletContext).getTemplate(ERROR_TEMPLATE); + template = result; } + return result; } public void handleError(HttpServletRequest request, HttpServletResponse response, int code, Exception e) { @@ -115,7 +133,7 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { } 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 6d44c7dac..f1f521047 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 class DefaultDispatcherErrorHandlerTest extends StrutsInternalTestCase { 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);
