Check if entity's stream is really compressed.

[ecdpalma]

(I know this use case is absolutely uncommon, but maybe someone may need the same thing.)

One of our clients has a server returning 404 errors with
Content-Encoding=gzip, but the content itself is not compressed, causing
'new GZIPInputStream' in InflatingEntity#getContent to throw an
IOException.

When using FileAsyncHttpResponseHandler, for some reason we couldn't
find out, after some requests with the wrong 404 response,
AsyncHttpClient was getting stuck and after some time it raised
ConnectionPoolTimeoutException. This wasn't happening when the
BaseJsonHttpResponseHandler was being used.

[fineswap]

@ecdpalma

I am bewildered as to why we need the library to support non-standard servers? This rests on the server's administrator shoulders, not the clients. If the server sends a content type (willingly) indicating a compressed content, then the content must be compressed. If the server wants to send uncompressed content, the content type must reflect that.

I'm afraid that this is a special case; you'll have to extend AsyncHttpClient class and implement it for your client.

[smarek]

I'd be willing to merge that, because it'd be better to have it working with warning (which is the only thing I'm missing) than to have them grope in the dark (I'm talking about the way this problem expresses itself).

So the question is, what will this change do, will it slow down the processing of each request? If so how much? @ecdpalma thanks for the pull request, and please, do me a favor and put this through some stress testing, and let us know how large performance change will occur, because change in such place shall not be underestimated.

@fineswap
We know this is un-common situation, we have few options:

1. Merge it as-is, but I'd like to add warning log output, saying "Server stated compressed response, but it is being send uncompressed"
2. Add it as optional fix, where you could either enable or disable it via AsyncHttpClient interface (such as disableStrictCompressedResponseCheck)

[fineswap]

Well, if there's a toggle to enable/disable this functionality (I should have thought of that myself) then it's a welcome addition.

However, I believe that when the toggle is in place, then other aspects of the library where compression is being utilized should also benefit from this (rather than consume the content as being compressed).

With this being said, maybe it'd be best to create a standalone method, maybe a static function even, to test if an InputStream is compressed or not. A utility function, much like silentCloseInputStream().

[smarek]

@ecdpalma again, thanks for suggestion, I've just included it in modified version: 25023d8

@fineswap review please?

[fineswap]

@smarek
I'll review it and fix as necessary. Good job guys!

[ecdpalma]

Sorry for the delay on answering, guys. It's been a tough week.

I agree completely with @fineswap's first comment. I have a very particular case for this, and if it were under my control, I would solve it in the server.

On the patch performance: I couldn't find any problem affecting user experience.

One doubt that remains is: why was the HttpClient getting stuck when using the FileAsyncHttpResponseHandler and not with BaseJsonHttpResponseHandler (please see the last paragraph in my first comment in this PR)?