Repository navigation
fix(jersey): run servlet filters before Jersey resources #1723
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,15 +30,16 @@ | |
| import org.glassfish.jersey.process.internal.RequestScoped; | ||
| import org.glassfish.jersey.server.ResourceConfig; | ||
|
|
||
| import jakarta.servlet.DispatcherType; | ||
| import jakarta.servlet.FilterRegistration; | ||
| import jakarta.servlet.Servlet; | ||
| import jakarta.servlet.ServletConfig; | ||
| import jakarta.servlet.ServletContext; | ||
| import jakarta.servlet.ServletRegistration; | ||
| import jakarta.servlet.ServletRequest; | ||
| import jakarta.servlet.ServletResponse; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import jakarta.ws.rs.core.Application; | ||
|
|
||
| import java.util.EnumSet; | ||
| import java.util.concurrent.CountDownLatch; | ||
|
|
||
|
|
||
|
|
@@ -72,7 +73,10 @@ public class JerseyLambdaContainerHandler<RequestType, ResponseType> extends Aws | |
| // Variables - Private | ||
| //------------------------------------------------------------- | ||
|
|
||
| private static final String JERSEY_SERVLET_NAME = "JerseyServlet"; | ||
|
|
||
| private JerseyHandlerFilter jerseyFilter; | ||
| private JerseyServlet jerseyServlet; | ||
| private boolean initialized; | ||
|
|
||
| //------------------------------------------------------------- | ||
|
|
@@ -174,6 +178,7 @@ protected void configure() { | |
| } | ||
|
|
||
| this.jerseyFilter = new JerseyHandlerFilter(jaxRsApplication); | ||
| this.jerseyServlet = new JerseyServlet(jerseyFilter); | ||
| Timer.stop("JERSEY_CONTAINER_CONSTRUCTOR"); | ||
| } | ||
|
|
||
|
|
@@ -194,7 +199,7 @@ protected void handleRequest(HttpServletRequest httpServletRequest, AwsHttpServl | |
| ((AwsHttpServletRequest)httpServletRequest).setServletContext(getServletContext()); | ||
| } | ||
|
|
||
| doFilter(httpServletRequest, httpServletResponse, null); | ||
| doFilter(httpServletRequest, httpServletResponse, jerseyServlet); | ||
| Timer.stop("JERSEY_HANDLE_REQUEST"); | ||
| } | ||
|
|
||
|
|
@@ -207,12 +212,13 @@ protected AwsHttpServletResponse getContainerResponse(HttpServletRequest request | |
| public void initialize() { | ||
| Timer.start("JERSEY_COLD_START_INIT"); | ||
|
|
||
| // manually add the filter to the chain. This should the last one and match all uris | ||
| FilterRegistration.Dynamic jerseyFilterReg = getServletContext().addFilter("JerseyFilter", jerseyFilter); | ||
| jerseyFilterReg.addMappingForUrlPatterns( | ||
| EnumSet.of(DispatcherType.REQUEST, DispatcherType.ASYNC, DispatcherType.INCLUDE, DispatcherType.FORWARD), | ||
| true, "/*" | ||
| ); | ||
| // Jersey runs as the servlet at the end of the filter chain, the way DispatcherServlet does for Spring. Every | ||
| // filter registered with the servlet context, before or after this method, runs first and can stop the | ||
| // request before it reaches a resource. | ||
| ServletRegistration.Dynamic jerseyServletReg = getServletContext().addServlet(JERSEY_SERVLET_NAME, jerseyServlet); | ||
| jerseyServletReg.addMapping("/*"); | ||
| // the filter chain reads the servlet name from its config, so the servlet has to be initialized up front | ||
| jerseyServlet.init(((AwsServletRegistration) jerseyServletReg).getServletConfig()); | ||
|
|
||
| Timer.stop("JERSEY_COLD_START_INIT"); | ||
| initialized = true; | ||
|
|
@@ -226,7 +232,53 @@ public InjectionManager getInjectionManager() { | |
| return jerseyFilter.getApplicationHandler().getInjectionManager(); | ||
| } | ||
|
|
||
| /** | ||
| * The servlet Jersey runs as, which is the terminal step of the filter chain for every request this handler | ||
| * routes. This returned null while Jersey ran as a filter and there was no servlet to hand back. | ||
| * @return The Jersey servlet | ||
| */ | ||
| public Servlet getServlet() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that Jersey runs as a real servlet, should this return
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| return null; | ||
| return jerseyServlet; | ||
| } | ||
|
|
||
| //------------------------------------------------------------- | ||
| // Inner Class - | ||
| //------------------------------------------------------------- | ||
|
|
||
| /** | ||
| * The servlet at the end of the filter chain. It hands the request to Jersey, which writes the response. | ||
| */ | ||
| private static final class JerseyServlet implements Servlet { | ||
| private final JerseyHandlerFilter jerseyFilter; | ||
| private ServletConfig config; | ||
|
|
||
| JerseyServlet(JerseyHandlerFilter jerseyFilter) { | ||
| this.jerseyFilter = jerseyFilter; | ||
| } | ||
|
|
||
| @Override | ||
| public void init(ServletConfig servletConfig) { | ||
| config = servletConfig; | ||
| } | ||
|
|
||
| @Override | ||
| public ServletConfig getServletConfig() { | ||
| return config; | ||
| } | ||
|
|
||
| @Override | ||
| public void service(ServletRequest servletRequest, ServletResponse servletResponse) { | ||
| jerseyFilter.handle(servletRequest, servletResponse); | ||
| } | ||
|
|
||
| @Override | ||
| public String getServletInfo() { | ||
| return ""; | ||
| } | ||
|
|
||
| @Override | ||
| public void destroy() { | ||
|
|
||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change
JerseyHandlerFilteris no longer registered as a filter, anddoFilter()ignores the chain, so it's effectively just a holder forhandle(). Keepingimplements Filterfor compatibility makes sense since the class is public, but could we update the class Javadoc (it still describes it as a "Servlet filter class") and consider markingdoFilter()@Deprecatedso nobody relies on it as a chain-continuing filter?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in ab3284e: updated the class Javadoc and deprecated
doFilter(). Nothing in the library calls it.