From 5949acf9c6e64c1ffc83f989735b177ffdcad3a5 Mon Sep 17 00:00:00 2001 From: junuu Date: Wed, 21 Jan 2026 21:02:01 +0900 Subject: [PATCH] Add logOnError option to ErrorReportValve --- .../catalina/valves/ErrorReportValve.java | 30 +++++++ .../catalina/valves/LocalStrings.properties | 1 + .../catalina/valves/TestErrorReportValve.java | 79 +++++++++++++++++++ webapps/docs/config/valve.xml | 9 +++ 4 files changed, 119 insertions(+) diff --git a/java/org/apache/catalina/valves/ErrorReportValve.java b/java/org/apache/catalina/valves/ErrorReportValve.java index 1f4a9974279f..722f0f18c32b 100644 --- a/java/org/apache/catalina/valves/ErrorReportValve.java +++ b/java/org/apache/catalina/valves/ErrorReportValve.java @@ -40,6 +40,7 @@ import org.apache.tomcat.util.descriptor.web.ErrorPage; import org.apache.tomcat.util.res.StringManager; import org.apache.tomcat.util.security.Escape; +import org.apache.catalina.valves.Constants; /** * Implementation of a Valve that outputs HTML error pages. @@ -50,10 +51,14 @@ */ public class ErrorReportValve extends ValveBase { + protected static final StringManager sm = StringManager.getManager(Constants.Package); + private boolean showReport = true; private boolean showServerInfo = true; + private boolean logOnError = false; + private final ErrorPageSupport errorPageSupport = new ErrorPageSupport(); @@ -189,6 +194,18 @@ protected void report(Request request, Response response, Throwable throwable) { return; } + // Log error if enabled + if (isLogOnError()) { + container.getLogger().error( + sm.getString("errorReportValve.errorLogged", + request.getCoyoteRequest().getMethod(), + request.getCoyoteRequest().requestURI(), + request.getCoyoteRequest().decodedURI(), + request.getCoyoteRequest().queryString(), + request.getCoyoteRequest().getMimeHeaders().toString()), + throwable); + } + ErrorPage errorPage = findErrorPage(statusCode, throwable); if (errorPage != null) { @@ -412,6 +429,19 @@ public boolean isShowServerInfo() { return showServerInfo; } + /** + * Enables/Disables error logging when an error page is generated + * + * @param logOnError true to log errors when error pages are generated + */ + public void setLogOnError(boolean logOnError) { + this.logOnError = logOnError; + } + + public boolean isLogOnError() { + return logOnError; + } + public boolean setProperty(String name, String value) { if (name.startsWith("errorCode.")) { diff --git a/java/org/apache/catalina/valves/LocalStrings.properties b/java/org/apache/catalina/valves/LocalStrings.properties index 35a458f4d49c..2a17b6149c00 100644 --- a/java/org/apache/catalina/valves/LocalStrings.properties +++ b/java/org/apache/catalina/valves/LocalStrings.properties @@ -43,6 +43,7 @@ errorReportValve.statusHeader=HTTP Status {0} – {1} errorReportValve.statusReport=Status Report errorReportValve.type=Type errorReportValve.unknownReason=Unknown Reason +errorReportValve.errorLogged=Error page generated for request: method [{0}], URI [{1}], decoded URI [{2}], query string [{3}], headers [{4}] extendedAccessLogValve.badXParam=Invalid x parameter format, needs to be 'x-#(...) extendedAccessLogValve.badXParamValue=Invalid x parameter value for Servlet request [{0}] diff --git a/test/org/apache/catalina/valves/TestErrorReportValve.java b/test/org/apache/catalina/valves/TestErrorReportValve.java index d4bdee420f37..eb3a52cb61d4 100644 --- a/test/org/apache/catalina/valves/TestErrorReportValve.java +++ b/test/org/apache/catalina/valves/TestErrorReportValve.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.logging.Level; import jakarta.servlet.AsyncContext; import jakarta.servlet.RequestDispatcher; @@ -32,11 +33,13 @@ import org.junit.Test; import org.apache.catalina.Context; +import org.apache.catalina.Host; import org.apache.catalina.Wrapper; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.descriptor.web.ErrorPage; +import org.apache.tomcat.unittest.TesterLogValidationFilter; import org.apache.tomcat.util.res.StringManager; public class TestErrorReportValve extends TomcatBaseTest { @@ -262,5 +265,81 @@ public void testErrorPageServlet() throws Exception { Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); } + @Test + public void testLogOnErrorEnabled() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + // Setup ErrorReportValve with logOnError enabled + Host host = tomcat.getHost(); + ErrorReportValve errorReportValve = new ErrorReportValve(); + errorReportValve.setLogOnError(true); + host.getPipeline().addValve(errorReportValve); + + // No file system docBase required + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "errorServlet", new ErrorServlet()); + ctx.addServletMappingDecoded("/", "errorServlet"); + + tomcat.start(); + + // Setup log filter to capture error logs + TesterLogValidationFilter filter = TesterLogValidationFilter.add( + Level.SEVERE, + "Error page generated for request", + null, + host.getLogName()); + + ByteChunk res = new ByteChunk(); + res.setCharset(StandardCharsets.UTF_8); + getUrl("http://localhost:" + getPort(), res, null); + + // Verify error page was generated + Assert.assertTrue(res.toString().contains( + "

" + sm.getString("errorReportValve.message") + " " + ErrorServlet.ERROR_TEXT + "

")); + + // Verify error was logged + int messageCount = filter.getMessageCount(); + Assert.assertTrue("Error should be logged when logOnError is enabled", + messageCount > 0); + } + + @Test + public void testLogOnErrorDisabled() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + // Setup ErrorReportValve with logOnError disabled (default) + Host host = tomcat.getHost(); + ErrorReportValve errorReportValve = new ErrorReportValve(); + errorReportValve.setLogOnError(false); + host.getPipeline().addValve(errorReportValve); + + // No file system docBase required + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "errorServlet", new ErrorServlet()); + ctx.addServletMappingDecoded("/", "errorServlet"); + + tomcat.start(); + + // Setup log filter to capture error logs + TesterLogValidationFilter filter = TesterLogValidationFilter.add( + Level.SEVERE, + "Error page generated for request", + null, + host.getLogName()); + + ByteChunk res = new ByteChunk(); + res.setCharset(StandardCharsets.UTF_8); + getUrl("http://localhost:" + getPort(), res, null); + + // Verify error page was generated + Assert.assertTrue(res.toString().contains( + "

" + sm.getString("errorReportValve.message") + " " + ErrorServlet.ERROR_TEXT + "

")); + + // Verify error was NOT logged + Assert.assertEquals("Error should NOT be logged when logOnError is disabled", + 0, filter.getMessageCount()); + } } diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml index 07947471f333..ae26a582bb30 100644 --- a/webapps/docs/config/valve.xml +++ b/webapps/docs/config/valve.xml @@ -2258,6 +2258,15 @@

+ +

Flag to determine if errors are logged when an error page is + generated. If set to true, then error information + (method, URI, query string, headers, and exception if present) + will be logged at error level when an error page is generated. + Default value: false +

+
+