From 3321ae71bec1102da821cab05905a849addccfe0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 9 Feb 2021 10:03:00 +0100 Subject: [PATCH] Fix #5939 unwrap ServletException (#5958) * Fix #5939 unwrap ServletException Fix #5939 unwrap ServletException Signed-off-by: Greg Wilkins * Fix #5939 unwrap ServletException reverted default to false Signed-off-by: Greg Wilkins --- .../jetty/servlet/ErrorPageErrorHandler.java | 46 +++++++++++++++---- .../eclipse/jetty/servlet/ErrorPageTest.java | 18 +++++++- 2 files changed, 53 insertions(+), 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 index 8409831e93e0..f5ebf2bc892d 100644 --- 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 _errorPages = new HashMap<>(); // code or exception to URL private final List _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 index 8fd8046a399b..766ba66d7f29 100644 --- 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")); } }