Skip to content

Commit

Permalink
Merge pull request #9739 from eclipse/jetty-12.0.x-9637-ServletReques…
Browse files Browse the repository at this point in the history 
…tListener

Issue #9637 - Update behaviour and add testing for ServletRequestListener
  • Loading branch information
@lachlan-roberts
lachlan-roberts authored May 5, 2023
2 parents baf7224 + 65db09d commit d0b49ff
Show file tree
Hide file tree
Showing 4 changed files with 383 additions and 37 deletions.
15 changes: 11 additions & 4 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 @@ -89,15 +89,17 @@ public boolean handle(Request request, Response response, Callback callback) thr
}

ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class);

HttpServletRequest httpServletRequest = servletContextRequest.getServletApiRequest();
HttpServletResponse httpServletResponse = servletContextRequest.getHttpServletResponse();
ServletContextHandler contextHandler = servletContextRequest.getContext().getServletContextHandler();
String cacheControl = getCacheControl();
if (cacheControl != null)
response.getHeaders().put(HttpHeader.CACHE_CONTROL.asString(), cacheControl);

// Look for an error page dispatcher
// This logic really should be in ErrorPageErrorHandler, but some implementations extend ErrorHandler
// and implement ErrorPageMapper directly, so we do this here in the base class.
String errorPage = (this instanceof ErrorPageMapper) ? ((ErrorPageMapper)this).getErrorPage(servletContextRequest.getServletApiRequest()) : null;
String errorPage = (this instanceof ErrorPageMapper) ? ((ErrorPageMapper)this).getErrorPage(httpServletRequest) : null;
ServletContextHandler.ServletScopedContext context = servletContextRequest.getErrorContext();
Dispatcher errorDispatcher = (errorPage != null && context != null)
? (Dispatcher)context.getServletContext().getRequestDispatcher(errorPage) : null;
Expand All @@ -106,7 +108,8 @@ public boolean handle(Request request, Response response, Callback callback) thr
{
try
{
errorDispatcher.error(servletContextRequest.getServletApiRequest(), servletContextRequest.getHttpServletResponse());
contextHandler.requestInitialized(servletContextRequest, httpServletRequest);
errorDispatcher.error(httpServletRequest, httpServletResponse);
callback.succeeded();
return true;
}
Expand All @@ -119,12 +122,16 @@ public boolean handle(Request request, Response response, Callback callback) thr
return true;
}
}
finally
{
contextHandler.requestDestroyed(servletContextRequest, httpServletRequest);
}
}

