Skip to content

Commit

Permalink
Issue #12505 server to servlet api error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@janbartel
janbartel committed Nov 21, 2024
1 parent 1c1dfc9 commit 37ab820
Show file tree
Hide file tree
Showing 7 changed files with 444 additions and 3 deletions.
2 changes: 2 additions & 0 deletions jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
}

ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class);
servletContextRequest.getServletChannel().associate(request, response, callback);
HttpServletRequest httpServletRequest = servletContextRequest.getServletApiRequest();
HttpServletResponse httpServletResponse = servletContextRequest.getHttpServletResponse();
ServletContextHandler contextHandler = servletContextRequest.getServletContext().getServletContextHandler();
Expand All @@ -111,6 +112,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
}
finally
{
httpServletResponse.flushBuffer();
contextHandler.requestDestroyed(servletContextRequest, httpServletRequest);
}
callback.succeeded();
Expand Down
143 changes: 143 additions & 0 deletions ...ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.net.URI;
import java.net.URL;
import java.nio.file.FileSystem;
Expand All @@ -32,16 +33,26 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import jakarta.servlet.DispatcherType;
import jakarta.servlet.GenericServlet;
import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.ee.WebAppClassLoading;
import org.eclipse.jetty.ee10.servlet.DefaultServlet;
import org.eclipse.jetty.ee10.servlet.Dispatcher;
import org.eclipse.jetty.ee10.servlet.ErrorPageErrorHandler;
import org.eclipse.jetty.ee10.servlet.ServletChannel;
import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
import org.eclipse.jetty.ee10.servlet.ServletHolder;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.LocalConnector;
Expand Down Expand Up @@ -84,6 +95,7 @@
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -153,6 +165,50 @@ private Path createWar(Path tempDir, String name) throws Exception
return warFile;
}

@Test
public void testErrorPage() throws Exception
{
WebAppContext contextHandler = new WebAppContext();
contextHandler.setContextPath("/foo");
contextHandler.setBaseResourceAsPath(Path.of("/tmp"));
ServletHolder defaultHolder = new ServletHolder(new DefaultServlet());
defaultHolder.setDisplayName("default");

contextHandler.addServlet(defaultHolder, "/");
contextHandler.addServlet(ErrorDumpServlet.class, "/error/*");
contextHandler.addServlet(GlobalErrorDumpServlet.class, "/global/*");
ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler();
errorPageErrorHandler.addErrorPage(404, "/error/TestException");
errorPageErrorHandler.addErrorPage(ErrorPageErrorHandler.GLOBAL_ERROR_PAGE, "/global/TestException");
contextHandler.setErrorHandler(errorPageErrorHandler);
Server server = new Server();
server.setHandler(contextHandler);

LocalConnector connector = new LocalConnector(server);
server.addConnector(connector);
server.start();

try (StacklessLogging stackless = new StacklessLogging(ServletChannel.class))
{
StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /foo/WEB-INF/classes/this/does/not/exist").append(" HTTP/1.1\r\n");
rawRequest.append("Host: test\r\n");
rawRequest.append("Connection: close\r\n");
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());

HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(404));
assertThat(response.getValuesList("ERRORDUMPSERVLET"), contains("ERRORDUMPSERVLET"));
String content = response.getContent();
assertThat(content, containsString("ERROR_REQUEST_URI: /foo/WEB-INF/classes/this/does/not/exist"));
assertThat(content, containsString("getRequestURI()=[/foo/error/TestException]"));
assertThat(content, containsString("DISPATCH: ERROR"));
assertThat(content, not(containsString("GLOBALERRORDUMPSERVLET")));
}
}

@Test
public void testDefaultContextPath() throws Exception
{
Expand Down Expand Up @@ -518,6 +574,93 @@ public void service(ServletRequest req, ServletResponse res)
}
}

public static class ErrorDumpServlet extends HttpServlet
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
if (request.getDispatcherType() != DispatcherType.ERROR && request.getDispatcherType() != DispatcherType.ASYNC)
throw new IllegalStateException("Bad Dispatcher Type " + request.getDispatcherType());

response.setHeader("ERRORDUMPSERVLET", "ERRORDUMPSERVLET");
PrintWriter writer = response.getWriter();
writer.println("DISPATCH: " + request.getDispatcherType().name());
writer.println("ERROR_PAGE: " + request.getPathInfo());
writer.println("ERROR_MESSAGE: " + request.getAttribute(Dispatcher.ERROR_MESSAGE));
writer.println("ERROR_CODE: " + request.getAttribute(Dispatcher.ERROR_STATUS_CODE));
writer.println("ERROR_EXCEPTION: " + request.getAttribute(Dispatcher.ERROR_EXCEPTION));
writer.println("ERROR_EXCEPTION_TYPE: " + request.getAttribute(Dispatcher.ERROR_EXCEPTION_TYPE));
writer.println("ERROR_SERVLET: " + request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
writer.println("ERROR_REQUEST_URI: " + request.getAttribute(Dispatcher.ERROR_REQUEST_URI));

writer.printf("getRequestURI()=%s%n", valueOf(request.getRequestURI()));
writer.printf("getRequestURL()=%s%n", valueOf(request.getRequestURL()));
writer.printf("getQueryString()=%s%n", valueOf(request.getQueryString()));
Map<String, String[]> params = request.getParameterMap();
writer.printf("getParameterMap().size=%d%n", params.size());
for (Map.Entry<String, String[]> entry : params.entrySet())
{
String value = null;
if (entry.getValue() != null)
{
value = String.join(", ", entry.getValue());
}
writer.printf("getParameterMap()[%s]=%s%n", entry.getKey(), valueOf(value));
}
}

