From 087a428390a334bd761a8a3d66475aa4dde72ed1 Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Fri, 18 Oct 2019 11:36:11 -0600 Subject: [PATCH] feat: wrap GZIPInputStream for connection reuse (#840) If a connection is closed and there are some bytes that have not been read that connection can't be reused. Now GZIPInputStream will have all of its bytes read on close automatically to promote connection reuse. Cherry-picked: #749 Fixes: #367 --- .../api/client/http/ConsumingInputStream.java | 47 ++++++++++++++ .../google/api/client/http/HttpResponse.java | 3 +- .../client/http/ConsumingInputStreamTest.java | 65 +++++++++++++++++++ .../api/client/http/HttpResponseTest.java | 38 +++++++++++ 4 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java create mode 100644 google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java diff --git a/google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java b/google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java new file mode 100644 index 000000000..f0170a61b --- /dev/null +++ b/google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java @@ -0,0 +1,47 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.api.client.http; + +import com.google.common.io.ByteStreams; +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; + +/** + * This class in meant to wrap an {@link InputStream} so that all bytes in the steam are read and + * discarded on {@link InputStream#close()}. This ensures that the underlying connection has the + * option to be reused. + */ +final class ConsumingInputStream extends FilterInputStream { + private boolean closed = false; + + ConsumingInputStream(InputStream inputStream) { + super(inputStream); + } + + @Override + public void close() throws IOException { + if (!closed && in != null) { + try { + ByteStreams.exhaust(this); + super.in.close(); + } finally { + this.closed = true; + } + } + } +} 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 90c3812f0..d972ab0d4 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 @@ -331,7 +331,8 @@ public InputStream getContent() throws IOException { if (!returnRawInputStream && contentEncoding != null && contentEncoding.contains("gzip")) { - lowLevelResponseContent = new GZIPInputStream(lowLevelResponseContent); + lowLevelResponseContent = + new ConsumingInputStream(new GZIPInputStream(lowLevelResponseContent)); } // logging (wrap content with LoggingInputStream) Logger logger = HttpTransport.LOGGER; diff --git a/google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java b/google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java new file mode 100644 index 000000000..d55b5a0d4 --- /dev/null +++ b/google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java @@ -0,0 +1,65 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.api.client.http; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import org.junit.Test; + +public class ConsumingInputStreamTest { + + @Test + public void testClose_drainsBytesOnClose() throws IOException { + MockInputStream mockInputStream = new MockInputStream("abc123".getBytes(StandardCharsets.UTF_8)); + InputStream consumingInputStream = new ConsumingInputStream(mockInputStream); + + assertEquals(6, mockInputStream.getBytesToRead()); + + // read one byte + consumingInputStream.read(); + assertEquals(5, mockInputStream.getBytesToRead()); + + // closing the stream should read the remaining bytes + consumingInputStream.close(); + assertEquals(0, mockInputStream.getBytesToRead()); + } + + private class MockInputStream extends InputStream { + private int bytesToRead; + + MockInputStream(byte[] data) { + this.bytesToRead = data.length; + } + + @Override + public int read() throws IOException { + if (bytesToRead == 0) { + return -1; + } + bytesToRead--; + return 1; + } + + int getBytesToRead() { + return bytesToRead; + } + } +} 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 7846778e5..a611d774a 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 @@ -26,10 +26,12 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.lang.reflect.Type; +import java.nio.charset.StandardCharsets; import java.text.NumberFormat; import java.util.Arrays; import java.util.logging.Level; import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; import junit.framework.TestCase; /** @@ -457,4 +459,40 @@ public LowLevelHttpResponse execute() throws IOException { "it should not decompress stream", request.execute().getContent() instanceof GZIPInputStream); } + + public void testGetContent_gzipEncoding_finishReading() throws IOException { + byte[] dataToCompress = "abcd".getBytes(StandardCharsets.UTF_8); + byte[] mockBytes; + try (ByteArrayOutputStream byteStream = new ByteArrayOutputStream(dataToCompress.length)) { + GZIPOutputStream zipStream = new GZIPOutputStream((byteStream)); + zipStream.write(dataToCompress); + zipStream.close(); + mockBytes = byteStream.toByteArray(); + } + final MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse(); + mockResponse.setContent(mockBytes); + mockResponse.setContentEncoding("gzip"); + mockResponse.setContentType("text/plain"); + + HttpTransport transport = + new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, final String url) + throws IOException { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + return mockResponse; + } + }; + } + }; + HttpRequest request = + transport.createRequestFactory().buildHeadRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpResponse response = request.execute(); + TestableByteArrayInputStream output = (TestableByteArrayInputStream) mockResponse.getContent(); + assertFalse(output.isClosed()); + assertEquals("abcd", response.parseAsString()); + assertTrue(output.isClosed()); + } }