Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12.1.x refactor servlet error handler #11982

Merged
merged 9 commits into from
Oct 24, 2024
Merged

Jetty 12.1.x refactor servlet error handler #11982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
76dddb8
WIP
janbartel Jun 27, 2024
dfa81a0
WIP
janbartel Jun 27, 2024
ceee0e7
WIP
janbartel Jun 28, 2024
ae8921c
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
janbartel Jun 28, 2024
73a6c26
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
janbartel Oct 10, 2024
be91eda
Fixes.
janbartel Oct 14, 2024
31786b0
Remove println.
janbartel Oct 17, 2024
38bca4c
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
janbartel Oct 22, 2024
86a9897
Merge remote-tracking branch 'origin/jetty-12.1.x' into jetty-12.1.x-…
janbartel Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 55 additions & 13 deletions jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -69,13 +71,15 @@ 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<String> 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");

boolean _showStacks = false;
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");

Expand All @@ -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);
Expand All @@ -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<String> acceptable = request.getHeaders().getQualityCSV(HttpHeader.ACCEPT, QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING);
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);
Expand All @@ -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("</head>\n<body>\n");
writeErrorHtmlBody(request, writer, code, message, cause, showStacks);
writeErrorHtmlBody(request, writer, code, message, cause);
writer.write("\n</body>\n</html>\n");
}

Expand All @@ -306,12 +316,12 @@ protected void writeErrorHtmlHead(Request request, Writer writer, int code, Stri
writer.write("</title>\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()
Expand All @@ -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);
Expand All @@ -341,7 +363,7 @@ protected void writeErrorHtmlMessage(Request request, Writer writer, int code, S
writer.write("</table>\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("<tr><th>");
writer.write(tag);
Expand All @@ -353,7 +375,7 @@ private void htmlRow(Writer writer, String tag, Object value) throws IOException
writer.write("</td></tr>\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));
Expand All @@ -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<String> consumer)
{
if (_showOrigin && origin != null)
consumer.accept(origin);
}

protected void writeErrorJson(Request request, PrintWriter writer, int code, String message, Throwable cause)
{
Map<String, String> 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)
{
Expand Down Expand Up @@ -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
*/
Expand Down
4 changes: 2 additions & 2 deletions ...-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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("<h1>Context: " + context.getContextPath() + "</h1>");
super.writeErrorHtmlBody(request, writer, code, message, cause, showStacks);
super.writeErrorHtmlBody(request, writer, code, message, cause);
}
});
_server.start();
Expand Down
43 changes: 21 additions & 22 deletions jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
}
7 changes: 7 additions & 0 deletions jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/Dispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}
Expand Down
Loading
Loading