protected String valueOf(Object obj)
{
if (obj == null)
return "null";
return valueOf(obj.toString());
}

protected String valueOf(String str)
{
if (str == null)
return "null";
return String.format("[%s]", str);
}
}

public static class GlobalErrorDumpServlet extends ErrorDumpServlet
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
if (request.getDispatcherType() != DispatcherType.ERROR && request.getDispatcherType() != DispatcherType.ASYNC)
throw new IllegalStateException("Bad Dispatcher Type " + request.getDispatcherType());

response.setHeader("GLOBALERRORDUMPSERVLET", "GLOBALERRORDUMPSERVLET");

PrintWriter writer = response.getWriter();
writer.println("GLOBAL DISPATCH: " + request.getDispatcherType().name());
writer.println("GLOBAL ERROR_PAGE: " + request.getPathInfo());
writer.println("GLOBAL ERROR_MESSAGE: " + request.getAttribute(Dispatcher.ERROR_MESSAGE));
writer.println("GLOBAL ERROR_CODE: " + request.getAttribute(Dispatcher.ERROR_STATUS_CODE));
writer.println("GLOBAL ERROR_EXCEPTION: " + request.getAttribute(Dispatcher.ERROR_EXCEPTION));
writer.println("GLOBAL ERROR_EXCEPTION_TYPE: " + request.getAttribute(Dispatcher.ERROR_EXCEPTION_TYPE));
writer.println("GLOBAL ERROR_SERVLET: " + request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
writer.println("GLOBAL ERROR_REQUEST_URI: " + request.getAttribute(Dispatcher.ERROR_REQUEST_URI));

writer.printf("getRequestURI()=%s%n", valueOf(request.getRequestURI()));
writer.printf("getRequestURL()=%s%n", valueOf(request.getRequestURL()));
writer.printf("getQueryString()=%s%n", valueOf(request.getQueryString()));
Map<String, String[]> params = request.getParameterMap();
writer.printf("getParameterMap().size=%d%n", params.size());
for (Map.Entry<String, String[]> entry : params.entrySet())
{
String value = null;
if (entry.getValue() != null)
{
value = String.join(", ", entry.getValue());
}
writer.printf("getParameterMap()[%s]=%s%n", entry.getKey(), valueOf(value));
}
}
}

@Test
public void testBaseResourceAbsolutePath(WorkDir workDir) throws Exception
{
Expand Down
5 changes: 5 additions & 0 deletions jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import java.nio.charset.StandardCharsets;
import java.util.List;

import jakarta.servlet.RequestDispatcher;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
Expand Down Expand Up @@ -65,7 +67,9 @@ public boolean handle(Request request, Response response, Callback callback) thr
generateCacheControl(response);

ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class);
servletContextRequest.getServletChannel().associate(request, response, callback);
HttpServletRequest httpServletRequest = servletContextRequest.getServletApiRequest();

HttpServletResponse httpServletResponse = servletContextRequest.getHttpServletResponse();
ServletContextHandler contextHandler = servletContextRequest.getServletContext().getServletContextHandler();
String cacheControl = getCacheControl();
Expand All @@ -91,6 +95,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
}
finally
{
httpServletResponse.flushBuffer();
contextHandler.requestDestroyed(servletContextRequest, httpServletRequest);
}
callback.succeeded();
Expand Down
4 changes: 3 additions & 1 deletion ...etty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ErrorPageErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import org.eclipse.jetty.server.Request;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -138,7 +139,8 @@ public String getErrorPage(HttpServletRequest request)
errorPage = _errorPages.get(GLOBAL_ERROR_PAGE);
}

if (LOG.isDebugEnabled())

if (LOG.isDebugEnabled())
{
StringBuilder dbg = new StringBuilder();
dbg.append("getErrorPage(");
Expand Down
4 changes: 2 additions & 2 deletions ...1/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletApiResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,8 @@ public PrintWriter getWriter() throws IOException
// We must use an implementation of AbstractOutputStreamWriter here as we rely on the non cached characters
// in the writer implementation for flush and completion operations.
WriteThroughWriter outputStreamWriter = WriteThroughWriter.newWriter(getServletChannel().getHttpOutput(), encoding);
getServletResponseInfo().setWriter(writer = new ResponseWriter(
outputStreamWriter, locale, encoding));
writer = new ResponseWriter(outputStreamWriter, locale, encoding);
getServletResponseInfo().setWriter(writer);
}

// Set the output type at the end, because setCharacterEncoding() checks for it.
Expand Down
Loading

0 comments on commit 37ab820

Please sign in to comment.