diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index 4cc5389b1b2d..455d5339a3cc 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; +import java.io.StringWriter; import java.io.Writer; import java.nio.BufferOverflowException; import java.nio.ByteBuffer; @@ -481,24 +482,24 @@ private void writeErrorJson(HttpServletRequest request, PrintWriter writer, int writer.append(json.entrySet().stream() .map(e -> QuotedStringTokenizer.quote(e.getKey()) + ":" + - QuotedStringTokenizer.quote((e.getValue()))) + QuotedStringTokenizer.quote(StringUtil.sanitizeXmlString((e.getValue())))) .collect(Collectors.joining(",\n", "{\n", "\n}"))); - - } protected void writeErrorPageStacks(HttpServletRequest request, Writer writer) throws IOException { Throwable th = (Throwable)request.getAttribute(RequestDispatcher.ERROR_EXCEPTION); - if (_showStacks && th != null) + if (th != null) { - PrintWriter pw = writer instanceof PrintWriter ? (PrintWriter)writer : new PrintWriter(writer); - pw.write("
");
-            while (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);
-                th = th.getCause();
+                pw.flush();
+                write(writer, sw.getBuffer().toString()); // sanitize
             }
             writer.write("
\n"); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java index dcc30b1ff727..87184c19e325 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java @@ -20,7 +20,6 @@ import java.io.IOException; import java.util.Map; - import javax.servlet.DispatcherType; import javax.servlet.RequestDispatcher; import javax.servlet.ServletException; @@ -28,30 +27,36 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.BadMessageException; -import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.ajax.JSON; +import org.eclipse.jetty.util.log.StacklessLogging; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.startsWith; +import static org.hamcrest.Matchers.nullValue; public class ErrorHandlerTest { + static StacklessLogging stacklessLogging; static Server server; static LocalConnector connector; @BeforeAll public static void before() throws Exception { + stacklessLogging = new StacklessLogging(HttpChannel.class); server = new Server(); connector = new LocalConnector(server); server.addConnector(connector); @@ -65,7 +70,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques if (baseRequest.getDispatcherType() == DispatcherType.ERROR) { baseRequest.setHandled(true); - response.sendError(((Integer)request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE)).intValue()); + response.sendError((Integer)request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE)); return; } @@ -79,7 +84,40 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques if (target.startsWith("/badmessage/")) { - throw new ServletException(new BadMessageException(Integer.valueOf(target.substring(12)))); + int code = Integer.parseInt(target.substring(target.lastIndexOf('/') + 1)); + throw new ServletException(new BadMessageException(code)); + } + + // produce an exception with an JSON formatted cause message + if (target.startsWith("/jsonmessage/")) + { + String message = "{\n \"glossary\": {\n \"title\": \"example\"\n }\n }"; + throw new ServletException(new RuntimeException(message)); + } + + // produce an exception with an XML cause message + if (target.startsWith("/xmlmessage/")) + { + String message = + "\n" + + " \n" + + " example glossary\n" + + " "; + throw new ServletException(new RuntimeException(message)); + } + + // produce an exception with an HTML cause message + if (target.startsWith("/htmlmessage/")) + { + String message = "
%3Cscript%3E"; + throw new ServletException(new RuntimeException(message)); + } + + // produce an exception with a UTF-8 cause message + if (target.startsWith("/utf8message/")) + { + String message = "Euro is € and \u20AC and %E2%82%AC"; + throw new ServletException(new RuntimeException(message)); } } }); @@ -90,193 +128,238 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques public static void after() throws Exception { server.stop(); + stacklessLogging.close(); } @Test public void test404NoAccept() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); + + assertContent(response); } @Test public void test404EmptyAccept() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Accept: \r\n" + "Host: Localhost\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, containsString("Content-Length: 0")); - assertThat(response, not(containsString("Content-Type"))); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), is(0)); + assertThat("Response Content-Type", response.getField(HttpHeader.CONTENT_TYPE), is(nullValue())); } @Test public void test404UnAccept() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Accept: text/*;q=0\r\n" + "Host: Localhost\r\n" + "\r\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, containsString("Content-Length: 0")); - assertThat(response, not(containsString("Content-Type"))); + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), is(0)); + assertThat("Response Content-Type", response.getField(HttpHeader.CONTENT_TYPE), is(nullValue())); } @Test public void test404AllAccept() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: */*\r\n" + "\r\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1")); + assertContent(response); } @Test public void test404HtmlAccept() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); + + assertContent(response); } @Test public void testMoreSpecificAccept() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html, some/other;specific=true\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); + + assertContent(response); } @Test public void test404HtmlAcceptAnyCharset() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html\r\n" + "Accept-Charset: *\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=utf-8")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=UTF-8")); + + assertContent(response); } @Test public void test404HtmlAcceptUtf8Charset() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html\r\n" + "Accept-Charset: utf-8\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=utf-8")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=UTF-8")); + + assertContent(response); } @Test public void test404HtmlAcceptNotUtf8Charset() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html\r\n" + "Accept-Charset: utf-8;q=0\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); + + assertContent(response); } @Test public void test404HtmlAcceptNotUtf8UnknownCharset() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html\r\n" + "Accept-Charset: utf-8;q=0,unknown\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, containsString("Content-Length: 0")); - assertThat(response, not(containsString("Content-Type"))); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), is(0)); + assertThat("Response Content-Type", response.getField(HttpHeader.CONTENT_TYPE), is(nullValue())); } @Test public void test404HtmlAcceptUnknownUtf8Charset() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html\r\n" + "Accept-Charset: utf-8;q=0.1,unknown\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=utf-8")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=UTF-8")); + + assertContent(response); } @Test public void test404PreferHtml() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html;q=1.0,text/json;q=0.5,*/*\r\n" + "Accept-Charset: *\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/html;charset=utf-8")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=UTF-8")); + + assertContent(response); } @Test public void test404PreferJson() throws Exception { - String response = connector.getResponse( + String rawResponse = connector.getResponse( "GET / HTTP/1.1\r\n" + "Host: Localhost\r\n" + "Accept: text/html;q=0.5,text/json;q=1.0,*/*\r\n" + "Accept-Charset: *\r\n" + "\r\n"); - assertThat(response, startsWith("HTTP/1.1 404 ")); - assertThat(response, not(containsString("Content-Length: 0"))); - assertThat(response, containsString("Content-Type: text/json")); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/json")); + + assertContent(response); } @Test @@ -287,12 +370,14 @@ public void testCharEncoding() throws Exception "Host: Localhost\r\n" + "Accept: text/plain\r\n" + "\r\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat("Response status code", response.getStatus(), is(404)); - HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); - assertThat("Response Content-Type", contentType, is(notNullValue())); - assertThat("Response Content-Type value", contentType.getValue(), not(containsString("null"))); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/plain")); + + assertContent(response); } @Test @@ -302,29 +387,154 @@ public void testBadMessage() throws Exception "GET /badmessage/444 HTTP/1.1\r\n" + "Host: Localhost\r\n" + "\r\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat("Response status code", response.getStatus(), is(444)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); + + assertContent(response); + } + + @ParameterizedTest + @ValueSource(strings = { + "/jsonmessage/", + "/xmlmessage/", + "/htmlmessage/", + "/utf8message/", + }) + public void testComplexCauseMessageNoAcceptHeader(String path) throws Exception + { + String rawResponse = connector.getResponse( + "GET " + path + " HTTP/1.1\r\n" + + "Host: Localhost\r\n" + + "\r\n"); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(500)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); + + String content = assertContent(response); + + if (path.startsWith("/utf8")) + { + // we are Not expecting UTF-8 output, look for mangled ISO-8859-1 version + assertThat("content", content, containsString("Euro is € and ? and %E2%82%AC")); + } + } + + @ParameterizedTest + @ValueSource(strings = { + "/jsonmessage/", + "/xmlmessage/", + "/htmlmessage/", + "/utf8message/", + }) + public void testComplexCauseMessageAcceptUtf8Header(String path) throws Exception + { + String rawResponse = connector.getResponse( + "GET " + path + " HTTP/1.1\r\n" + + "Host: Localhost\r\n" + + "Accept: text/html\r\n" + + "Accept-Charset: utf-8\r\n" + + "\r\n"); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(500)); + assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); + assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=UTF-8")); + + String content = assertContent(response); + + if (path.startsWith("/utf8")) + { + // we are Not expecting UTF-8 output, look for mangled ISO-8859-1 version + assertThat("content", content, containsString("Euro is € and \u20AC and %E2%82%AC")); + } + } + + private String assertContent(HttpTester.Response response) + { + String contentType = response.get(HttpHeader.CONTENT_TYPE); + String content = response.getContent(); + + if (contentType.contains("text/html")) + { + assertThat(content, not(containsString("