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

fix: Correctly handling chunked response streams with gzip #990

merged 2 commits into from Mar 11, 2020


Copy link

@hiranya911 hiranya911 commented Mar 6, 2020

Fixes #367 (see my last comment on that issue)

The fix done in #840 is incorrect if not inadequate. It doesn't properly cleanup the last chunk of a chunked response when the response is also gzipped. This in turns prevents proper connection reuse.

This PR addresses this issue.

@hiranya911 hiranya911 requested a review from as a code owner Mar 6, 2020
@googlebot googlebot added the cla: yes label Mar 6, 2020
@elharo elharo requested a review from codyoss Mar 6, 2020
Copy link

@elharo elharo commented Mar 6, 2020

@codyoss Can you take a look at this and see what you think? Thanks.

codyoss approved these changes Mar 6, 2020
Copy link

@codyoss codyoss left a comment

LGTM, looks like the test did not adequately cover the case where the buffer size of GzipInputStream was exceeded.

Copy link
Contributor Author

@hiranya911 hiranya911 commented Mar 10, 2020

Hi Folks. Please let me know I need to do anything before this can be merged.

@codyoss codyoss merged commit 1ba2197 into googleapis:master Mar 11, 2020
9 checks passed
gcf-merge-on-green bot pushed a commit that referenced this issue Apr 27, 2020
🤖 I have created a release \*beep\* \*boop\* 
## [1.35.0]( (2020-04-27)

### Features

* add logic for verifying ES256 JsonWebSignatures ([#1033]( ([bb4227f](

### Bug Fixes

* add linkage monitor plugin ([#1000]( ([027c227](
* Correctly handling chunked response streams with gzip ([#990]( ([1ba2197](, closes [#367](
* FileDataStoreFactory will throw IOException for any permissions errors ([#1012]( ([fd33073](
* include request method and URL into HttpResponseException message ([#1002]( ([15111a1](
* incorrect check for Windows OS in FileDataStoreFactory ([#927]( ([8b4eabe](
* reuse reference instead of calling getter twice ([#983]( ([1f66222](, closes [#982](
* **android:** set minimum API level to 19 a.k.a. 4.4 Kit Kat ([#1016]( ([b9a8023](, closes [#1015](

### Documentation

* android 4.4 or later is required ([#1008]( ([bcc41dd](
* libraries-bom 4.0.1 ([#976]( ([fc21dc4](
* libraries-bom 4.1.1 ([#984]( ([635c813](
* libraries-bom 5.2.0 ([#1032]( ([ca34202](
* require Android 4.4 ([#1007]( ([f9d2bb0](

### Dependencies

* httpclient 4.5.12 ([#991]( ([79bc1c7](
* update to Guava 29 ([#1024]( ([ca9520f](

This PR was generated with [Release Please](
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
cla: yes
None yet

Successfully merging this pull request may close these issues.

4 participants