Skip to content

Commit

Permalink
Fix BZ 69444 - set jakarta.servlet.error.message for error pages
Browse files Browse the repository at this point in the history 
Ensure the attribute is always set even if empty
https://bz.apache.org/bugzilla/show_bug.cgi?id=69444
  • Loading branch information
@markt-asf
markt-asf committed Nov 18, 2024
1 parent b72d7da commit b9a2c8a
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 36 deletions.
78 changes: 47 additions & 31 deletions java/org/apache/catalina/core/StandardHostValve.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,24 +203,8 @@ private void status(Request request, Response response) {
}
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_METHOD, request.getMethod());
request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI());
request.setAttribute(RequestDispatcher.ERROR_QUERY_STRING, request.getQueryString());
if (custom(request, response, errorPage)) {
response.setErrorReported();
try {
Expand Down Expand Up @@ -274,20 +258,9 @@ protected void throwable(Request request, Response response, Throwable throwable
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_METHOD, request.getMethod());
request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI());
request.setAttribute(RequestDispatcher.ERROR_QUERY_STRING, request.getQueryString());
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();
Expand All @@ -313,6 +286,49 @@ protected void throwable(Request request, Response response, Throwable throwable
}


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_METHOD, request.getMethod());
request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI());
request.setAttribute(RequestDispatcher.ERROR_QUERY_STRING, request.getQueryString());

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
Expand Down
55 changes: 50 additions & 5 deletions test/org/apache/catalina/core/TestStandardHostValve.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -40,6 +41,9 @@

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");
Expand All @@ -64,6 +68,12 @@ public void testErrorPageHandlingDefault() throws Exception {
}


@Test
public void testErrorPageHandlingException() throws Exception {
doTestErrorPageHandling(0, IOException.class.getCanonicalName(), "/IOE");
}


private void doTestErrorPageHandling(int error, String exception, String report)
throws Exception {

Expand Down Expand Up @@ -93,6 +103,12 @@ private void doTestErrorPageHandling(int error, String exception, String report)
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");
Expand All @@ -105,8 +121,18 @@ private void doTestErrorPageHandling(int error, String exception, String report)
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]);
}


Expand Down Expand Up @@ -215,7 +241,9 @@ private static class ErrorServlet extends HttpServlet {
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");
Expand All @@ -226,7 +254,15 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
// 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;
}
}
}
}
}
Expand Down Expand Up @@ -254,7 +290,16 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
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);
}
}
}
5 changes: 5 additions & 0 deletions webapps/docs/changelog.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down

0 comments on commit b9a2c8a

Please sign in to comment.