From 1d2d192be318d712eee16093c01ad8903d51907d Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Mon, 26 Dec 2022 13:00:51 +1000 Subject: [PATCH 1/2] [TCK] if any matching error page we keep the exception matching the error page found Signed-off-by: Olivier Lamy --- .../ee10/servlet/ErrorPageErrorHandler.java | 9 ++-- .../jetty/ee10/servlet/ErrorPageTest.java | 40 ++++++++++++++++++ .../ee9/servlet/ErrorPageErrorHandler.java | 9 ++-- .../jetty/ee9/servlet/ErrorPageTest.java | 41 +++++++++++++++++++ 4 files changed, 93 insertions(+), 6 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java index ec7ecbee4969..fa89086ed339 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java @@ -93,7 +93,7 @@ public String getErrorPage(HttpServletRequest request) if (error instanceof ServletException && _unwrapServletException) { - Throwable unwrapped = getFirstNonServletException(error); + Throwable unwrapped = getFirstNonServletException(error, matchedThrowable); if (unwrapped != null) { request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped); @@ -175,13 +175,16 @@ public String getErrorPage(HttpServletRequest request) /** * * @param t the initial exception + * @param matchedThrowable the class we found matching the error page (can be null) * @return the first non {@link ServletException} from root cause chain */ - private Throwable getFirstNonServletException(Throwable t) + private Throwable getFirstNonServletException(Throwable t, Class matchedThrowable) { + if (matchedThrowable != null && t.getClass() == matchedThrowable) + return t; if (t instanceof ServletException && t.getCause() != null) { - return getFirstNonServletException(t.getCause()); + return getFirstNonServletException(t.getCause(), matchedThrowable); } return t; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java index a558e44a383e..bcdcf9eda059 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java @@ -29,6 +29,7 @@ import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; import jakarta.servlet.FilterConfig; +import jakarta.servlet.RequestDispatcher; import jakarta.servlet.Servlet; import jakarta.servlet.ServletException; import jakarta.servlet.ServletRequest; @@ -101,6 +102,7 @@ public void init() throws Exception _context.addServlet(DeleteServlet.class, "/delete/*"); _context.addServlet(ErrorAndStatusServlet.class, "/error-and-status/*"); _context.addServlet(ErrorContentTypeCharsetWriterInitializedServlet.class, "/error-mime-charset-writer/*"); + _context.addServlet(ExceptionServlet.class, "/exception-servlet"); Handler.Wrapper noopHandler = new Handler.Wrapper() { @@ -124,6 +126,7 @@ public boolean process(Request request, Response response, Callback callback) th _errorPageErrorHandler.addErrorPage(IllegalStateException.class.getCanonicalName(), "/error/TestException"); _errorPageErrorHandler.addErrorPage(BadMessageException.class, "/error/BadMessageException"); _errorPageErrorHandler.addErrorPage(ErrorPageErrorHandler.GLOBAL_ERROR_PAGE, "/error/GlobalErrorPage"); + _errorPageErrorHandler.addErrorPage(TestServletException.class, "/error"); _server.start(); _stackless = new StacklessLogging(ServletHandler.class); @@ -529,6 +532,18 @@ public void testUnavailable() throws Exception } } + @Test + public void testNonUnwrappedMatchExceptionWithErrorPage() throws Exception + { + try (StacklessLogging stackless = new StacklessLogging(ServletChannel.class)) + { + String response = _connector.getResponse("GET /exception-servlet HTTP/1.0\r\n\r\n"); + assertThat(response, Matchers.containsString("HTTP/1.1 500 Server Error")); + assertThat(response, Matchers.containsString("ERROR_EXCEPTION: org.eclipse.jetty.ee10.servlet.ErrorPageTest$TestServletException")); + assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class org.eclipse.jetty.ee10.servlet.ErrorPageTest$TestServletException")); + } + } + public static class AppServlet extends HttpServlet implements Servlet { @Override @@ -850,4 +865,29 @@ public void destroy() { } } + + public static class ExceptionServlet extends HttpServlet implements Servlet + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + throw new TestServletException(new TestException("error page invoked")); + } + } + + private static class TestException extends Exception + { + public TestException(String message) + { + super(message); + } + } + + public static class TestServletException extends ServletException + { + public TestServletException(Throwable rootCause) + { + super(rootCause); + } + } } diff --git a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java index 16ed401d722a..8c4806c955d0 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java @@ -98,7 +98,7 @@ public String getErrorPage(HttpServletRequest request) if (error instanceof ServletException && _unwrapServletException) { - Throwable unwrapped = getFirstNonServletException(error); + Throwable unwrapped = getFirstNonServletException(error, matchedThrowable); if (unwrapped != null) { request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped); @@ -180,13 +180,16 @@ public String getErrorPage(HttpServletRequest request) /** * * @param t the initial exception + * @param matchedThrowable the class we found matching the error page (can be null) * @return the first non {@link ServletException} from root cause chain */ - private Throwable getFirstNonServletException(Throwable t) + private Throwable getFirstNonServletException(Throwable t, Class matchedThrowable) { + if (matchedThrowable != null && t.getClass() == matchedThrowable) + return t; if (t instanceof ServletException && t.getCause() != null) { - return getFirstNonServletException(t.getCause()); + return getFirstNonServletException(t.getCause(), matchedThrowable); } return t; } diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java index f4b15b8d6f80..8d697bebaff8 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java @@ -29,6 +29,7 @@ import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; import jakarta.servlet.FilterConfig; +import jakarta.servlet.RequestDispatcher; import jakarta.servlet.Servlet; import jakarta.servlet.ServletException; import jakarta.servlet.ServletRequest; @@ -103,6 +104,7 @@ public void init() throws Exception _context.addServlet(DeleteServlet.class, "/delete/*"); _context.addServlet(ErrorAndStatusServlet.class, "/error-and-status/*"); _context.addServlet(ErrorContentTypeCharsetWriterInitializedServlet.class, "/error-mime-charset-writer/*"); + _context.addServlet(ExceptionServlet.class, "/exception-servlet"); HandlerWrapper noopHandler = new HandlerWrapper() { @@ -127,6 +129,8 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques _errorPageErrorHandler.addErrorPage(IllegalStateException.class.getCanonicalName(), "/error/TestException"); _errorPageErrorHandler.addErrorPage(BadMessageException.class, "/error/BadMessageException"); _errorPageErrorHandler.addErrorPage(ErrorPageErrorHandler.GLOBAL_ERROR_PAGE, "/error/GlobalErrorPage"); + _errorPageErrorHandler.addErrorPage(ErrorPageErrorHandler.GLOBAL_ERROR_PAGE, "/error/GlobalErrorPage"); + _errorPageErrorHandler.addErrorPage(TestServletException.class, "/error"); _server.start(); _stackless = new StacklessLogging(ServletHandler.class); @@ -518,6 +522,18 @@ public void testUnavailable() throws Exception } } + @Test + public void testNonUnwrappedMatchExceptionWithErrorPage() throws Exception + { + try (StacklessLogging stackless = new StacklessLogging(_context.getLogger())) + { + String response = _connector.getResponse("GET /exception-servlet HTTP/1.0\r\n\r\n"); + assertThat(response, Matchers.containsString("HTTP/1.1 500 Server Error")); + assertThat(response, Matchers.containsString("ERROR_EXCEPTION: org.eclipse.jetty.ee9.servlet.ErrorPageTest$TestServletException")); + assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class org.eclipse.jetty.ee9.servlet.ErrorPageTest$TestServletException")); + } + } + public static class AppServlet extends HttpServlet implements Servlet { @Override @@ -830,4 +846,29 @@ public void destroy() { } } + + public static class ExceptionServlet extends HttpServlet implements Servlet + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + throw new TestServletException(new TestException("error page invoked")); + } + } + + private static class TestException extends Exception + { + public TestException(String message) + { + super(message); + } + } + + public static class TestServletException extends ServletException + { + public TestServletException(Throwable rootCause) + { + super(rootCause); + } + } } From 36438582c26a64e61a2c03325bb47f35b58b1a6d Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Fri, 27 Jan 2023 18:02:29 +1000 Subject: [PATCH 2/2] rename of method Signed-off-by: Olivier Lamy --- .../eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java | 6 +++--- .../eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java index fa89086ed339..ce368fbbe970 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java @@ -93,7 +93,7 @@ public String getErrorPage(HttpServletRequest request) if (error instanceof ServletException && _unwrapServletException) { - Throwable unwrapped = getFirstNonServletException(error, matchedThrowable); + Throwable unwrapped = unwrapServletException(error, matchedThrowable); if (unwrapped != null) { request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped); @@ -178,13 +178,13 @@ public String getErrorPage(HttpServletRequest request) * @param matchedThrowable the class we found matching the error page (can be null) * @return the first non {@link ServletException} from root cause chain */ - private Throwable getFirstNonServletException(Throwable t, Class matchedThrowable) + private Throwable unwrapServletException(Throwable t, Class matchedThrowable) { if (matchedThrowable != null && t.getClass() == matchedThrowable) return t; if (t instanceof ServletException && t.getCause() != null) { - return getFirstNonServletException(t.getCause(), matchedThrowable); + return unwrapServletException(t.getCause(), matchedThrowable); } return t; } diff --git a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java index 8c4806c955d0..d043ae88bba9 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java +++ b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java @@ -98,7 +98,7 @@ public String getErrorPage(HttpServletRequest request) if (error instanceof ServletException && _unwrapServletException) { - Throwable unwrapped = getFirstNonServletException(error, matchedThrowable); + Throwable unwrapped = unwrapServletException(error, matchedThrowable); if (unwrapped != null) { request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped); @@ -183,13 +183,13 @@ public String getErrorPage(HttpServletRequest request) * @param matchedThrowable the class we found matching the error page (can be null) * @return the first non {@link ServletException} from root cause chain */ - private Throwable getFirstNonServletException(Throwable t, Class matchedThrowable) + private Throwable unwrapServletException(Throwable t, Class matchedThrowable) { if (matchedThrowable != null && t.getClass() == matchedThrowable) return t; if (t instanceof ServletException && t.getCause() != null) { - return getFirstNonServletException(t.getCause(), matchedThrowable); + return unwrapServletException(t.getCause(), matchedThrowable); } return t; }