Reactive applications on Tomcat not compatible with servlet filters

[mwisnicki]

Affects: 5.x/current

I have a Servlet filter that adds request headers via decorating request object (the only way allowed by Servlet API).

class AddHeaders extends HttpServletRequestWrapper {
    private Map<String, String> headers;

    public AddHeaders(HttpServletRequest request, Map<String, String> headers) {
        super(request);
        this.headers = headers;
    }

    public String getHeader(String name) {
        return headers.getOrDefault(name, super.getHeader(name));
    }

    public Enumeration getHeaderNames() {
        List<String> names = Collections.list(super.getHeaderNames());
        names.addAll(headers.keySet());
        return Collections.enumeration(names);
    }
}

Unfortunately TomcatServerHttpRequest/TomcatHeadersAdapter bypasses request object and reaches directly into backing map thus my wrapper is ignored.

Generic ServletServerHttpRequest looks to handle it correctly but Tomcat subclass seems to have been too optimized.

[bclozel]

@mwisnicki I’ll look into skipping the optimization in this case. In the meantime, I think using a ˋWebFilter` instead for this will be much more efficient as you’ll keep the optimization. We’ve measured the performance improvement of this one and it’s not negligible.

Is your filter doing anything else or just adding a header?

[mwisnicki]

This was just a minimal reproducible test case. Unfortunately I have no control over actual servlet code :(

[bclozel]

Then you should make sure that the filter does not perform blocking operations (like an HTTP client fetching a token synchronously) or is not reading/writing to the request or response with the blocking Servlet APIs.

This would completely defeat the runtime benefits of WebFlux.

[rstoyanchev]

Generally speaking, the Servlet API is not meant to be used directly in a WebFlux application, and we don't aim to support it. You might run into other similar limitations since the ServletHttpHandlerAdapter layer assumes it is at the start of request handling. As Brian mentioned, using a WebFilter and ServerWebExchange is really what we expect an application to use.

[bclozel]

While we don't actively support using Servlet Filters with WebFlux, we will try and avoid using the native server optimizations when we see a request being wrapped. In such cases, this will undo performance improvements where we avoid copying HTTP request/response headers, and use Servlet container specific buffer APIs to reduce copying to/from byte[].

Again, using any Servlet Filter with a WebFlux application can lead to production issues as such filters are likely to interfere with the Async IO operations performed by our Servlet reactive bridge. This is now scheduled for 6.1.x.

[bclozel]

After working on this issue, I think that undoing this native adaptation would bring no benefit and would make performance worse for most WebFlux applications. The HttpServletRequestWrapper and HttpServletResponseWrapper cases are handled for compatibility reasons in all adapter implementations. The only behavior change we can consider here would be to reject requests and responses if they are wrapped and fail the exchange. This is a breaking change that brings no added value and would break existing apps for no good reason.

In this very case, the reporter has no control over actual servlet code which is very likely to cause important issues, even if this particular behavior is changed.

I'll turn this issue into a documentation issue - let's improve the message there and underline that one should not interact with Servlet-level contracts in this case as they're likely to face this issue or worse, mix async and blocking I/O code in the same app.