Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4335 from eclipse/jetty-9.4.x-improve-error-handler-output 

Issue #4334 - Improve testing of ErrorHandler behavior
This commit is contained in:
Joakim Erdfelt 2019-11-20 15:21:07 -06:00 committed by GitHub
parent cba60d2e37 cf0df6e3ff
commit ba1fe2826d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 288 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
View File

@ -21,6 +21,7 @@ package org.eclipse.jetty.server.handler;
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 @@ public class ErrorHandler extends AbstractHandler
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("<pre>");
while (th != null)
writer.write("<h3>Caused by:</h3><pre>");
// 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("</pre>\n");
}

348
jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java
View File

@ -20,7 +20,6 @@ package org.eclipse.jetty.server;
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.HttpServletRequest;
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 class ErrorHandlerTest
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 class ErrorHandlerTest
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 =
"<!DOCTYPE glossary PUBLIC \"-//OASIS//DTD DocBook V3.1//EN\">\n" +
" <glossary>\n" +
" <title>example glossary</title>\n" +
" </glossary>";
throw new ServletException(new RuntimeException(message));
}
// produce an exception with an HTML cause message
if (target.startsWith("/htmlmessage/"))
{
String message = "<hr/><script>alert(42)</script>%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 &euro; and \u20AC and %E2%82%AC";
throw new ServletException(new RuntimeException(message));
}
}
});
@ -90,193 +128,238 @@ public class ErrorHandlerTest
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, startsWith("HTTP/1.1 404 "));
assertThat(response, not(containsString("Content-Length: 0")));
assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1"));
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 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 class ErrorHandlerTest
"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 class ErrorHandlerTest
"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 &amp;euro; 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 &amp;euro; 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("<script>")));
assertThat(content, not(containsString("<glossary>")));
assertThat(content, not(containsString("<!DOCTYPE>")));
assertThat(content, not(containsString("&euro;")));
}
else if (contentType.contains("text/json"))
{
Map jo = (Map)JSON.parse(response.getContent());
assertThat("url field", jo.get("url"), is(notNullValue()));
String expectedStatus = String.valueOf(response.getStatus());
assertThat("status field", jo.get("status"), is(expectedStatus));
String message = (String)jo.get("message");
assertThat("message field", message, is(notNullValue()));
assertThat("message field", message, anyOf(
not(containsString("<")),
not(containsString(">"))));
}
else if (contentType.contains("text/plain"))
{
assertThat(content, containsString("STATUS: " + response.getStatus()));
}
else
{
System.out.println("Not checked Content-Type: " + contentType);
System.out.println(content);
}
return content;
}
@Test
public void testJsonResponse() throws Exception
{
String rawResponse = connector.getResponse(
"GET /badmessage/444 HTTP/1.1\r\n" +
"Host: Localhost\r\n" +
"Accept: text/json\r\n" +
"\r\n");
"GET /badmessage/444 HTTP/1.1\r\n" +
"Host: Localhost\r\n" +
"Accept: text/json\r\n" +
"\r\n");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat("Response status code", response.getStatus(), is(444));
System.out.println("response:" + response.getContent());
assertContent(response);
}
Map<Object,Object> jo = (Map) JSON.parse(response.getContent());
@ParameterizedTest
@ValueSource(strings = {
"/jsonmessage/",
"/xmlmessage/",
"/htmlmessage/",
"/utf8message/",
})
public void testJsonResponseWorse(String path) throws Exception
{
String rawResponse = connector.getResponse(
"GET " + path + " HTTP/1.1\r\n" +
"Host: Localhost\r\n" +
"Accept: text/json\r\n" +
"\r\n");
assertThat("url field null", jo.get("url"), is(notNullValue()));
assertThat("status field null", jo.get("status"), is(notNullValue()));
assertThat("message field null", jo.get("message"), is(notNullValue()));
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat("Response status code", response.getStatus(), is(500));
String content = assertContent(response);
if (path.startsWith("/utf8"))
{
// we are expecting UTF-8 output, look for it.
assertThat("content", content, containsString("Euro is &amp;euro; and \u20AC and %E2%82%AC"));
}
}
}