From 2587fe8581cfd7ca78592a805fe266a1f3d6b17b Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 8 Jul 2024 07:30:33 +0200 Subject: [PATCH] Jetty 12.1.x tck error message (#12011) * Fix for bad error message in tck test --- .../jetty/ee11/servlet/ErrorHandler.java | 30 +++++++++++--- .../jetty/ee11/servlet/ResponseTest.java | 39 +++++++++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java index 1fc353f87db..41d522bfcc6 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java @@ -73,6 +73,25 @@ public class ErrorHandler implements Request.Handler }; } + /** + * 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 { @@ -126,7 +145,8 @@ public class ErrorHandler implements Request.Handler } } - 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); @@ -416,7 +436,7 @@ public class ErrorHandler implements Request.Handler { 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); @@ -451,7 +471,7 @@ public class ErrorHandler implements Request.Handler { 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); @@ -465,7 +485,7 @@ public class ErrorHandler implements Request.Handler 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 json = new HashMap<>(); @@ -490,7 +510,7 @@ public class ErrorHandler implements Request.Handler 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("

Caused by:

");
diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResponseTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResponseTest.java
index 5a17565fa14..4823fe15ae5 100644
--- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResponseTest.java
+++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResponseTest.java
@@ -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;
@@ -22,6 +23,7 @@ import java.util.function.BiConsumer;
 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;
@@ -105,6 +107,43 @@ public class ResponseTest
         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 redirects()
     {
         List cases = new ArrayList<>();