Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

keep matching exception to found errorpage #9090

Merged
merged 2 commits into from Jan 28, 2023
Merged

keep matching exception to found errorpage #9090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d2d192
[TCK] if any matching error page we keep the exception matching the e…
olamy Dec 26, 2022
3643858
rename of method
olamy Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions ...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
}
}
}