Skip to content

Commit

Permalink
cherry-pick of #7803 (#8708)
Browse files Browse the repository at this point in the history 
Signed-off-by: Olivier Lamy <olamy@apache.org>
  • Loading branch information
@olamy
olamy committed Oct 13, 2022
1 parent acb13e2 commit 573f27f
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 17 deletions.
16 changes: 15 additions & 1 deletion ...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public String getErrorPage(HttpServletRequest request)

if (error instanceof ServletException && _unwrapServletException)
{
Throwable unwrapped = ((ServletException)error).getRootCause();
Throwable unwrapped = getFirstNonServletException(error);
if (unwrapped != null)
{
request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped);
Expand Down Expand Up @@ -172,6 +172,20 @@ public String getErrorPage(HttpServletRequest request)
return errorPage;
}

/**
*
* @param t the initial exception
* @return the first non {@link ServletException} from root cause chain
*/
private Throwable getFirstNonServletException(Throwable t)
{
if (t instanceof ServletException && t.getCause() != null)
{
return getFirstNonServletException(t.getCause());
}
return t;
}

public Map<String, String> getErrorPages()
{
return _errorPages;
Expand Down
30 changes: 30 additions & 0 deletions ...y-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public void init() throws Exception
_context.addFilter(SingleDispatchFilter.class, "/*", EnumSet.allOf(DispatcherType.class));
_context.addServlet(DefaultServlet.class, "/");
_context.addServlet(FailServlet.class, "/fail/*");
_context.addServlet(FailServletDoubleWrap.class, "/fail-double-wrap/*");
_context.addServlet(FailClosedServlet.class, "/fail-closed/*");
_context.addServlet(ErrorServlet.class, "/error/*");
_context.addServlet(AppServlet.class, "/app/*");
Expand Down Expand Up @@ -320,6 +321,14 @@ public void testErrorException() throws Exception
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class jakarta.servlet.ServletException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee10.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/exception"));
response = _connector.getResponse("GET /fail-double-wrap/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: jakarta.servlet.ServletException: jakarta.servlet.ServletException: java.lang.IllegalStateException: Test Exception"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class jakarta.servlet.ServletException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee10.servlet.ErrorPageTest$FailServletDoubleWrap-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail-double-wrap/exception"));
}

_errorPageErrorHandler.setUnwrapServletException(true);
Expand All @@ -333,6 +342,14 @@ public void testErrorException() throws Exception
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class java.lang.IllegalStateException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee10.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/exception"));
response = _connector.getResponse("GET /fail-double-wrap/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.ee10.servlet.ErrorPageTest$FailServletDoubleWrap-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail-double-wrap/exception"));
}
}

Expand Down Expand Up @@ -660,6 +677,19 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
}
}

public static class FailServletDoubleWrap extends HttpServlet implements Servlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
String code = request.getParameter("code");
if (code != null)
response.sendError(Integer.parseInt(code));
else
throw new ServletException(new ServletException(new IllegalStateException("Test Exception")));
}
}

public static class FailClosedServlet extends HttpServlet implements Servlet
{
@Override
Expand Down
16 changes: 15 additions & 1 deletion .../jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public String getErrorPage(HttpServletRequest request)

if (error instanceof ServletException && _unwrapServletException)
{
Throwable unwrapped = ((ServletException)error).getRootCause();
Throwable unwrapped = getFirstNonServletException(error);
if (unwrapped != null)
{
request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped);
Expand Down Expand Up @@ -177,6 +177,20 @@ public String getErrorPage(HttpServletRequest request)
return errorPage;
}

/**
*
* @param t the initial exception
* @return the first non {@link ServletException} from root cause chain
*/
private Throwable getFirstNonServletException(Throwable t)
{
if (t instanceof ServletException && t.getCause() != null)
{
return getFirstNonServletException(t.getCause());
}
return t;
}

public Map<String, String> getErrorPages()
{
return _errorPages;
Expand Down
60 changes: 45 additions & 15 deletions jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

@Disabled // TODO
//@Disabled // TODO
public class ErrorPageTest
{
private static final Logger LOG = LoggerFactory.getLogger(ErrorPageTest.class);
Expand Down Expand Up @@ -91,6 +91,7 @@ public void init() throws Exception
_context.addFilter(SingleDispatchFilter.class, "/*", EnumSet.allOf(DispatcherType.class));
_context.addServlet(DefaultServlet.class, "/");
_context.addServlet(FailServlet.class, "/fail/*");
_context.addServlet(FailServletDoubleWrap.class, "/fail-double-wrap/*");
_context.addServlet(FailClosedServlet.class, "/fail-closed/*");
_context.addServlet(ErrorServlet.class, "/error/*");
_context.addServlet(AppServlet.class, "/app/*");
Expand Down Expand Up @@ -167,7 +168,7 @@ public void testErrorOverridesMimeTypeAndCharset() throws Exception
assertThat(body, containsString("ERROR_CODE: 595"));
assertThat(body, containsString("ERROR_EXCEPTION: null"));
assertThat(body, containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(body, containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$ErrorContentTypeCharsetWriterInitializedServlet-"));
assertThat(body, containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$ErrorContentTypeCharsetWriterInitializedServlet-"));
assertThat(body, containsString("ERROR_REQUEST_URI: /error-mime-charset-writer/"));
}

Expand All @@ -181,7 +182,7 @@ public void testErrorOverridesStatus() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 594"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$ErrorAndStatusServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$ErrorAndStatusServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /error-and-status/anything"));
}

Expand Down Expand Up @@ -276,7 +277,7 @@ public void testSendErrorClosedResponse() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 599"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailClosedServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailClosedServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail-closed/"));

