This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new eca49efd30 Fix BZ 69444 - set jakarta.servlet.error.message for error pages eca49efd30 is described below commit eca49efd30847beee6a006ecca8e44e6206fbfd1 Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Nov 18 15:49:49 2024 +0000 Fix BZ 69444 - set jakarta.servlet.error.message for error pages Ensure the attribute is always set even if empty https://bz.apache.org/bugzilla/show_bug.cgi?id=69444 --- .../apache/catalina/core/StandardHostValve.java | 72 ++++++++++++++-------- .../catalina/core/TestStandardHostValve.java | 55 +++++++++++++++-- webapps/docs/changelog.xml | 5 ++ 3 files changed, 100 insertions(+), 32 deletions(-) diff --git a/java/org/apache/catalina/core/StandardHostValve.java b/java/org/apache/catalina/core/StandardHostValve.java index 158e9af7e3..a29ebd6621 100644 --- a/java/org/apache/catalina/core/StandardHostValve.java +++ b/java/org/apache/catalina/core/StandardHostValve.java @@ -203,22 +203,8 @@ final class StandardHostValve extends ValveBase { } if (errorPage != null && response.isErrorReportRequired()) { response.setAppCommitted(false); - request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, Integer.valueOf(statusCode)); + setRequestErrorAttributes(request, statusCode, null, response.getMessage(), null, errorPage.getLocation()); - String message = response.getMessage(); - if (message == null) { - message = ""; - } - request.setAttribute(RequestDispatcher.ERROR_MESSAGE, message); - request.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR, errorPage.getLocation()); - request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, DispatcherType.ERROR); - - - Wrapper wrapper = request.getWrapper(); - if (wrapper != null) { - request.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME, wrapper.getName()); - } - request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI()); if (custom(request, response, errorPage)) { response.setErrorReported(); try { @@ -273,18 +259,9 @@ final class StandardHostValve extends ValveBase { if (errorPage != null) { if (response.setErrorReported()) { response.setAppCommitted(false); - request.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR, errorPage.getLocation()); - request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, DispatcherType.ERROR); - request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, - Integer.valueOf(HttpServletResponse.SC_INTERNAL_SERVER_ERROR)); - request.setAttribute(RequestDispatcher.ERROR_MESSAGE, throwable.getMessage()); - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, realError); - Wrapper wrapper = request.getWrapper(); - if (wrapper != null) { - request.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME, wrapper.getName()); - } - request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI()); - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE, realError.getClass()); + setRequestErrorAttributes(request, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, realError.getClass(), + throwable.getMessage(), realError, errorPage.getLocation()); + if (custom(request, response, errorPage)) { try { response.finishResponse(); @@ -310,6 +287,47 @@ final class StandardHostValve extends ValveBase { } + private void setRequestErrorAttributes(Request request, int statusCode, Class<?> exceptionType, String message, + Throwable exception, String location) { + /* + * Generally, don't set attributes to null as that will trigger an unnecessary (in this case) call to + * removeAttribute(). + */ + + request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, Integer.valueOf(statusCode)); + + if (exceptionType != null) { + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE, exceptionType); + } + + /* + * https://bz.apache.org/bugzilla/show_bug.cgi?id=69444 + * + * Need to ensure message attribute is set even if there is no message (e.g. if error was triggered by an + * exception with a null message). + */ + if (message == null) { + request.setAttribute(RequestDispatcher.ERROR_MESSAGE, ""); + } else { + request.setAttribute(RequestDispatcher.ERROR_MESSAGE, message); + } + + if (exception != null) { + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, exception); + } + + request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI()); + + Wrapper wrapper = request.getWrapper(); + if (wrapper != null) { + request.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME, wrapper.getName()); + } + + request.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR, location); + request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, DispatcherType.ERROR); + } + + /** * Handle an HTTP status code or Java exception by forwarding control to the location included in the specified * errorPage object. It is assumed that the caller has already recorded any request attributes that are to be diff --git a/test/org/apache/catalina/core/TestStandardHostValve.java b/test/org/apache/catalina/core/TestStandardHostValve.java index 2f0799c7a8..ae7d2e526a 100644 --- a/test/org/apache/catalina/core/TestStandardHostValve.java +++ b/test/org/apache/catalina/core/TestStandardHostValve.java @@ -17,6 +17,7 @@ package org.apache.catalina.core; import java.io.IOException; +import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; @@ -40,6 +41,9 @@ import org.apache.tomcat.util.descriptor.web.ErrorPage; public class TestStandardHostValve extends TomcatBaseTest { + private static final String FAIL_NO_MESSAGE = "FAIL - NO MESSAGE"; + private static final String EMPTY_NO_MESSAGE = "EMPTY - NO MESSAGE"; + @Test public void testErrorPageHandling400() throws Exception { doTestErrorPageHandling(400, "", "/400"); @@ -64,6 +68,12 @@ public class TestStandardHostValve extends TomcatBaseTest { } + @Test + public void testErrorPageHandlingException() throws Exception { + doTestErrorPageHandling(0, IOException.class.getCanonicalName(), "/IOE"); + } + + private void doTestErrorPageHandling(int error, String exception, String report) throws Exception { @@ -93,6 +103,12 @@ public class TestStandardHostValve extends TomcatBaseTest { errorPage500.setLocation("/report/500"); ctx.addErrorPage(errorPage500); + // And the handling for IOEs + ErrorPage errorPageIOE = new ErrorPage(); + errorPageIOE.setExceptionType(IOException.class.getCanonicalName()); + errorPageIOE.setLocation("/report/IOE"); + ctx.addErrorPage(errorPageIOE); + // And the default error handling ErrorPage errorPageDefault = new ErrorPage(); errorPageDefault.setLocation("/report/default"); @@ -105,8 +121,18 @@ public class TestStandardHostValve extends TomcatBaseTest { int rc = getUrl("http://localhost:" + getPort() + "/error?errorCode=" + error + "&exception=" + exception, bc, null); - Assert.assertEquals(error, rc); - Assert.assertEquals(report, bc.toString()); + if (error > 399) { + // Specific status code expected + Assert.assertEquals(error, rc); + } else { + // Default error status code expected + Assert.assertEquals(500, rc); + } + String[] responseLines = (bc.toString().split("\n")); + // First line should be the path + Assert.assertEquals(report, responseLines[0]); + // Second line should not be the null message warning + Assert.assertNotEquals(FAIL_NO_MESSAGE, responseLines[1]); } @@ -215,7 +241,9 @@ public class TestStandardHostValve extends TomcatBaseTest { protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { int error = Integer.parseInt(req.getParameter("errorCode")); - resp.sendError(error); + if (error > 399) { + resp.sendError(error); + } Throwable t = null; String exception = req.getParameter("exception"); @@ -226,7 +254,15 @@ public class TestStandardHostValve extends TomcatBaseTest { // Should never happen but in case it does... t = new IllegalArgumentException(); } - req.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t); + if (error < 400) { + if (t instanceof RuntimeException) { + throw (RuntimeException) t; + } else if (t instanceof IOException) { + throw (IOException) t; + } else if (t instanceof ServletException) { + throw (ServletException) t; + } + } } } } @@ -254,7 +290,16 @@ public class TestStandardHostValve extends TomcatBaseTest { throws ServletException, IOException { String pathInfo = req.getPathInfo(); resp.setContentType("text/plain"); - resp.getWriter().print(pathInfo); + PrintWriter pw = resp.getWriter(); + pw.println(pathInfo); + + String message = (String) req.getAttribute(RequestDispatcher.ERROR_MESSAGE); + if (message == null) { + message = FAIL_NO_MESSAGE; + } else if (message.length() == 0) { + message = EMPTY_NO_MESSAGE; + } + pw.println(message); } } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 7a250c2811..546a59548b 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -136,6 +136,11 @@ <pr>780</pr>: Fix <code>content-range</code> header length. Submitted by Justin Chen. (remm) </fix> + <fix> + <bug>69444</bug>: Ensure that the + <code>jakarta.servlet.error.message</code> request attribute is set when + an application defined error page is called. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org