Skip to content

Commit

Permalink
Issue #4334 - Improve testing of ErrorHandler behavior
Browse files Browse the repository at this point in the history 
+ Cleanup from PR review

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@joakime
joakime committed Nov 20, 2019
1 parent 9e40fc9 commit cf0df6e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 31 deletions.
20 changes: 9 additions & 11 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
View file
Expand Up @@ -482,28 +482,26 @@ 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);
while (th != null)
if (th != null)
{
writer.write("<h3>Caused by:</h3><pre>");
// You have to pre-generate and then use #write(writer, String)
StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw);
th.printStackTrace(pw);
pw.flush();
write(writer, sw.getBuffer().toString()); // IMPORTANT STEP
try (StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw))
{
th.printStackTrace(pw);
pw.flush();
write(writer, sw.getBuffer().toString()); // sanitize
}
writer.write("</pre>\n");

th = th.getCause();
}
}

Expand Down
46 changes: 26 additions & 20 deletions jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java
View file
Expand Up @@ -31,13 +31,15 @@
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;
Expand All @@ -47,12 +49,14 @@

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);
Expand All @@ -66,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;
}

Expand All @@ -80,43 +84,40 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques

if (target.startsWith("/badmessage/"))
{
int code = Integer.valueOf(target.substring(target.lastIndexOf('/') + 1));
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/"))
{
StringBuilder message = new StringBuilder();
message.append("{\n \"glossary\": {\n \"title\": \"example\"\n }\n }");
throw new ServletException(new RuntimeException(message.toString()));
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/"))
{
StringBuilder message = new StringBuilder();
message.append("<!DOCTYPE glossary PUBLIC \"-//OASIS//DTD DocBook V3.1//EN\">\n");
message.append(" <glossary>\n");
message.append(" <title>example glossary</title>\n");
message.append(" </glossary>");
throw new ServletException(new RuntimeException(message.toString()));
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/"))
{
StringBuilder message = new StringBuilder();
message.append("<hr/><script>alert(42)</script>");
throw new ServletException(new RuntimeException(message.toString()));
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/"))
{
StringBuilder message = new StringBuilder();
message.append("Euro is &euro; and \u20AC and %E2%82%AC");
throw new ServletException(new RuntimeException(message.toString()));
String message = "Euro is &euro; and \u20AC and %E2%82%AC";
throw new ServletException(new RuntimeException(message));
}
}
});
Expand All @@ -127,6 +128,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
public static void after() throws Exception
{
server.stop();
stacklessLogging.close();
}

@Test
Expand Down Expand Up @@ -469,12 +471,16 @@ private String assertContent(HttpTester.Response response)
}
else if (contentType.contains("text/json"))
{
Map<Object, Object> jo = (Map)JSON.parse(response.getContent());
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));
assertThat("message field", jo.get("message"), is(notNullValue()));
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"))
{
Expand Down Expand Up @@ -528,7 +534,7 @@ public void testJsonResponseWorse(String path) throws Exception
if (path.startsWith("/utf8"))
{
// we are expecting UTF-8 output, look for it.
assertThat("content", content, containsString("Euro is &euro; and \u20AC and %E2%82%AC"));
assertThat("content", content, containsString("Euro is &amp;euro; and \u20AC and %E2%82%AC"));
}
}
}

0 comments on commit cf0df6e

Please sign in to comment.