[ckawl]

Issue #, if available:

Description of changes:

Servlet filters registered on a Jersey handler now run before the JAX-RS resource, the way they run in front of a servlet in a servlet container.

The problem

JerseyLambdaContainerHandler.initialize() registered Jersey as a filter mapped to /*, and JerseyHandlerFilter.doFilter() ran the resource before calling filterChain.doFilter(). The default factory methods (getAwsProxyHandler, getHttpApiV2ProxyHandler) call initialize() before returning, so any filter an application added afterwards, through onStartup() or getServletContext().addFilter(), was placed after Jersey. It ran only once the resource had executed and written its response. A filter that ended the request without calling chain.doFilter() could change the status code, but the resource had already run and its body was returned.

Building the handler with the constructor, registering filters, and then calling initialize() avoided this, but nothing documented that the order mattered.

The change

• Jersey runs as a servlet (JerseyServlet, mapped to /*) passed to doFilter() as the terminal step of the chain. This is the same mechanism the Spring module uses for DispatcherServlet: FilterChainManager already appends the servlet after every matching filter, so filters run first and in registration order, however and whenever they were registered.
• JerseyHandlerFilter.doFilter() no longer calls filterChain.doFilter() after Jersey has written the response. The Jersey call moved to a package-private handle() that the servlet uses.
• The servlet is initialized in initialize() because the filter chain reads its name from the servlet config when it builds the chain. The base class's initialize() isn't reused for this because it throws a checked exception that Jersey's public initialize() doesn't declare.
• Calling initialize() a second time used to throw a NullPointerException, because the duplicate filter registration returned null. It now works.

Reordering the calls inside JerseyHandlerFilter.doFilter() was not enough: when a filter ends the request, control just returns to the Jersey filter, which can't tell and runs the resource anyway.

Behavior changes for applications

• A filter that never calls chain.doFilter() now stops the request before Jersey runs, which is standard servlet behavior. In applications whose filters forgot to continue the chain, those requests no longer reach Jersey.
• Filters that set request attributes or wrap the request now affect what the resource sees, because they run first.
• getServletContext().getFilterRegistration("JerseyFilter") returns null. Jersey is registered as the servlet JerseyServlet instead.

No changes outside the Jersey module.

Testing

• New JerseyFilterChainTest, 96 cases across API Gateway, ALB and HTTP API events, all sent through proxyStream. Without the change, 48 fail and 3 error:
  • a filter registered through onStartup() after a factory method that ends the request must stop it before the resource runs
  • a filter scoped to /filter-chain/admin/* must leave other paths served, and must apply to the other spellings of a protected path that Jersey routes to it (doubled slashes, matrix parameters, trailing slash, dot segments after the prefix, encoded ?). Spellings that Jersey answers with 404 stay in the list as guards.
  • a pass-through filter runs before the resource, and filters registered before and after initialize() run in registration order
  • a filter that forwards to another path after a factory method
  • initialize() called twice
• Cases that pass both before and after, showing unchanged behavior: the constructor + initialize() registration order, forward dispatch from a filter registered before initialize(), a filter that sets a response header before continuing, and a handler with no filters.
• Jersey module: 198 tests, 11 skipped as on main, coverage checks met, no new SpotBugs findings.
• gha_build.sh jersey true true: the Jersey archetype project builds with Maven and Gradle against this change and its tests pass, and the Jersey pet-store sample builds with Maven and Gradle.
• Core: unchanged. Locally the same two environment-dependent tests fail as on main (serverName_albHostHeader_returnsHostHeader, getMimeType_mimeTypeOfJavascript_expectApplicationJavascript).

Related

The same production change applies to the other two maintained lines. The diffs to the main source files are identical across all three apart from javax imports on 1.x. Only the new test's scaffolding differs: on both backports Jackson 2's readValue throws a checked IOException, and on 1.x the test filters implement init and destroy because the Servlet 3.1 Filter interface has no default methods.

• 2.x: fix(jersey): run servlet filters before Jersey resources 2.1.x #1724
• 1.x: fix(jersey): run servlet filters before Jersey resources 1.x #1725

By submitting this pull request

• I confirm that my contribution is made under the terms of the Apache 2.0 license.
• I confirm that I've made a best effort attempt to update all relevant documentation.

[seshubaws]

Now that Jersey runs as a real servlet, should this return jerseyServlet instead of null? It's public, and returning the actual terminal servlet would match what the handler routes to. If keeping null is intentional (e.g. to avoid exposing the private inner class), a short comment explaining that would help.

[seshubaws]

With this change JerseyHandlerFilter is no longer registered as a filter, and doFilter() ignores the chain, so it's effectively just a holder for handle(). Keeping implements Filter for 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 marking doFilter() @Deprecated so nobody relies on it as a chain-continuing filter?