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

Reply via email to