[TCK] if any matching error page we keep the exception matching the e…

…rror page found Signed-off-by: Olivier Lamy <[email protected]>
jetty · Dec 27, 2022 · d8ba0d9 · d8ba0d9
1 parent 082e972
commit d8ba0d9
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 6 deletions.
diff --git a/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java b/...etty-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/...y-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java b/...y-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-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java b/.../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
@@ -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);
+        }
+    }
 }