From a80c07f52be9cdee3e4a3a089889fe2c1a09d62a Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 25 Oct 2024 08:54:55 +1100 Subject: [PATCH] Jetty 12.1.x refactor servlet error handler (#11982) Issue #11982 refactor common code for ee11 ErrorHandler --- .../jetty/server/handler/ErrorHandler.java | 68 ++- .../server/handler/ContextHandlerTest.java | 4 +- .../server/handler/DebugHandlerTest.java | 43 +- .../jetty/ee11/servlet/Dispatcher.java | 7 + .../jetty/ee11/servlet/ErrorHandler.java | 514 +----------------- .../ee11/servlet/ErrorPageErrorHandler.java | 4 +- .../jetty/ee11/servlet/ResourceServlet.java | 2 +- .../ee11/servlet/ServletChannelState.java | 13 +- .../jetty/ee11/servlet/AsyncListenerTest.java | 9 +- .../jetty/ee9/servlet/AsyncListenerTest.java | 8 +- 10 files changed, 128 insertions(+), 544 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index 8070e51e59e6..f6a59f7f1200 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -17,6 +17,7 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.StringWriter; +import java.io.UncheckedIOException; import java.io.Writer; import java.nio.BufferOverflowException; import java.nio.ByteBuffer; @@ -29,6 +30,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.stream.Collectors; import org.eclipse.jetty.http.HttpException; @@ -69,6 +71,7 @@ public class ErrorHandler implements Request.Handler public static final String ERROR_MESSAGE = "org.eclipse.jetty.server.error_message"; public static final String ERROR_EXCEPTION = "org.eclipse.jetty.server.error_exception"; public static final String ERROR_CONTEXT = "org.eclipse.jetty.server.error_context"; + public static final String ERROR_ORIGIN = "org.eclipse.jetty.server.error_origin"; public static final Set ERROR_METHODS = Set.of("GET", "POST", "HEAD", "BAD"); public static final HttpField ERROR_CACHE_CONTROL = new PreEncodedHttpField(HttpHeader.CACHE_CONTROL, "must-revalidate,no-cache,no-store"); @@ -76,6 +79,7 @@ public class ErrorHandler implements Request.Handler boolean _showCauses = false; boolean _showMessageInTitle = true; int _bufferSize = -1; + boolean _showOrigin = false; String _defaultResponseMimeType = Type.TEXT_HTML.asString(); HttpField _cacheControl = new PreEncodedHttpField(HttpHeader.CACHE_CONTROL, "must-revalidate,no-cache,no-store"); @@ -93,8 +97,8 @@ public boolean handle(Request request, Response response, Callback callback) thr { if (LOG.isDebugEnabled()) LOG.debug("handle({}, {}, {})", request, response, callback); - if (_cacheControl != null) - response.getHeaders().put(_cacheControl); + + generateCacheControl(response); int code = response.getStatus(); String message = (String)request.getAttribute(ERROR_MESSAGE); @@ -120,6 +124,12 @@ public boolean handle(Request request, Response response, Callback callback) thr return true; } + protected void generateCacheControl(Response response) + { + if (_cacheControl != null && response != null) + response.getHeaders().put(_cacheControl); + } + protected void generateResponse(Request request, Response response, int code, String message, Throwable cause, Callback callback) throws IOException { List acceptable = request.getHeaders().getQualityCSV(HttpHeader.ACCEPT, QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING); @@ -218,9 +228,9 @@ else if (charsets.contains(StandardCharsets.ISO_8859_1)) switch (type) { - case TEXT_HTML -> writeErrorHtml(request, writer, charset, code, message, cause, showStacks); - case TEXT_JSON, APPLICATION_JSON -> writeErrorJson(request, writer, code, message, cause, showStacks); - case TEXT_PLAIN -> writeErrorPlain(request, writer, code, message, cause, showStacks); + case TEXT_HTML -> writeErrorHtml(request, writer, charset, code, message, cause); + case TEXT_JSON, APPLICATION_JSON -> writeErrorJson(request, writer, code, message, cause); + case TEXT_PLAIN -> writeErrorPlain(request, writer, code, message, cause); default -> throw new IllegalStateException(); } @@ -273,7 +283,7 @@ protected int computeBufferSize(Request request) return bufferSize; } - protected void writeErrorHtml(Request request, Writer writer, Charset charset, int code, String message, Throwable cause, boolean showStacks) throws IOException + protected void writeErrorHtml(Request request, Writer writer, Charset charset, int code, String message, Throwable cause) throws IOException { if (message == null) message = HttpStatus.getMessage(code); @@ -282,7 +292,7 @@ protected void writeErrorHtml(Request request, Writer writer, Charset charset, i writeErrorHtmlMeta(request, writer, charset); writeErrorHtmlHead(request, writer, code, message); writer.write("\n\n"); - writeErrorHtmlBody(request, writer, code, message, cause, showStacks); + writeErrorHtmlBody(request, writer, code, message, cause); writer.write("\n\n\n"); } @@ -306,12 +316,12 @@ protected void writeErrorHtmlHead(Request request, Writer writer, int code, Stri writer.write("\n"); } - protected void writeErrorHtmlBody(Request request, Writer writer, int code, String message, Throwable cause, boolean showStacks) throws IOException + protected void writeErrorHtmlBody(Request request, Writer writer, int code, String message, Throwable cause) throws IOException { String uri = request.getHttpURI().toString(); writeErrorHtmlMessage(request, writer, code, message, cause, uri); - if (showStacks) + if (isShowStacks()) writeErrorHtmlStacks(request, writer); request.getConnectionMetaData().getHttpConfiguration() @@ -333,6 +343,18 @@ protected void writeErrorHtmlMessage(Request request, Writer writer, int code, S htmlRow(writer, "URI", uri); htmlRow(writer, "STATUS", status); htmlRow(writer, "MESSAGE", message); + writeErrorOrigin((String)request.getAttribute(ERROR_ORIGIN), (o) -> + { + try + { + htmlRow(writer, "ORIGIN", o); + } + catch (IOException x) + { + throw new UncheckedIOException(x); + } + }); + while (_showCauses && cause != null) { htmlRow(writer, "CAUSED BY", cause); @@ -341,7 +363,7 @@ protected void writeErrorHtmlMessage(Request request, Writer writer, int code, S writer.write("\n"); } - private void htmlRow(Writer writer, String tag, Object value) throws IOException + protected void htmlRow(Writer writer, String tag, Object value) throws IOException { writer.write(""); writer.write(tag); @@ -353,7 +375,7 @@ private void htmlRow(Writer writer, String tag, Object value) throws IOException writer.write("\n"); } - protected void writeErrorPlain(Request request, PrintWriter writer, int code, String message, Throwable cause, boolean showStacks) + protected void writeErrorPlain(Request request, PrintWriter writer, int code, String message, Throwable cause) { writer.write("HTTP ERROR "); writer.write(Integer.toString(code)); @@ -366,22 +388,32 @@ protected void writeErrorPlain(Request request, PrintWriter writer, int code, St writer.printf("URI: %s%n", request.getHttpURI()); writer.printf("STATUS: %s%n", code); writer.printf("MESSAGE: %s%n", message); + + writeErrorOrigin((String)request.getAttribute(ERROR_ORIGIN), (o) -> writer.printf("ORIGIN: %s%n", o)); + while (_showCauses && cause != null) { writer.printf("CAUSED BY %s%n", cause); - if (showStacks) + if (isShowStacks()) cause.printStackTrace(writer); cause = cause.getCause(); } } - protected void writeErrorJson(Request request, PrintWriter writer, int code, String message, Throwable cause, boolean showStacks) + protected void writeErrorOrigin(String origin, Consumer consumer) + { + if (_showOrigin && origin != null) + consumer.accept(origin); + } + + protected void writeErrorJson(Request request, PrintWriter writer, int code, String message, Throwable cause) { Map json = new HashMap<>(); json.put("url", request.getHttpURI().toString()); json.put("status", Integer.toString(code)); json.put("message", message); + writeErrorOrigin((String)request.getAttribute(ERROR_ORIGIN), (o) -> json.put("origin", o)); int c = 0; while (_showCauses && cause != null) { @@ -502,6 +534,16 @@ public void setShowMessageInTitle(boolean showMessageInTitle) _showMessageInTitle = showMessageInTitle; } + public boolean isShowOrigin() + { + return _showOrigin; + } + + public void setShowOrigin(boolean showOrigin) + { + _showOrigin = showOrigin; + } + /** * @return The mime type to be used when a client does not specify an Accept header, or the request did not fully parse */ diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index 3b82e1d8817d..00763fa7cf4d 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -602,12 +602,12 @@ public boolean handle(Request request, Response response, Callback callback) _contextHandler.setErrorHandler(new ErrorHandler() { @Override - protected void writeErrorHtmlBody(Request request, Writer writer, int code, String message, Throwable cause, boolean showStacks) throws IOException + protected void writeErrorHtmlBody(Request request, Writer writer, int code, String message, Throwable cause) throws IOException { Context context = request.getContext(); if (context != null) writer.write("

Context: " + context.getContextPath() + "

"); - super.writeErrorHtmlBody(request, writer, code, message, cause, showStacks); + super.writeErrorHtmlBody(request, writer, code, message, cause); } }); _server.start(); diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java index cacb7ac44f33..bf6cfe742a31 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java @@ -30,26 +30,29 @@ import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.io.IOResources; import org.eclipse.jetty.server.AbstractConnectionFactory; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -@Disabled // TODO public class DebugHandlerTest { + public static final String INBOUND_STARTS_WITH = ">> r="; + public static final String OUTBOUND_STARTS_WITH = "<< r="; public static final HostnameVerifier __hostnameverifier = (hostname, session) -> true; private SSLContext sslContext; @@ -85,19 +88,18 @@ public void startServer() throws Exception debugHandler = new DebugHandler(); capturedLog = new ByteArrayOutputStream(); debugHandler.setOutputStream(capturedLog); - /* TODO - debugHandler.setHandler(new AbstractHandler() + server.setHandler(new Handler.Abstract() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + public boolean handle(Request request, Response response, Callback callback) throws Exception { - baseRequest.setHandled(true); - response.setStatus(HttpStatus.OK_200); + response.setStatus(200); + callback.succeeded(); + return true; } }); - server.setHandler(debugHandler); + server.insertHandler(debugHandler); - */ server.start(); String host = httpConnector.getHost(); @@ -146,30 +148,27 @@ public void stopServer() throws Exception } @Test - public void testThreadName() throws IOException + public void testDebugInboundOutboundMessages() throws IOException { HttpURLConnection http = (HttpURLConnection)serverURI.resolve("/foo/bar?a=b").toURL().openConnection(); assertThat("Response Code", http.getResponseCode(), is(200)); String log = capturedLog.toString(StandardCharsets.UTF_8.name()); - String expectedThreadName = ":/foo/bar?a=b"; - assertThat("ThreadName", log, containsString(expectedThreadName)); - // Look for bad/mangled/duplicated schemes - assertThat("ThreadName", log, not(containsString("http:" + expectedThreadName))); - assertThat("ThreadName", log, not(containsString("https:" + expectedThreadName))); + + assertThat("Inbound", log, containsString(INBOUND_STARTS_WITH)); + assertThat("Outbound", log, containsString(OUTBOUND_STARTS_WITH)); } @Test - public void testSecureThreadName() throws IOException + public void testSecureInboundOutboundMessages() throws IOException { HttpURLConnection http = (HttpURLConnection)secureServerURI.resolve("/foo/bar?a=b").toURL().openConnection(); assertThat("Response Code", http.getResponseCode(), is(200)); String log = capturedLog.toString(StandardCharsets.UTF_8.name()); - String expectedThreadName = ":/foo/bar?a=b"; - assertThat("ThreadName", log, containsString(expectedThreadName)); - // Look for bad/mangled/duplicated schemes - assertThat("ThreadName", log, not(containsString("http:" + expectedThreadName))); - assertThat("ThreadName", log, not(containsString("https:" + expectedThreadName))); + System.err.println("****" + log + "******"); + + assertThat("Inbound", log, containsString(INBOUND_STARTS_WITH)); + assertThat("Outbound", log, containsString(OUTBOUND_STARTS_WITH)); } } diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java index de76b26a37f7..eafab6a086ec 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java @@ -837,6 +837,13 @@ public Object getAttribute(String name) case ERROR_QUERY_STRING -> _httpServletRequest.getQueryString(); case ERROR_STATUS_CODE -> super.getAttribute(ErrorHandler.ERROR_STATUS); case ERROR_MESSAGE -> super.getAttribute(ErrorHandler.ERROR_MESSAGE); + case ERROR_SERVLET_NAME -> super.getAttribute(ErrorHandler.ERROR_ORIGIN); + case ERROR_EXCEPTION -> super.getAttribute(ErrorHandler.ERROR_EXCEPTION); + case ERROR_EXCEPTION_TYPE -> + { + Object err = super.getAttribute(ErrorHandler.ERROR_EXCEPTION); + yield err == null ? null : err.getClass(); + } default -> super.getAttribute(name); }; } 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 41d522bfcc60..adf4923fecb5 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 @@ -16,23 +16,17 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; -import java.io.StringWriter; +import java.io.UncheckedIOException; import java.io.Writer; import java.nio.BufferOverflowException; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; -import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MimeTypes; @@ -42,54 +36,21 @@ import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ErrorHandler implements Request.Handler +public class ErrorHandler extends org.eclipse.jetty.server.handler.ErrorHandler { - // TODO This class's API needs to be majorly refactored/cleanup private static final Logger LOG = LoggerFactory.getLogger(ErrorHandler.class); public static final String ERROR_CHARSET = "org.eclipse.jetty.server.error_charset"; - boolean _showServlet = true; - boolean _showStacks = true; - boolean _disableStacks = false; - boolean _showMessageInTitle = true; - String _cacheControl = "must-revalidate,no-cache,no-store"; - public ErrorHandler() { - } - - public boolean errorPageForMethod(String method) - { - return switch (method) - { - case "GET", "POST", "HEAD" -> true; - default -> false; - }; - } - - /** - * 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; + setShowOrigin(true); + setShowStacks(true); + setShowMessageInTitle(true); } @Override @@ -101,6 +62,8 @@ public boolean handle(Request request, Response response, Callback callback) thr return true; } + generateCacheControl(response); + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); HttpServletRequest httpServletRequest = servletContextRequest.getServletApiRequest(); HttpServletResponse httpServletResponse = servletContextRequest.getHttpServletResponse(); @@ -145,279 +108,27 @@ public boolean handle(Request request, Response response, Callback callback) thr } } - //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); + String message = (String)request.getAttribute(ERROR_MESSAGE); if (message == null) message = HttpStatus.getMessage(response.getStatus()); - generateAcceptableResponse(servletContextRequest, httpServletRequest, httpServletResponse, response.getStatus(), message); + generateResponse(request, response, response.getStatus(), message, (Throwable)request.getAttribute(ERROR_EXCEPTION), callback); callback.succeeded(); return true; } - /** - * Generate an acceptable error response. - *

This method is called to generate an Error page of a mime type that is - * acceptable to the user-agent. The Accept header is evaluated in - * quality order and the method - * {@link #generateAcceptableResponse(ServletContextRequest, HttpServletRequest, HttpServletResponse, int, String, String)} - * is called for each mimetype until the response is written to or committed.

- * - * @param baseRequest The base request - * @param request The servlet request (may be wrapped) - * @param response The response (may be wrapped) - * @param code the http error code - * @param message the http error message - * @throws IOException if the response cannot be generated - */ - protected void generateAcceptableResponse(ServletContextRequest baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message) throws IOException - { - List acceptable = baseRequest.getHeaders().getQualityCSV(HttpHeader.ACCEPT, QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING); - - if (acceptable.isEmpty() && !baseRequest.getHeaders().contains(HttpHeader.ACCEPT)) - { - generateAcceptableResponse(baseRequest, request, response, code, message, MimeTypes.Type.TEXT_HTML.asString()); - } - else - { - for (String mimeType : acceptable) - { - generateAcceptableResponse(baseRequest, request, response, code, message, mimeType); - if (response.isCommitted() || baseRequest.getServletContextResponse().isWritingOrStreaming()) - break; - } - } - } - - /** - * Returns an acceptable writer for an error page. - *

Uses the user-agent's Accept-Charset to get response - * {@link Writer}. The acceptable charsets are tested in quality order - * if they are known to the JVM and the first known is set on - * {@link HttpServletResponse#setCharacterEncoding(String)} and the - * {@link HttpServletResponse#getWriter()} method used to return a writer. - * If there is no Accept-Charset header then - * ISO-8859-1 is used. If '*' is the highest quality known - * charset, then utf-8 is used. - *

- * - * @param baseRequest The base request - * @param request The servlet request (may be wrapped) - * @param response The response (may be wrapped) - * @return A {@link Writer} if there is a known acceptable charset or null - * @throws IOException if a Writer cannot be returned - */ - @Deprecated - protected Writer getAcceptableWriter(Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + protected boolean generateAcceptableResponse(Request request, Response response, Callback callback, String contentType, List charsets, int code, String message, Throwable cause) throws IOException { - List acceptable = baseRequest.getHeaders().getQualityCSV(HttpHeader.ACCEPT_CHARSET); - if (acceptable.isEmpty()) - { - response.setCharacterEncoding(StandardCharsets.ISO_8859_1.name()); - return response.getWriter(); - } - - for (String charset : acceptable) + boolean result = super.generateAcceptableResponse(request, response, callback, contentType, charsets, code, message, cause); + if (result) { - try - { - if ("*".equals(charset)) - response.setCharacterEncoding(StandardCharsets.UTF_8.name()); - else - response.setCharacterEncoding(Charset.forName(charset).name()); - return response.getWriter(); - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } + // Do an asynchronous completion + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + servletContextRequest.getServletChannel().sendErrorResponseAndComplete(); } - return null; + return result; } - /** - * Generate an acceptable error response for a mime type. - *

This method is called for each mime type in the users agent's - * Accept header, a response of the appropriate type is generated. - *

- *

The default implementation handles "text/html", "text/*" and "*/*". - * The method can be overridden to handle other types. Implementations must - * immediate produce a response and may not be async. - *

- * - * @param baseRequest The base request - * @param request The servlet request (may be wrapped) - * @param response The response (may be wrapped) - * @param code the http error code - * @param message the http error message - * @param contentType The mimetype to generate (may be */*or other wildcard) - * @throws IOException if a response cannot be generated - */ - protected void generateAcceptableResponse(ServletContextRequest baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message, String contentType) throws IOException - { - // We can generate an acceptable contentType, but can we generate an acceptable charset? - // TODO refactor this in jetty-10 to be done in the other calling loop - Charset charset = null; - List acceptable = baseRequest.getHeaders().getQualityCSV(HttpHeader.ACCEPT_CHARSET); - if (!acceptable.isEmpty()) - { - for (String name : acceptable) - { - if ("*".equals(name)) - { - charset = StandardCharsets.UTF_8; - break; - } - - try - { - charset = Charset.forName(name); - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } - } - if (charset == null) - return; - } - - MimeTypes.Type type; - switch (contentType) - { - case "text/html": - case "text/*": - case "*/*": - type = MimeTypes.Type.TEXT_HTML; - if (charset == null) - charset = StandardCharsets.ISO_8859_1; - break; - - case "text/json": - case "application/json": - type = MimeTypes.Type.TEXT_JSON; - if (charset == null) - charset = StandardCharsets.UTF_8; - break; - - case "text/plain": - type = MimeTypes.Type.TEXT_PLAIN; - if (charset == null) - charset = StandardCharsets.ISO_8859_1; - break; - - default: - return; - } - - // write into the response aggregate buffer and flush it asynchronously. - while (true) - { - try - { - // TODO currently the writer used here is of fixed size, so a large - // TODO error page may cause a BufferOverflow. In which case we try - // TODO again with stacks disabled. If it still overflows, it is - // TODO written without a body. - ByteBuffer buffer = baseRequest.getServletContextResponse().getHttpOutput().getByteBuffer(); - ByteBufferOutputStream out = new ByteBufferOutputStream(buffer); - PrintWriter writer = new PrintWriter(new OutputStreamWriter(out, charset)); - - switch (type) - { - case TEXT_HTML: - response.setContentType(MimeTypes.Type.TEXT_HTML.asString()); - response.setCharacterEncoding(charset.name()); - request.setAttribute(ERROR_CHARSET, charset); - handleErrorPage(request, writer, code, message); - break; - case TEXT_JSON: - response.setContentType(contentType); - writeErrorJson(request, writer, code, message); - break; - case TEXT_PLAIN: - response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); - response.setCharacterEncoding(charset.name()); - writeErrorPlain(request, writer, code, message); - break; - default: - throw new IllegalStateException(); - } - - writer.flush(); - break; - } - catch (BufferOverflowException e) - { - if (LOG.isDebugEnabled()) - LOG.warn("Error page too large: {} {} {}", code, message, request, e); - else - LOG.warn("Error page too large: {} {} {}", code, message, request); - baseRequest.getServletContextResponse().resetContent(); - if (!_disableStacks) - { - LOG.info("Disabling showsStacks for {}", this); - _disableStacks = true; - continue; - } - break; - } - } - - // Do an asynchronous completion. - baseRequest.getServletChannel().sendErrorResponseAndComplete(); - } - - protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException - { - writeErrorPage(request, writer, code, message, _showStacks); - } - - protected void writeErrorPage(HttpServletRequest request, Writer writer, int code, String message, boolean showStacks) throws IOException - { - if (message == null) - message = HttpStatus.getMessage(code); - - writer.write("\n\n"); - writeErrorPageHead(request, writer, code, message); - writer.write("\n"); - writeErrorPageBody(request, writer, code, message, showStacks); - writer.write("\n\n\n"); - } - - protected void writeErrorPageHead(HttpServletRequest request, Writer writer, int code, String message) throws IOException - { - Charset charset = (Charset)request.getAttribute(ERROR_CHARSET); - if (charset != null) - { - writer.write("\n"); - } - writer.write("Error "); - // TODO this code is duplicated in writeErrorPageMessage - String status = Integer.toString(code); - writer.write(status); - if (message != null && !message.equals(status)) - { - writer.write(' '); - writer.write(StringUtil.sanitizeXmlString(message)); - } - writer.write("\n"); - } - - protected void writeErrorPageBody(HttpServletRequest request, Writer writer, int code, String message, boolean showStacks) throws IOException - { - String uri = request.getRequestURI(); - - writeErrorPageMessage(request, writer, code, message, uri); - if (showStacks && !_disableStacks) - writeErrorPageStacks(request, writer); - - ((ServletApiRequest)request).getServletRequestInfo().getServletChannel().getHttpConfiguration() - .writePoweredBy(writer, "
", "
\n"); - } - - protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException + protected void writeErrorHtmlMessage(Request request, Writer writer, int code, String message, Throwable cause, String uri) throws IOException { writer.write("

HTTP ERROR "); String status = Integer.toString(code); @@ -432,195 +143,24 @@ protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, htmlRow(writer, "URI", uri); htmlRow(writer, "STATUS", status); htmlRow(writer, "MESSAGE", message); - if (isShowServlet()) + writeErrorOrigin((String)request.getAttribute(ERROR_ORIGIN), (o) -> { - htmlRow(writer, "SERVLET", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME)); - } - Throwable cause = (Throwable)getRequestErrorAttribute(request, Dispatcher.ERROR_EXCEPTION, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - while (cause != null) - { - htmlRow(writer, "CAUSED BY", cause); - cause = cause.getCause(); - } - writer.write("\n"); - } - - private void htmlRow(Writer writer, String tag, Object value) throws IOException - { - writer.write(""); - writer.write(tag); - writer.write(":"); - if (value == null) - writer.write("-"); - else - writer.write(StringUtil.sanitizeXmlString(value.toString())); - writer.write("\n"); - } - - protected void writeErrorPlain(HttpServletRequest request, PrintWriter writer, int code, String message) - { - writer.write("HTTP ERROR "); - writer.write(Integer.toString(code)); - writer.write(' '); - writer.write(StringUtil.sanitizeXmlString(message)); - writer.write("\n"); - writer.printf("URI: %s%n", request.getRequestURI()); - writer.printf("STATUS: %s%n", code); - writer.printf("MESSAGE: %s%n", message); - if (isShowServlet()) - { - writer.printf("SERVLET: %s%n", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME)); - } - 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); - if (isShowStacks() && !_disableStacks) + try { - cause.printStackTrace(writer); + htmlRow(writer, "SERVLET", o); } - cause = cause.getCause(); - } - } - - protected void writeErrorJson(HttpServletRequest request, PrintWriter writer, int code, String message) - { - 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<>(); + catch (IOException x) + { + throw new UncheckedIOException(x); + } + }); - json.put("url", request.getRequestURI()); - json.put("status", Integer.toString(code)); - json.put("message", message); - if (isShowServlet() && servlet != null) - { - json.put("servlet", servlet.toString()); - } - int c = 0; while (cause != null) { - json.put("cause" + c++, cause.toString()); + htmlRow(writer, "CAUSED BY", cause); cause = cause.getCause(); } - - writer.append(json.entrySet().stream() - .map(e -> HttpField.NAME_VALUE_TOKENIZER.quote(e.getKey()) + ":" + HttpField.NAME_VALUE_TOKENIZER.quote(StringUtil.sanitizeXmlString((e.getValue())))) - .collect(Collectors.joining(",\n", "{\n", "\n}"))); - } - - protected void writeErrorPageStacks(HttpServletRequest request, Writer writer) throws IOException - { - Throwable th = (Throwable)getRequestErrorAttribute(request, RequestDispatcher.ERROR_EXCEPTION, org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); - if (th != null) - { - writer.write("

Caused by:

");
-            // You have to pre-generate and then use #write(writer, String)
-            try (StringWriter sw = new StringWriter();
-                 PrintWriter pw = new PrintWriter(sw))
-            {
-                th.printStackTrace(pw);
-                pw.flush();
-                write(writer, sw.getBuffer().toString()); // sanitize
-            }
-            writer.write("
\n"); - } - } - - /** - * Bad Message Error body - *

Generate an error response body to be sent for a bad message. - * In this case there is something wrong with the request, so either - * a request cannot be built, or it is not safe to build a request. - * This method allows for a simple error page body to be returned - * and some response headers to be set. - * - * @param status The error code that will be sent - * @param reason The reason for the error code (may be null) - * @param fields The header fields that will be sent with the response. - * @return The content as a ByteBuffer, or null for no body. - */ - public ByteBuffer badMessageError(int status, String reason, HttpFields.Mutable fields) - { - if (reason == null) - reason = HttpStatus.getMessage(status); - if (HttpStatus.hasNoBody(status)) - return BufferUtil.EMPTY_BUFFER; - fields.put(HttpHeader.CONTENT_TYPE, MimeTypes.Type.TEXT_HTML_8859_1.asString()); - return BufferUtil.toBuffer("

Bad Message " + status + "

reason: " + reason + "
"); - } - - /** - * Get the cacheControl. - * - * @return the cacheControl header to set on error responses. - */ - public String getCacheControl() - { - return _cacheControl; - } - - /** - * Set the cacheControl. - * - * @param cacheControl the cacheControl header to set on error responses. - */ - public void setCacheControl(String cacheControl) - { - _cacheControl = cacheControl; - } - - /** - * @return True if the error page will show the Servlet that generated the error - */ - public boolean isShowServlet() - { - return _showServlet; - } - - /** - * @param showServlet True if the error page will show the Servlet that generated the error - */ - public void setShowServlet(boolean showServlet) - { - _showServlet = showServlet; - } - - /** - * @return True if stack traces are shown in the error pages - */ - public boolean isShowStacks() - { - return _showStacks; - } - - /** - * @param showStacks True if stack traces are shown in the error pages - */ - public void setShowStacks(boolean showStacks) - { - _showStacks = showStacks; - } - - /** - * Set if true, the error message appears in page title. - * @param showMessageInTitle if true, the error message appears in page title - */ - public void setShowMessageInTitle(boolean showMessageInTitle) - { - _showMessageInTitle = showMessageInTitle; - } - - public boolean getShowMessageInTitle() - { - return _showMessageInTitle; - } - - protected void write(Writer writer, String string) throws IOException - { - if (string == null) - return; - - writer.write(StringUtil.sanitizeXmlString(string)); + writer.write("\n"); } public interface ErrorPageMapper diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java index 05a4b2e8e46b..51f1a6f8f2c2 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java @@ -67,7 +67,7 @@ public String getErrorPage(HttpServletRequest request) PageLookupTechnique pageSource = null; Class matchedThrowable = null; - Throwable error = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION); + Throwable error = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION); Throwable cause = error; // Walk the cause hierarchy @@ -98,7 +98,7 @@ public String getErrorPage(HttpServletRequest request) Throwable unwrapped = unwrapServletException(error, matchedThrowable); if (unwrapped != null) { - request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped); + request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION, unwrapped); request.setAttribute(Dispatcher.ERROR_EXCEPTION_TYPE, unwrapped.getClass()); } } diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java index a81b2071c054..36f75a6e5490 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java @@ -780,7 +780,7 @@ protected void writeHttpError(Request coreRequest, Response coreResponse, Callba if (isIncluded(request)) return; if (cause != null) - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, cause); + request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION, cause); response.sendError(statusCode, reason); } catch (IOException e) diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java index ac2050c8e84a..fb55b9b9be48 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletChannelState.java @@ -34,9 +34,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static jakarta.servlet.RequestDispatcher.ERROR_EXCEPTION; -import static jakarta.servlet.RequestDispatcher.ERROR_EXCEPTION_TYPE; -import static jakarta.servlet.RequestDispatcher.ERROR_SERVLET_NAME; +import static org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION; +import static org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN; /** * holder of the state of request-response cycle. @@ -1008,9 +1007,6 @@ else if (cause instanceof UnavailableException) // No ISE, so good to modify request/state request.setAttribute(ERROR_EXCEPTION, th); - request.setAttribute(ERROR_EXCEPTION_TYPE, th.getClass()); - - // Set Jetty specific attributes. request.setAttribute(ErrorHandler.ERROR_EXCEPTION, th); // Ensure any async lifecycle is ended! @@ -1055,10 +1051,7 @@ public void sendError(int code, String message) response.setStatus(code); servletContextRequest.errorClose(); - request.setAttribute(ERROR_SERVLET_NAME, servletContextRequest.getServletName()); - // Additional servlet error attributes are provided in org.eclipse.jetty.ee11.servlet.Dispatcher.ErrorRequest - - // Set Jetty Specific Attributes. + request.setAttribute(ERROR_ORIGIN, servletContextRequest.getServletName()); request.setAttribute(ErrorHandler.ERROR_CONTEXT, servletContextRequest.getServletContext()); request.setAttribute(ErrorHandler.ERROR_MESSAGE, message); request.setAttribute(ErrorHandler.ERROR_STATUS, code); diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/AsyncListenerTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/AsyncListenerTest.java index 574093c14d0b..6726483dc185 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/AsyncListenerTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/AsyncListenerTest.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.io.QuietException; import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; @@ -160,10 +161,10 @@ public void testStartAsyncThrowOnErrorSendErrorCustomErrorPage() throws Exceptio ErrorHandler errorHandler = new ErrorHandler() { @Override - protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException + protected void writeErrorHtmlMessage(Request request, Writer writer, int code, String message, Throwable cause, String uri) throws IOException { writer.write("CUSTOM\n"); - super.writeErrorPageMessage(request, writer, code, message, uri); + super.writeErrorHtmlMessage(request, writer, code, message, cause, uri); } }; server.setErrorHandler(errorHandler); @@ -314,10 +315,10 @@ public void testStartAsyncOnTimeoutSendErrorCustomErrorPage() throws Exception ErrorHandler errorHandler = new ErrorHandler() { @Override - protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException + protected void writeErrorHtmlMessage(Request request, Writer writer, int code, String message, Throwable cause, String uri) throws IOException { writer.write("CUSTOM\n"); - super.writeErrorPageMessage(request, writer, code, message, uri); + super.writeErrorHtmlMessage(request, writer, code, message, cause, uri); } }; server.setErrorHandler(errorHandler); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/AsyncListenerTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/AsyncListenerTest.java index 5edbc9771b76..448b92ab8213 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/AsyncListenerTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/AsyncListenerTest.java @@ -15,6 +15,8 @@ import java.io.IOException; import java.io.PrintWriter; +import java.io.Writer; +import java.nio.charset.Charset; import java.util.concurrent.TimeUnit; import jakarta.servlet.AsyncContext; @@ -166,10 +168,10 @@ public void testStartAsyncThrowOnErrorSendErrorCustomErrorPage() throws Exceptio ErrorHandler errorHandler = new ErrorHandler() { @Override - protected void writeErrorPlain(Request request, PrintWriter writer, int code, String message, Throwable cause, boolean showStacks) + protected void writeErrorHtml(Request request, Writer writer, Charset charset, int code, String message, Throwable cause) throws IOException { writer.write("CUSTOM\n"); - super.writeErrorPlain(request, writer, code, message, cause, showStacks); + super.writeErrorHtml(request, writer, charset, code, message, cause); } }; server.setErrorHandler(errorHandler); @@ -321,7 +323,7 @@ public void testStartAsyncOnTimeoutSendErrorCustomErrorPage() throws Exception ErrorHandler errorHandler = new ErrorHandler() { @Override - protected void writeErrorPlain(Request request, PrintWriter writer, int code, String message, Throwable cause, boolean showStacks) + protected void writeErrorPlain(Request request, PrintWriter writer, int code, String message, Throwable cause) { writer.write("CUSTOM\n"); }