assertThat(response, not(containsString("This shouldn't be seen")));
Expand All @@ -291,7 +292,7 @@ public void testErrorCode() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 599"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/code"));
}

Expand All @@ -307,8 +308,16 @@ public void testErrorException() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 500"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: jakarta.servlet.ServletException: java.lang.IllegalStateException: Test Exception"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class jakarta.servlet.ServletException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/exception"));
response = _connector.getResponse("GET /fail-double-wrap/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: jakarta.servlet.ServletException: jakarta.servlet.ServletException: java.lang.IllegalStateException: Test Exception"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class jakarta.servlet.ServletException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailServletDoubleWrap-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail-double-wrap/exception"));
}

_errorPageErrorHandler.setUnwrapServletException(true);
Expand All @@ -320,8 +329,16 @@ public void testErrorException() throws Exception
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_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/exception"));
response = _connector.getResponse("GET /fail-double-wrap/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.ee9.servlet.ErrorPageTest$FailServletDoubleWrap-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail-double-wrap/exception"));
}
}

Expand All @@ -334,7 +351,7 @@ public void testGlobalErrorCode() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 598"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/global"));
}

Expand All @@ -349,7 +366,7 @@ public void testGlobalErrorException() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 500"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: java.lang.NumberFormatException: For input string: \"NAN\""));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class java.lang.NumberFormatException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$FailServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/global"));
}
}
Expand All @@ -366,7 +383,7 @@ public void testBadMessage() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 400"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: org.eclipse.jetty.http.BadMessageException: 400: Bad query encoding"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: class org.eclipse.jetty.http.BadMessageException"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$AppServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$AppServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /app"));
assertThat(response, Matchers.containsString("getParameterMap()= {}"));
}
Expand All @@ -383,7 +400,7 @@ public void testAsyncErrorPageDSC() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 599"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$AsyncSendErrorServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$AsyncSendErrorServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /async/info"));
assertTrue(__asyncSendErrorCompleted.await(10, TimeUnit.SECONDS));
}
Expand All @@ -400,7 +417,7 @@ public void testAsyncErrorPageSDC() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 599"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$AsyncSendErrorServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$AsyncSendErrorServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /async/info"));
assertTrue(__asyncSendErrorCompleted.await(10, TimeUnit.SECONDS));
}
Expand All @@ -417,7 +434,7 @@ public void testAsyncErrorPageSCD() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 599"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$AsyncSendErrorServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$AsyncSendErrorServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /async/info"));
assertTrue(__asyncSendErrorCompleted.await(10, TimeUnit.SECONDS));
}
Expand All @@ -433,7 +450,7 @@ public void testNoop() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 404"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.DefaultServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.DefaultServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /noop/info"));
}

Expand All @@ -447,7 +464,7 @@ public void testNotEnough() throws Exception
assertThat(response, Matchers.containsString("ERROR_CODE: 500"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION: null"));
assertThat(response, Matchers.containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$NotEnoughServlet-"));
assertThat(response, Matchers.containsString("ERROR_SERVLET: org.eclipse.jetty.ee9.servlet.ErrorPageTest$NotEnoughServlet-"));
assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /notenough/info"));
}

Expand Down Expand Up @@ -639,6 +656,19 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
}
}

public static class FailServletDoubleWrap extends HttpServlet implements Servlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
String code = request.getParameter("code");
if (code != null)
response.sendError(Integer.parseInt(code));
else
throw new ServletException(new ServletException(new IllegalStateException("Test Exception")));
}
}

public static class FailClosedServlet extends HttpServlet implements Servlet
{
@Override
Expand Down

0 comments on commit 573f27f

Please sign in to comment.