From 1ba219743e65c89bc3fdb196acc5d2042e01f542 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 11 Mar 2020 07:24:42 -0700 Subject: [PATCH] fix: Correctly handling chunked response streams with gzip (#990) Previous implementation doesn't properly cleanup the last chunk of a chunked response when the response is also gzipped. This in turns prevents proper connection reuse. Fixes #367 --- .../java/com/google/api/client/http/HttpResponse.java | 7 ++++++- .../java/com/google/api/client/http/HttpResponseTest.java | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java index 3341720a6..276ce0e74 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java @@ -353,8 +353,13 @@ public InputStream getContent() throws IOException { if (!returnRawInputStream && this.contentEncoding != null) { String oontentencoding = this.contentEncoding.trim().toLowerCase(Locale.ENGLISH); if (CONTENT_ENCODING_GZIP.equals(oontentencoding) || CONTENT_ENCODING_XGZIP.equals(oontentencoding)) { + // Wrap the original stream in a ConsumingInputStream before passing it to + // GZIPInputStream. The GZIPInputStream leaves content unconsumed in the original + // stream (it almost always leaves the last chunk unconsumed in chunked responses). + // ConsumingInputStream ensures that any unconsumed bytes are read at close. + // GZIPInputStream.close() --> ConsumingInputStream.close() --> exhaust(ConsumingInputStream) lowLevelResponseContent = - new ConsumingInputStream(new GZIPInputStream(lowLevelResponseContent)); + new GZIPInputStream(new ConsumingInputStream(lowLevelResponseContent)); } } // logging (wrap content with LoggingInputStream) diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java index f7638d675..57b400e40 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java @@ -567,6 +567,12 @@ private void do_testGetContent_gzipEncoding_finishReading(String contentEncoding ) { zipStream.write(dataToCompress); zipStream.close(); + + // GZIPInputStream uses a default buffer of 512B. Add enough content to exceed this + // limit, so that some content will be left in the connection. + for (int i = 0; i < 1024; i++) { + byteStream.write('7'); + } mockBytes = byteStream.toByteArray(); } final MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse(); @@ -594,6 +600,8 @@ public LowLevelHttpResponse execute() throws IOException { assertFalse(output.isClosed()); assertEquals("abcd", response.parseAsString()); assertTrue(output.isClosed()); + // The underlying stream should be fully consumed, even if gzip only returns some of it. + assertEquals(-1, output.read()); } }