Skip to content

Commit

Permalink
Jetty 12.1.x tck error message (#12011)
Browse files Browse the repository at this point in the history
* Fix for bad error message in tck test
  • Loading branch information
janbartel authored Jul 8, 2024
1 parent 497b6dc commit 2587fe8
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,25 @@ public boolean errorPageForMethod(String method)
};
}

/**
* Look up an attribute in the request that contains an exception message
* to use during an error dispatch. We first try the name of the attribute
* as defined by the servlet spec, falling back to checking a similar jetty
* core attribute name, if there is one.
*
* @param request the request from which to obtain the error attribute
* @param servletAttributeName the name of the attribute according to the servlet api
* @param jettyAttributeName the name of the attribute according to core jetty
* @return the exception message to use during the error dispatch or null
*/
private static Object getRequestErrorAttribute(HttpServletRequest request, String servletAttributeName, String jettyAttributeName)
{
Object o = request.getAttribute(servletAttributeName);
if (o == null)
o = request.getAttribute(jettyAttributeName);
return o;
}

@Override
public boolean handle(Request request, Response response, Callback callback) throws Exception
{
Expand Down Expand Up @@ -126,7 +145,8 @@ public boolean handle(Request request, Response response, Callback callback) thr
}
}

String message = (String)request.getAttribute(Dispatcher.ERROR_MESSAGE);
//TODO we should refactor the servlet ErrorHandler to extend and use most of the core ErrorHandler to use the core error attributes
String message = (String)getRequestErrorAttribute(httpServletRequest, Dispatcher.ERROR_MESSAGE, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_MESSAGE);
if (message == null)
message = HttpStatus.getMessage(response.getStatus());
generateAcceptableResponse(servletContextRequest, httpServletRequest, httpServletResponse, response.getStatus(), message);
Expand Down Expand Up @@ -416,7 +436,7 @@ protected void writeErrorPageMessage(HttpServletRequest request, Writer writer,
{
htmlRow(writer, "SERVLET", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
}
Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
Throwable cause = (Throwable)getRequestErrorAttribute(request, Dispatcher.ERROR_EXCEPTION, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
while (cause != null)
{
htmlRow(writer, "CAUSED BY", cause);
Expand Down Expand Up @@ -451,7 +471,7 @@ protected void writeErrorPlain(HttpServletRequest request, PrintWriter writer, i
{
writer.printf("SERVLET: %s%n", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
}
Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
Throwable cause = (Throwable)getRequestErrorAttribute(request, Dispatcher.ERROR_EXCEPTION, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
while (cause != null)
{
writer.printf("CAUSED BY %s%n", cause);
Expand All @@ -465,7 +485,7 @@ protected void writeErrorPlain(HttpServletRequest request, PrintWriter writer, i

protected void writeErrorJson(HttpServletRequest request, PrintWriter writer, int code, String message)
{
Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
Throwable cause = (Throwable)getRequestErrorAttribute(request, Dispatcher.ERROR_EXCEPTION, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
Object servlet = request.getAttribute(Dispatcher.ERROR_SERVLET_NAME);
Map<String, String> json = new HashMap<>();

Expand All @@ -490,7 +510,7 @@ protected void writeErrorJson(HttpServletRequest request, PrintWriter writer, in

protected void writeErrorPageStacks(HttpServletRequest request, Writer writer) throws IOException
{
Throwable th = (Throwable)request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
Throwable th = (Throwable)getRequestErrorAttribute(request, RequestDispatcher.ERROR_EXCEPTION, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
if (th != null)
{
writer.write("<h3>Caused by:</h3><pre>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package org.eclipse.jetty.ee11.servlet;

import java.io.IOException;
import java.io.PrintWriter;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand All @@ -22,6 +23,7 @@
import java.util.stream.Stream;

import jakarta.servlet.ServletException;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
Expand Down Expand Up @@ -105,6 +107,43 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
assertThat(response.getContent(), containsString("Hello"));
}

@Test
public void testErrorWithMessage() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setErrorHandler(new ErrorPageErrorHandler());
contextHandler.setContextPath("/");
HttpServlet servlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
PrintWriter pw = response.getWriter();
pw.println("THIS TEXT SHOULD NOT APPEAR");
response.addHeader("header", "sendError_StringTest");
response.addCookie(new Cookie("cookie1", "value1"));
response.sendError(HttpServletResponse.SC_GONE, "The content is gone.");
}
};

contextHandler.addServlet(servlet, "/error/*");
startServer(contextHandler);

HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/error/");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");

ByteBuffer responseBuffer = _connector.getResponse(request.generate());
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);

assertThat(response.getStatus(), is(410));
assertThat(response.get("Content-Type"), is("text/html;charset=ISO-8859-1"));
assertThat(response.getContent(), containsString("The content is gone."));
}

public static Stream<Arguments> redirects()
{
List<Arguments> cases = new ArrayList<>();
Expand Down

0 comments on commit 2587fe8

Please sign in to comment.