Fix #5939 unwrap ServletException (#5958)

* Fix #5939 unwrap ServletException Fix #5939 unwrap ServletException Signed-off-by: Greg Wilkins <[email protected]> * Fix #5939 unwrap ServletException reverted default to false Signed-off-by: Greg Wilkins <[email protected]>
jetty · Feb 9, 2021 · 3321ae7 · 3321ae7
1 parent de76d44
commit 3321ae7
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 11 deletions.
diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ErrorPageErrorHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ErrorPageErrorHandler.java
@@ -41,9 +41,26 @@ private enum PageLookupTechnique
         THROWABLE, STATUS_CODE, GLOBAL
     }
 
-    protected ServletContext _servletContext;
     private final Map<String, String> _errorPages = new HashMap<>(); // code or exception to URL
     private final List<ErrorCodeRange> _errorPageList = new ArrayList<>(); // list of ErrorCode by range
+    protected ServletContext _servletContext;
+    private boolean _unwrapServletException = false;
+
+    /**
+     * @return True if ServletException is unwrapped for {@link Dispatcher#ERROR_EXCEPTION}
+     */
+    public boolean isUnwrapServletException()
+    {
+        return _unwrapServletException;
+    }
+
+    /**
+     * @param unwrapServletException True if ServletException should be unwrapped for {@link Dispatcher#ERROR_EXCEPTION}
+     */
+    public void setUnwrapServletException(boolean unwrapServletException)
+    {
+        _unwrapServletException = unwrapServletException;
+    }
 
     @Override
     public String getErrorPage(HttpServletRequest request)
@@ -53,14 +70,15 @@ public String getErrorPage(HttpServletRequest request)
         PageLookupTechnique pageSource = null;
 
         Class<?> matchedThrowable = null;
-        Throwable th = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+        Throwable error = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+        Throwable cause = error;
 
         // Walk the cause hierarchy
-        while (errorPage == null && th != null)
+        while (errorPage == null && cause != null)
         {
             pageSource = PageLookupTechnique.THROWABLE;
 
-            Class<?> exClass = th.getClass();
+            Class<?> exClass = cause.getClass();
             errorPage = _errorPages.get(exClass.getName());
 
             // walk the inheritance hierarchy
@@ -75,7 +93,17 @@ public String getErrorPage(HttpServletRequest request)
             if (errorPage != null)
                 matchedThrowable = exClass;
 
-            th = (th instanceof ServletException) ? ((ServletException)th).getRootCause() : null;
+            cause = (cause instanceof ServletException) ? ((ServletException)cause).getRootCause() : null;
+        }
+
+        if (error instanceof ServletException && _unwrapServletException)
+        {
+            Throwable unwrapped = ((ServletException)error).getRootCause();
+            if (unwrapped != null)
+            {
+                request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped);
+                request.setAttribute(Dispatcher.ERROR_EXCEPTION_TYPE, unwrapped.getClass());
+            }
         }
 
         Integer errorStatusCode = null;
@@ -129,7 +157,7 @@ public String getErrorPage(HttpServletRequest request)
                     Throwable originalThrowable = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
                     dbg.append(originalThrowable.getClass().getName());
                     dbg.append(')');
-                    LOG.debug(dbg.toString(), th);
+                    LOG.debug(dbg.toString(), cause);
                     break;
                 case STATUS_CODE:
                     dbg.append(" (from status code ");
@@ -225,9 +253,9 @@ protected void doStart() throws Exception
 
     private static class ErrorCodeRange
     {
-        private int _from;
-        private int _to;
-        private String _uri;
+        private final int _from;
+        private final int _to;
+        private final String _uri;
 
         ErrorCodeRange(int from, int to, String uri)
             throws IllegalArgumentException

diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java
@@ -296,17 +296,31 @@ public void testErrorCode() throws Exception
     @Test
     public void testErrorException() throws Exception
     {
+        _errorPageErrorHandler.setUnwrapServletException(false);
         try (StacklessLogging stackless = new StacklessLogging(HttpChannel.class))
         {
             String response = _connector.getResponse("GET /fail/exception HTTP/1.0\r\n\r\n");
             assertThat(response, Matchers.containsString("HTTP/1.1 500 Server Error"));
             assertThat(response, Matchers.containsString("ERROR_PAGE: /TestException"));
             assertThat(response, Matchers.containsString("ERROR_CODE: 500"));
-            assertThat(response, Matchers.containsString("ERROR_EXCEPTION: javax.servlet.ServletException: java.lang.IllegalStateException"));
+            assertThat(response, Matchers.containsString("ERROR_EXCEPTION: javax.servlet.ServletException: java.lang.IllegalStateException: Test Exception"));
             assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class javax.servlet.ServletException"));
             assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailServlet-"));
             assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/exception"));
         }
+
+        _errorPageErrorHandler.setUnwrapServletException(true);
+        try (StacklessLogging stackless = new StacklessLogging(HttpChannel.class))
+        {
+            String response = _connector.getResponse("GET /fail/exception HTTP/1.0\r\n\r\n");
+            assertThat(response, Matchers.containsString("HTTP/1.1 500 Server Error"));
+            assertThat(response, Matchers.containsString("ERROR_PAGE: /TestException"));
+            assertThat(response, Matchers.containsString("ERROR_CODE: 500"));
+            assertThat(response, Matchers.containsString("ERROR_EXCEPTION: java.lang.IllegalStateException: Test Exception"));
+            assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class java.lang.IllegalStateException"));
+            assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailServlet-"));
+            assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/exception"));
+        }
     }
 
     @Test
@@ -619,7 +633,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
             if (code != null)
                 response.sendError(Integer.parseInt(code));
             else
-                throw new ServletException(new IllegalStateException());
+                throw new ServletException(new IllegalStateException("Test Exception"));
         }
     }