This is an automated email from the ASF dual-hosted git repository. dsoumis pushed a commit to branch add-json-error-report-valve-tests in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 252d60a6236634864ebc774b662ec191fc7a7425 Author: Dimitris Soumis <[email protected]> AuthorDate: Mon Mar 30 14:25:34 2026 +0300 Fix formatting and increase coverage by adding test cases --- .../catalina/valves/TestJsonErrorReportValve.java | 139 +++++++++++++++++---- 1 file changed, 116 insertions(+), 23 deletions(-) diff --git a/test/org/apache/catalina/valves/TestJsonErrorReportValve.java b/test/org/apache/catalina/valves/TestJsonErrorReportValve.java index f9d370dd6f..4770a93802 100644 --- a/test/org/apache/catalina/valves/TestJsonErrorReportValve.java +++ b/test/org/apache/catalina/valves/TestJsonErrorReportValve.java @@ -24,7 +24,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import jakarta.servlet.ServletException; import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServlet; @@ -43,8 +42,7 @@ import org.apache.tomcat.util.json.JSONParser; public class TestJsonErrorReportValve extends TomcatBaseTest { - private static final String JSON_VALVE = - "org.apache.catalina.valves.JsonErrorReportValve"; + private static final String JSON_VALVE = "org.apache.catalina.valves.JsonErrorReportValve"; @Test @@ -61,7 +59,6 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { tomcat.start(); ByteChunk res = new ByteChunk(); - res.setCharset(StandardCharsets.UTF_8); Map<String, List<String>> resHead = new HashMap<>(); int rc = getUrl("http://localhost:"; + getPort(), res, resHead); @@ -79,8 +76,7 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { LinkedHashMap<String, Object> json = parser.parseObject(); Assert.assertEquals("Status Report", json.get("type")); - Assert.assertEquals(500, - ((Number) json.get("status")).intValue()); + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ((Number) json.get("status")).intValue()); Assert.assertEquals("Server broke", json.get("message")); Assert.assertNotNull(json.get("description")); } @@ -100,7 +96,6 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { tomcat.start(); ByteChunk res = new ByteChunk(); - res.setCharset(StandardCharsets.UTF_8); int rc = getUrl("http://localhost:"; + getPort(), res, null); Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); @@ -145,7 +140,6 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { tomcat.start(); ByteChunk res = new ByteChunk(); - res.setCharset(StandardCharsets.UTF_8); int rc = getUrl("http://localhost:"; + getPort(), res, null); Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); @@ -156,17 +150,14 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { LinkedHashMap<String, Object> json = parser.parseObject(); Assert.assertEquals("Status Report", json.get("type")); - Assert.assertEquals(500, - ((Number) json.get("status")).intValue()); + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ((Number) json.get("status")).intValue()); // Verify the message field is present and contains the // expected substrings (the parser returns raw escaped values) String message = (String) json.get("message"); Assert.assertNotNull("message should be present", message); - Assert.assertTrue("message should contain quotes", - message.contains("quotes")); - Assert.assertTrue("message should contain backslash", - message.contains("backslash")); + Assert.assertTrue("message should contain quotes", message.contains("quotes")); + Assert.assertTrue("message should contain backslash", message.contains("backslash")); } @@ -184,7 +175,6 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { tomcat.start(); ByteChunk res = new ByteChunk(); - res.setCharset(StandardCharsets.UTF_8); int rc = getUrl("http://localhost:"; + getPort(), res, null); Assert.assertEquals(999, rc); @@ -194,9 +184,9 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { JSONParser parser = new JSONParser(body); LinkedHashMap<String, Object> json = parser.parseObject(); - Assert.assertEquals(999, - ((Number) json.get("status")).intValue()); + Assert.assertEquals(999, ((Number) json.get("status")).intValue()); Assert.assertEquals("The sky is falling", json.get("message")); + Assert.assertNotNull(json.get("description")); } @@ -266,6 +256,107 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { throwableStr.contains("RuntimeException")); Assert.assertTrue("Response should contain root cause", throwableStr.contains("IllegalStateException")); + Assert.assertFalse("Catalina core classes should be filtered", + throwableStr.contains("org.apache.catalina.core.")); + } + + @Test + public void testJsonErrorWithoutMessage() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(JSON_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "noMessage", new SendErrorServlet( + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null)); + ctx.addServletMappingDecoded("/", "noMessage"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + + String body = res.toString(); + JSONParser parser = new JSONParser(body); + LinkedHashMap<String, Object> json = parser.parseObject(); + + Assert.assertEquals("Status Report", json.get("type")); + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ((Number) json.get("status")).intValue()); + Assert.assertEquals("", json.get("message")); + Assert.assertNotNull(json.get("description")); + } + + @Test + public void testNoJsonBodyForNonErrorStatus() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(JSON_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + Map<String, List<String>> resHead = new HashMap<>(); + int rc = getUrl("http://localhost:"; + getPort(), res, resHead); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + + String body = res.toString(); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, body); + } + + @Test + public void testJsonErrorWithUnicodeMessage() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(JSON_VALVE); + + Context ctx = getProgrammaticRootContext(); + + String unicodeMessage = "Error: \u00e9\u00e8\u00ea \u4e2d\u6587 \u00f1"; + Tomcat.addServlet(ctx, "unicode", new SendErrorServlet( + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, unicodeMessage)); + ctx.addServletMappingDecoded("/", "unicode"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + res.setCharset(StandardCharsets.UTF_8); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + + String body = res.toString(); + JSONParser parser = new JSONParser(body); + LinkedHashMap<String, Object> json = parser.parseObject(); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ((Number) json.get("status")).intValue()); + Assert.assertEquals(unicodeMessage, json.get("message")); + } + + @Test + public void testNoJsonForUnknownStatusWithoutMessage() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(JSON_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "unknownNoMessage", new SendErrorServlet(999, null)); + ctx.addServletMappingDecoded("/", "unknownNoMessage"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(999, rc); + + String body = res.toString(); + Assert.assertTrue(body == null || body.isEmpty()); } @@ -282,8 +373,12 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) - throws ServletException, IOException { - resp.sendError(statusCode, message); + throws IOException { + if (message != null) { + resp.sendError(statusCode, message); + } else { + resp.sendError(statusCode); + } } } @@ -298,8 +393,7 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { } @Override - public void service(ServletRequest request, ServletResponse response) - throws IOException { + public void service(ServletRequest request, ServletResponse response) { throw new RuntimeException(message); } } @@ -310,8 +404,7 @@ public class TestJsonErrorReportValve extends TomcatBaseTest { private static final long serialVersionUID = 1L; @Override - public void service(ServletRequest request, ServletResponse response) - throws IOException { + public void service(ServletRequest request, ServletResponse response) { throw new RuntimeException("Outer exception", new IllegalStateException("Root cause")); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
