-
Notifications
You must be signed in to change notification settings - Fork 256
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
Response decompression in Apache Http Client 5 #1112
Comments
@aleksey-petrov-github Thank you very much for your detailed investigation! 📣 I'd be in favor of option 1, just because it wouldn't affect api and core modules. |
@whiskeysierra |
In order to prioritize the support for Logbook, we would like to check whether the old issues are still relevant.
|
Hi 🙂 I am currently using the latest Logbook and HttpClient5. I noticed that the fix from above works well but has one critical This is important downside that must be at least documented in README, in my opinion. The usual way of building an HTTP client instance is to use HttpClients.custom()
.setDefaultHeaders(List.of(new BasicHeader("name", "value")))
.setUserAgent("asd")
.setConnectionManager(...)
... And in this case, neither user agent, nor default headers are logged by Logbook because of exactly that. |
There is known issue (#135) : The Logbook HTTP Client integration is handling gzip-compressed response entities incorrectly if the interceptor runs before a decompressing interceptor. Since logging compressed contents is not really helpful it's advised to register the logbook interceptor as the last interceptor in the chain.
This advice is not suitable for Apache Http Client 5, because ResponseContentEncoding interceptor now is implemented as ContentCompressionExec handler which is called after interceptors.
So the logged content is compressed and also body filters are broken because they test compressed content.
For my project workaround is to implement strategy, that wraps response with decompressing logic:
It might be not suitable for any reason for others. And also it is not optimal because there is no information about response that helps to define that decompression is needed inside strategy - no access to body length and content encoding header at this step, so decision is made in DecompressingHttpResponse. But strategy is the only place to wrap before FilteredHttpResponse.
In my opinion there are 2 ways to handle decompression:
Inline old ResponseContentEncoding interceptor and explicitly include it in common chain for Http Client 5 (like it done for OkHttp* #386) seems not proper way also.
Interesting to know your opinion first before any pull requests.
The text was updated successfully, but these errors were encountered: