Skip to content
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

EOFException in HttpMessageConverterExtractor while extracting data of a empty GZiped response #28613

Conversation

davidvieiratrustly
Copy link

Problem: When calling RestTemplate#exchange with response type != Void.class, if

  • The server responds with
    • an empty body and
    • Content-Encoding: gzip

we get a EOFException in the RestTemplate client.

Solution: IntrospectingClientHttpResponse#hasEmptyMessageBody should not throw an exception when the response body is an empty lazy gzip stream.

After reading the first byte, if it receives an EOFException, it should understand it as a signal of the end of the stream, thus returning true to the question "Has an empty message body?", not EOFException to the RestTemplate client.

Related issue:
#12671

@pivotal-cla
Copy link

@davidvieiratrustly Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@davidvieiratrustly Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 13, 2022
@davidvieiratrustly davidvieiratrustly force-pushed the fix-has-empty-message-body-for-eofexception-cases branch 2 times, most recently from 23c6b2a to 52df119 Compare June 13, 2022 15:46
should not throw an exception when the response body
is an empty lazy GZIP stream.
@davidvieiratrustly davidvieiratrustly force-pushed the fix-has-empty-message-body-for-eofexception-cases branch from 52df119 to f9ce8ba Compare June 13, 2022 16:38
@bclozel
Copy link
Member

bclozel commented Jun 13, 2022

I don't think an empty stream is valid gzip. This looks like an incorrect server response, which is in line with the exception thrown.

For example, you can try and create an empty gzip file and see that it's starting with the "magical number" 1f8b and contains data:

$ echo -n | gzip > empty.gz
$ ls -alh empty.gz
-rw-r--r--  1 bclozel  staff    20B Jun 13 21:03 empty.gz
$ xxd empty.gz
00000000: 1f8b 0800 f389 a762 0003 0300 0000 0000  .......b........
00000010: 0000 0000                                ....

We are probably going to decline this change. Unless you've seen servers produce valid gzip empty responses?
Can you show an example of a server producing such a stream (for example, using Tomcat or Netty)?

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Jun 13, 2022
@davidvieiratrustly
Copy link
Author

davidvieiratrustly commented Jun 13, 2022

We have this problem in production right now while making requests to a bank API. We don't know what server the bank is using.

I've created an example project to replicate the error using Netty Mock Server:
https://github.com/davidvieiratrustly/RestTemplateGzipBug

As EOFException documentation states:
This exception is mainly used by data input streams to signal end of stream.

We are reading the first byte of the response from the body into hasEmptyMessageBody; I don't think it's necessary to throw the exception, as when we get it, it's already indicative enough that the message body is empty.

So, we must catch this exception so that the RestTemplate is not coupled with the request client implementation.

Thank you for your time and assistance!

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 13, 2022
@rstoyanchev
Copy link
Contributor

Which is the underlying HTTP client library? I'm not sure why the InputStream raises an exception rather than returning -1. Also wondering why the response is treated as not empty. Could you share the response details, including status and headers?

@rstoyanchev rstoyanchev added this to the Triage Queue milestone Jun 30, 2022
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 30, 2022
@davidvieiratrustly
Copy link
Author

davidvieiratrustly commented Jun 30, 2022

I've tested with Apache Http Client (HttpComponentsClientHttpRequestFactory), OkHttp(OkHttp3ClientHttpRequestFactory) and SimpleJDK(SimpleClientHttpRequestFactory). The first two fail, while the Simple JDK works as inteded.

You can check the full test in this repo: https://github.com/davidvieiratrustly/RestTemplateGzipBug/blob/main/src/test/java/com/example/demo/Demo1ApplicationTests.java

Screenshot 2022-06-30 at 15 12 19

Also, everything works as expected in the new WebClient; it only fails for RestTemplate with OkHTTP and Apache HTTP Client.

Thank you in advance 🙏

@simonbasle simonbasle assigned simonbasle and unassigned simonbasle Jan 19, 2023
@simonbasle
Copy link
Contributor

As said above, this is likely to be an issue with the server and a blanket handling of EOFException in the IntrospectingClientHttpResponse is potentially going to hide further issues.

Unfortunately, at least with the RestTemplate the OutputStream comes from the underlying client and is typically NOT a GZipOutputStream directly (so we can't apply the patch selectively).

Here the GZipInputStream is trying to tell us that the bytes don't actually represent gzipped content. An empty content should result in 20 bytes of gzip headers once "compressed" not 0 bytes, as @bclozel said above.

The EOFException seems to occur if there are 0 bytes but also if the 20 zippedBytes are truncated (truncatedBytes and zeroBytes examples above).
If the first bytes are removed like in shiftedBytes, GZipInputStream won't see a proper magic number and will throw a ZipException("Not in GZIP format") instead.

The GZipInputStream javadoc for read states:

public int read()
throws java.io.IOException

Reads a byte of uncompressed data. This method will block until enough input is available for decompression.

Returns:
the byte read, or -1 if end of compressed input is reached

Throws:
IOException – if an I/O error has occurred

This is all indicative of a bad response from that particular server, from what we can gather from the mock response you've show in your reproducer code. We'd still be interested in a (redacted if needed) dump of the actual response if possible, but the issue certainly doesn't lie with Spring Framework.

Please note a few additional interesting things regarding your tests with several backing clients:

In your test the JDK case is green because it happily consumes 0 bytes (since it doesn't attempt to decompress). If you feed it actual gzip content and turn it into a ResponseEntity<String>, you'll get compressed garbage String.

As a result, these "work" when the actual body is byte[0], because they don't attempt to decompress.
If you flip the case around, returning the equivalent of zippedBytes from my example in the mockServer, then you'll see that all is green except these two. Chunked vs connection close doesn't make a difference either.

I've rewritten the tests (using a @ParameterizedTest to cover combinations easily) and this is what I get:
image

(see https://gist.github.com/simonbasle/f7880ded86857d71ee68bbe10cf2edce)

@simonbasle simonbasle added status: invalid An issue that we don't feel is valid for: external-project Needs a fix in external project and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jan 20, 2023
@simonbasle simonbasle removed this from the Triage Queue milestone Jan 20, 2023
@simonbasle
Copy link
Contributor

Conclusion

If you removed RestTemplate from the equation, you would get the same issue.

You'll need to find a way to configure your underlying client to work around the issue, e.g. by deactivating automatic gzip support and adding some sort of interceptor to decompress the bytes yourself, provisioning for an empty byte array for that specific remote server.

Another option is to query the server WITHOUT Accept-Encoding: gzip in the request (no gzipping involved).

@simonbasle simonbasle closed this Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project Needs a fix in external project in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants