Reactive stack on Tomcat does not handle chunked transfer encoding correctly

[jshs]

Summary

HTTP 1.1 request bodies using Transfer-Encoding: chunked are sometimes rejected when running a reactive servlet on top of Tomcat.
This affects at least Spring Boot 3.2.3 when combining the webflux and tomcat starters.

Example

A complete code base to reproduce the problem is available here.

It consists of a simple server application with a POST endpoint:

  @PostMapping(path = "/length", consumes = "*/*", produces = "application/json")
  public Mono<Integer> length(ServerHttpRequest request) {
    return request.getBody().map(DataBuffer::readableByteCount).reduce(0, Integer::sum);
  }

and a client that streams a request to this endpoint:

  private static final int LENGTH = 1_000_000;

  void test() {
    Integer result = WebClient.builder()
        .baseUrl("http://localhost:" + port)
        .build()
        .post()
        .uri("/length")
        .body(BodyInserters.fromOutputStream(this::writeBody, executor))
        .retrieve()
        .bodyToMono(Integer.class)
        .block();
    assertEquals(LENGTH, result);
  }

  private void writeBody(OutputStream output) {
    try {
      for (int i = 0; i < LENGTH; ++i) {
        output.write(65);
        output.flush();
      }
    } catch (IOException e) {
      throw new RuntimeException(e);
    }
  }

The expected outcome is that the server responds as asserted. Instead, we observe a prematurely closed connection on the client and the following exception on the server:

org.apache.coyote.BadRequestException: Invalid chunk header

(others might be possible)

This example was extracted from a more complex application where we first noticed the problem, hence the artificial client setup.

Remarks

The above example is potentially a flaky test, although I can reliably reproduce the issue on my machine. The problem does not seem to occur with Undertow, nor if MVC is used on the server.

My hypothesis is that the Coyote HTTP/1.1 connector does not support the combination of a non-blocking socket and chunked transfer encoding. When parsing chunk headers or the CRLF sequence after each chunk, a read size of 0 results in the request being aborted:

• https://github.com/apache/tomcat/blob/870b52f410468b6155e8bf4bd4aaf8f92ef941a7/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java#L339
• https://github.com/apache/tomcat/blob/870b52f410468b6155e8bf4bd4aaf8f92ef941a7/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java#L409

These parsers would need to store their state and yield control (like doRead) if no input is available at the socket.

[poutsma]

So far, I am not capable of reproducing the issue with the linked sample. Could you please specify which OS and JVM versions you are using?

[jshs]

Thanks for looking into this issue. I observe the test failure using Microsoft Windows 11 Pro (10.0.22631 Build 22631) and OpenJDK 64-Bit Server VM 17.0.8.1 (Temurin). It doesn't appear to be reproducible on Linux, however.

Note: The --rerun flag should be used with gradle test to ensure that the test is actually run. I've updated the readme.

[poutsma]

I can verify that the tests fails under Windows 10 as well 11 (but not MacOS nor Linux). I can also add that the test succeeds with Jetty as well as Undertow; just not Tomcat.

[poutsma]

@markt-asf, does @jshs 's theory about the ChunkedInputFilter, listed in Remarks above, hold water? Or is Spring Framework doing something wrong here?

[poutsma]

Additionally, the test succeeds when using a JdkClientHttpConnector for the WebClient, instead of the default ReactorClientHttpConnector. So the issue only seems to occur when using Reactor Netty on the client in combination with Tomcat on the server.

[markt-asf]

@poutsma The theory that Tomcat doesn't handle a non-blocking read of a chunk body if a partial chunk header is received looks plausible. I'll create a Tomcat unit test to simulate that scenario and report back.

[markt-asf]

@poutsma Confirmed. It was a Tomcat bug. The bug has been fixed in all currently supported branches and will be included from the May releases.

[poutsma]

Thank you, @markt-asf. Is there an issue we can to link to here?

[markt-asf]

No issue, but this is the key commit for main: apache/tomcat@cbed8e1