Skip to content

Commit

Permalink
keep matching exception to found errorpage (#9090)
Browse files Browse the repository at this point in the history 
* [TCK] if any matching error page we keep the exception matching the error page found

Signed-off-by: Olivier Lamy <olamy@apache.org>
  • Loading branch information
@olamy
olamy committed Jan 28, 2023
1 parent cd732a1 commit b0e5074
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 6 deletions.
9 changes: 6 additions & 3 deletions ...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java
View file
Expand Up @@ -93,7 +93,7 @@ public String getErrorPage(HttpServletRequest request)

if (error instanceof ServletException && _unwrapServletException)
{
Throwable unwrapped = getFirstNonServletException(error);
Throwable unwrapped = unwrapServletException(error, matchedThrowable);
if (unwrapped != null)
{
request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped);
Expand Down Expand Up @@ -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 unwrapServletException(Throwable t, Class<?> matchedThrowable)
{
if (matchedThrowable != null && t.getClass() == matchedThrowable)
return t;
if (t instanceof ServletException && t.getCause() != null)
{
return getFirstNonServletException(t.getCause());
return unwrapServletException(t.getCause(), matchedThrowable);
}
return t;
}
Expand Down
40 changes: 40 additions & 0 deletions ...y-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}
9 changes: 6 additions & 3 deletions .../jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/ErrorPageErrorHandler.java
View file
Expand Up @@ -98,7 +98,7 @@ public String getErrorPage(HttpServletRequest request)

if (error instanceof ServletException && _unwrapServletException)
{
Throwable unwrapped = getFirstNonServletException(error);
Throwable unwrapped = unwrapServletException(error, matchedThrowable);
if (unwrapped != null)
{
request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped);
Expand Down Expand Up @@ -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 unwrapServletException(Throwable t, Class<?> matchedThrowable)
{
if (matchedThrowable != null && t.getClass() == matchedThrowable)
return t;
if (t instanceof ServletException && t.getCause() != null)
{
return getFirstNonServletException(t.getCause());
return unwrapServletException(t.getCause(), matchedThrowable);
}
return t;
}
Expand Down
41 changes: 41 additions & 0 deletions jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}

0 comments on commit b0e5074

Please sign in to comment.