StringHttpMessageConverter truncates output when data is compressed

[fmiguelez]

Affects: 6.1.3

When migrating from Spring Boot 2.7.17 (Spring Framework 5.3.30) to 3.1.2 (SF 6.1.3) we have detected that our REST API controller method using a @RequestBody String parameter receives truncated data when compression is enabled. We have verified that the exact number of bytes or resulting string match the Content-Type header of the HTTP request, which indicates the data length in compressed format (not uncompressed).

After verifying with Wireshark that the HTTP request is correct we have come down to StringHttpMessageConverter.readInternal() method that on new version looks like this:

  protected String readInternal(Class<? extends String> clazz, HttpInputMessage inputMessage) throws IOException {
    Charset charset = this.getContentTypeCharset(inputMessage.getHeaders().getContentType());
    long length = inputMessage.getHeaders().getContentLength(); 
    byte[] bytes = length >= 0L && length <= 2147483647L ? inputMessage.getBody().readNBytes((int)length) : inputMessage.getBody().readAllBytes();
    return new String(bytes, charset);
  }

On (working) version 5.3.30 it looked like the following:

	protected String readInternal(Class<? extends String> clazz, HttpInputMessage inputMessage) throws IOException {
		Charset charset = getContentTypeCharset(inputMessage.getHeaders().getContentType());
		return StreamUtils.copyToString(inputMessage.getBody(), charset);
	}

In newer version it uses the original Content-Length HTTP header value to limit the amount of data being read (inputMessage.getBody().readNBytes() produces uncompressed output), while the older version tried to read up to the end of the input stream (which seems more correct).

Below is a sample controller method where parameter gets truncated:

    @Override
    @PutMapping("/datasets/{dataset}/{entity-type}")
    public void publishEntities(@PathVariable("dataset") String dataset, @PathVariable("entity-type") String entityType, @RequestBody String entities,
            @RequestParam(name = "waitForStore", defaultValue = "false") Boolean waitForStore,
            @RequestParam(name = "compact", defaultValue = "false") Boolean compact) {
            
           // Here "entities" parameter gets truncated when compression is enabled

     }

[bclozel]

I'm not sure I understand. What component is decompressing the incoming request body? Is this the Servlet container? Or a custom component? Arguably, a component that decompresses the request body should also update the content length header.

I think the change introduced in #30942 makes perfect sense. If the incoming request has a body that's larger than the declared content-length, I'd say the request is invalid.

[fmiguelez]

@snicoll thanks for the pointer. I have reviewed the link provided, but have not found anything that applies to this case.

@bclozel As indicated we did not change our code, only the required changes when upgrading (mostly security-related) from SB 2.7.17/SF 5.3.30 to SB 3.2.1/SF 6.1.3.

We have a REST controller with a method defined as indicated above. Such piece of code stays the same.

I suppose it is the Servlet container (Undertow) the one uncompressing the data transparently. To enable it we define these two properties in our application:

server.compression.enabled=true
server.compression.min-response-size=1024

[bclozel]

The server.compression.enabled property says:

Whether response compression is enabled.

Spring Boot does not configure anything related to request compression support; it's been declined so far in spring-projects/spring-boot#11827. In guess in your case Undertow is supporting this directly.

Just like the content-length is changed by the Servlet container when the response is compressed, I think the opposite should also be true: if the response body is decompressed, the content length header should be consistent. I'd suggest creating an issue with the Undertow project directly in this case. You should provide them with a minimal sample application that does not involve Spring at all to demonstrate the problem.

Thanks!

[fmiguelez]

@bclozel while you might be right regarding that Spring Boot does not state that such property affects requests, only response, the reality is that it does.

I also agree that it seems an Undertow issue, but I do not think that SpringBoot/Spring Framework team should simply ignore this issue. IMHO as I see it:

• Underlying Undertow/Tomcat server configuration is performed through Spring Boot framework properties
• The choice of the underlying server to use lies on Spring Boot maintainers/developers, not Spring Boot users. So if Undertow has an issue, it should be known and handled by SB/SF team
• Previous versions of SB/SF behaved differently, regardless of whether the SB/SF layer was unconsciously masking an unknown Undertow BUG.

Please reconsider a further analysis of the issue.

[bclozel]

while you might be right regarding that Spring Boot does not state that such property affects requests, only response, the reality is that it does.

Again, I don't think Spring Boot is configuring the request decompression support. I assume the same behavior can be seen without this property being enabled as it's done by Undertow automatically.

The choice of the underlying server to use lies on Spring Boot maintainers/developers, not Spring Boot users. So if Undertow has an issue, it should be known and handled by SB/SF team

I'm not sure about what you're asking. For any issue with any library we're integrating with, we should be responsible for fixing the problem in those libraries? This is not sustainable for our team and I don't think it's reasonable to expect that. On top of that, the default Servlet container implementation we provide in Spring Boot is Tomcat.

I understand that this is an unpleasant behavior change for your application, but the only possibility for us would be to revert #30942. Most of our community doesn't use Undertow nor the request compression support and it would be unfair to undo a performance optimization for something we suspect is a Servlet container bug.

By replying to this issue, I've already helped you to track down the most probable source of the problem. I'll be happy to reopen this issue if it turns out we don't behave as expected with regards to HTTP. Please engage with the Undertow team to get their opinion, as it's the best course of action to get this problem fixed.

[fmiguelez]

Again, I don't think Spring Boot is configuring the request decompression support. I assume the same behavior can be seen without this property being enabled as it's done by Undertow automatically.

The outcome is that through a SB property it gets configured, whether it is a desired behavior or not. SB/SF users do not have control over what parameters are passed to Undertow/Tomcat.

I understand that this is an unpleasant behavior change for your application, but the only possibility for us would be to revert #30942. Most of our community doesn't use Undertow nor the request compression support and it would be unfair to undo a performance optimization for something we suspect is a Servlet container bug.

More than an unpleasant behaviour, it is a show stopper for us regarding upgrading to latest SB/SF. It seems there were detected more breaking changes (#31327) regarding such optimization (#30942) and it was actually [undone for version 6.0.13] (#31327 (comment)), though recovered for 6.1. @rstoyanchev could probably also add his grain of salt to this.

By replying to this issue, I've already helped you to track down the most probable source of the problem. I'll be happy to reopen this issue if it turns out we don't behave as expected with regards to HTTP. Please engage with the Undertow team to get their opinion, as it's the best course of action to get this problem fixed.

Do not get me wrong. I do hugely appreciate your help to troubleshoot this issue and will contact Undertow to let them know about the issue. Thank you very much.

In any case should this get fixed on their side (by properly updating HTTP headers) SB/SF team would need to be involved in upgrading the dependency. So please do not let this issue get into oblvion.

[bclozel]

More than an unpleasant behaviour, it is a show stopper for us regarding upgrading to latest SB/SF. It seems there were detected more breaking changes (#31327) regarding such optimization (#30942) and it was actually [undone for version 6.0.13] (#31327 (comment)), though recovered for 6.1. @rstoyanchev could probably also add his grain of salt to this.

Yeah we temporarily undid that on the 6.0.x branch because it was too late for this generation to introduce behavior changes, but it's in 6.1.x because this is still the correct behavior.

In any case should this get fixed on their side (by properly updating HTTP headers) SB/SF team would need to be involved in upgrading the dependency. So please do not let this issue get into oblivion.

For each release, Spring Boot automatically upgrades all its dependencies to their latest maintenance versions. So I don't see anything to be done for now. If the issue is fixed in an upcoming Undertow version, this will be picked up automatically by Spring Boot.

[fmiguelez]

@bclozel just wanted to come back to you to let you know that you were right. After thoroughly reviewing all our code, I discovered that we were actually doing some configuration on Undertow to process compressed requests. We were actually doing this: https://stackoverflow.com/q/55516683/34880

I am testing a workaround and will post it on SO for future reference for those tripping on same stone.

The Undertow ticket: https://issues.redhat.com/browse/UNDERTOW-2340

Thank you very much for your help and support!!!

[bclozel]

Thanks for letting us know. I've subscribed to the issue.

As far as I understand, Jetty replaces the Content-Length header with a X-Content-Length header to keep the information around but to avoid being used when the body is read. I'm not sure what the correct behavior here as there is no RFC backing this up.