This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new c302462764 Fix BZ 69444 - set javax.servlet.error.message for error
pages
c302462764 is described below
commit c302462764de0d35a7f7b3c10d1f71514d5c8e84
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Nov 18 15:49:49 2024 +0000
Fix BZ 69444 - set javax.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 af4f9b0aa0..14505f6c29 100644
--- a/java/org/apache/catalina/core/StandardHostValve.java
+++ b/java/org/apache/catalina/core/StandardHostValve.java
@@ -218,22 +218,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 {
@@ -288,18 +274,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();
@@ -325,6 +302,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 e081a12173..cd8d99ce5a 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 7f2fa71437..e997d7c0f1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,11 @@
The previous fix for incosistent resource metadata during concurrent
reads and writes was incomplete. (markt)
</fix>
+ <fix>
+ <bug>69444</bug>: Ensure that the
+ <code>javax.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: [email protected]
For additional commands, e-mail: [email protected]