String message = (String)request.getAttribute(Dispatcher.ERROR_MESSAGE);
if (message == null)
message = HttpStatus.getMessage(response.getStatus());
generateAcceptableResponse(servletContextRequest, servletContextRequest.getServletApiRequest(), servletContextRequest.getHttpServletResponse(), response.getStatus(), message);
generateAcceptableResponse(servletContextRequest, httpServletRequest, httpServletResponse, response.getStatus(), message);
callback.succeeded();
return true;
}
Expand Down
80 changes: 49 additions & 31 deletions ...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,19 @@ public boolean handle()
{
dispatch(() ->
{
ServletHandler servletHandler = _context.getServletContextHandler().getServletHandler();
ServletHandler.MappedServlet mappedServlet = _servletContextRequest._mappedServlet;
try
{
_context.getServletContextHandler().requestInitialized(_servletContextRequest, _servletContextRequest.getServletApiRequest());

ServletHandler servletHandler = _context.getServletContextHandler().getServletHandler();
ServletHandler.MappedServlet mappedServlet = _servletContextRequest._mappedServlet;

mappedServlet.handle(servletHandler, Request.getPathInContext(_servletContextRequest), _servletContextRequest.getServletApiRequest(), _servletContextRequest.getHttpServletResponse());
mappedServlet.handle(servletHandler, Request.getPathInContext(_servletContextRequest), _servletContextRequest.getServletApiRequest(), _servletContextRequest.getHttpServletResponse());
}
finally
{
_context.getServletContextHandler().requestDestroyed(_servletContextRequest, _servletContextRequest.getServletApiRequest());
}
});

break;
Expand All @@ -433,39 +442,48 @@ public boolean handle()
{
dispatch(() ->
{
HttpURI uri;
String pathInContext;
AsyncContextEvent asyncContextEvent = _state.getAsyncContextEvent();
String dispatchString = asyncContextEvent.getDispatchPath();
if (dispatchString != null)
try
{
String contextPath = _context.getContextPath();
HttpURI.Immutable dispatchUri = HttpURI.from(dispatchString);
pathInContext = URIUtil.canonicalPath(dispatchUri.getPath());
uri = HttpURI.build(_servletContextRequest.getHttpURI())
.path(URIUtil.addPaths(contextPath, pathInContext))
.query(dispatchUri.getQuery());
}
else
{
uri = asyncContextEvent.getBaseURI();
if (uri == null)
_context.getServletContextHandler().requestInitialized(_servletContextRequest, _servletContextRequest.getServletApiRequest());

HttpURI uri;
String pathInContext;
AsyncContextEvent asyncContextEvent = _state.getAsyncContextEvent();
String dispatchString = asyncContextEvent.getDispatchPath();
if (dispatchString != null)
{
uri = _servletContextRequest.getHttpURI();
pathInContext = Request.getPathInContext(_servletContextRequest);
String contextPath = _context.getContextPath();
HttpURI.Immutable dispatchUri = HttpURI.from(dispatchString);
pathInContext = URIUtil.canonicalPath(dispatchUri.getPath());
uri = HttpURI.build(_servletContextRequest.getHttpURI())
.path(URIUtil.addPaths(contextPath, pathInContext))
.query(dispatchUri.getQuery());
}
else
{
pathInContext = uri.getCanonicalPath();
if (_context.getContextPath().length() > 1)
pathInContext = pathInContext.substring(_context.getContextPath().length());
uri = asyncContextEvent.getBaseURI();
if (uri == null)
{
uri = _servletContextRequest.getHttpURI();
pathInContext = Request.getPathInContext(_servletContextRequest);
}
else
{
pathInContext = uri.getCanonicalPath();
if (_context.getContextPath().length() > 1)
pathInContext = pathInContext.substring(_context.getContextPath().length());
}
}
}
// We first worked with the core pathInContext above, but now need to convert to servlet style
String decodedPathInContext = URIUtil.decodePath(pathInContext);
// We first worked with the core pathInContext above, but now need to convert to servlet style
String decodedPathInContext = URIUtil.decodePath(pathInContext);

Dispatcher dispatcher = new Dispatcher(getContextHandler(), uri, decodedPathInContext);
dispatcher.async(asyncContextEvent.getSuppliedRequest(), asyncContextEvent.getSuppliedResponse());
Dispatcher dispatcher = new Dispatcher(getContextHandler(), uri, decodedPathInContext);
dispatcher.async(asyncContextEvent.getSuppliedRequest(), asyncContextEvent.getSuppliedResponse());
}
finally
{
_context.getServletContextHandler().requestDestroyed(_servletContextRequest, _servletContextRequest.getServletApiRequest());
}
});
break;
}
Expand Down Expand Up @@ -513,6 +531,8 @@ public boolean handle()
// _state.completing();
try (Blocker.Callback blocker = Blocker.callback())
{
// We do not notify ServletRequestListener on this dispatch because it might not
// be dispatched to an error page, so we delegate this responsibility to the ErrorHandler.
dispatch(() -> errorHandler.handle(_servletContextRequest, getResponse(), blocker));
blocker.block();
}
Expand Down Expand Up @@ -662,7 +682,6 @@ private void dispatch(Dispatchable dispatchable) throws Exception
try
{
_servletContextRequest.getResponse().getHttpOutput().reopen();
_context.getServletContextHandler().requestInitialized(_servletContextRequest, _servletContextRequest.getServletApiRequest());
getHttpOutput().reopen();
_combinedListener.onBeforeDispatch(_servletContextRequest);
dispatchable.dispatch();
Expand All @@ -675,7 +694,6 @@ private void dispatch(Dispatchable dispatchable) throws Exception
finally
{
_combinedListener.onAfterDispatch(_servletContextRequest);
_context.getServletContextHandler().requestDestroyed(_servletContextRequest, _servletContextRequest.getServletApiRequest());
}
}

Expand Down
3 changes: 1 addition & 2 deletions ...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import jakarta.servlet.descriptor.JspConfigDescriptor;
import jakarta.servlet.descriptor.JspPropertyGroupDescriptor;
import jakarta.servlet.descriptor.TaglibDescriptor;
import jakarta.servlet.http.HttpFilter;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
Expand Down Expand Up @@ -1450,7 +1449,7 @@ public FilterHolder addFilter(String filterClass, String pathSpec, EnumSet<Dispa
* @param dispatches the dispatcher types for this filter
* @return the FilterHolder that was created
*/
public FilterHolder addFilter(HttpFilter filter, String pathSpec, EnumSet<DispatcherType> dispatches)
public FilterHolder addFilter(Filter filter, String pathSpec, EnumSet<DispatcherType> dispatches)
{
FilterHolder filterHolder = new FilterHolder(filter);
getServletHandler().addFilterWithMapping(filterHolder, pathSpec, dispatches);
Expand Down
Loading

0 comments on commit d0b49ff

Please sign in to comment.