diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index 4b9d9b8d7..a0349ad8d 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -35,6 +35,8 @@ import java.io.OutputStream; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.apache.http.Header; @@ -213,11 +215,32 @@ public void testConnectTimeout() { } } + static class FakeServer implements AutoCloseable { + private final HttpServer server; + private final ExecutorService executorService; + + public FakeServer(HttpHandler httpHandler) throws IOException { + this.server = HttpServer.create(new InetSocketAddress(0), 0); + this.executorService = Executors.newFixedThreadPool(1); + server.setExecutor(this.executorService); + server.createContext("/", httpHandler); + server.start(); + } + + public int getPort() { + return server.getAddress().getPort(); + } + + @Override + public void close() { + this.server.stop(0); + this.executorService.shutdownNow(); + } + } + @Test public void testNormalizedUrl() throws IOException { - HttpServer server = HttpServer.create(new InetSocketAddress(0), 0); - server.createContext( - "/", + final HttpHandler handler = new HttpHandler() { @Override public void handle(HttpExchange httpExchange) throws IOException { @@ -227,19 +250,53 @@ public void handle(HttpExchange httpExchange) throws IOException { out.write(response); } } - }); - server.start(); - - ApacheHttpTransport transport = new ApacheHttpTransport(); - GenericUrl testUrl = new GenericUrl("http://localhost/foo//bar"); - testUrl.setPort(server.getAddress().getPort()); - com.google.api.client.http.HttpResponse response = - transport.createRequestFactory().buildGetRequest(testUrl).execute(); - assertEquals(200, response.getStatusCode()); - assertEquals("/foo//bar", response.parseAsString()); + }; + try (FakeServer server = new FakeServer(handler)) { + HttpTransport transport = new ApacheHttpTransport(); + GenericUrl testUrl = new GenericUrl("http://localhost/foo//bar"); + testUrl.setPort(server.getPort()); + com.google.api.client.http.HttpResponse response = + transport.createRequestFactory().buildGetRequest(testUrl).execute(); + assertEquals(200, response.getStatusCode()); + assertEquals("/foo//bar", response.parseAsString()); + } } private boolean isWindows() { return System.getProperty("os.name").startsWith("Windows"); } + + @Test(timeout = 10_000L) + public void testDisconnectShouldNotWaitToReadResponse() throws IOException { + // This handler waits for 100s before returning writing content. The test should + // timeout if disconnect waits for the response before closing the connection. + final HttpHandler handler = + new HttpHandler() { + @Override + public void handle(HttpExchange httpExchange) throws IOException { + byte[] response = httpExchange.getRequestURI().toString().getBytes(); + httpExchange.sendResponseHeaders(200, response.length); + + // Sleep for longer than the test timeout + try { + Thread.sleep(100_000); + } catch (InterruptedException e) { + throw new IOException("interrupted", e); + } + try (OutputStream out = httpExchange.getResponseBody()) { + out.write(response); + } + } + }; + + try (FakeServer server = new FakeServer(handler)) { + HttpTransport transport = new ApacheHttpTransport(); + GenericUrl testUrl = new GenericUrl("http://localhost/foo//bar"); + testUrl.setPort(server.getPort()); + com.google.api.client.http.HttpResponse response = + transport.createRequestFactory().buildGetRequest(testUrl).execute(); + // disconnect should not wait to read the entire content + response.disconnect(); + } + } } 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 37f4d7f11..68d8850c8 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 @@ -351,9 +351,9 @@ public InputStream getContent() throws IOException { try { // gzip encoding (wrap content with GZipInputStream) if (!returnRawInputStream && this.contentEncoding != null) { - String oontentencoding = this.contentEncoding.trim().toLowerCase(Locale.ENGLISH); - if (CONTENT_ENCODING_GZIP.equals(oontentencoding) - || CONTENT_ENCODING_XGZIP.equals(oontentencoding)) { + String contentEncoding = this.contentEncoding.trim().toLowerCase(Locale.ENGLISH); + if (CONTENT_ENCODING_GZIP.equals(contentEncoding) + || CONTENT_ENCODING_XGZIP.equals(contentEncoding)) { // 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). @@ -419,9 +419,12 @@ public void download(OutputStream outputStream) throws IOException { /** Closes the content of the HTTP response from {@link #getContent()}, ignoring any content. */ public void ignore() throws IOException { - InputStream content = getContent(); - if (content != null) { - content.close(); + if (this.response == null) { + return; + } + InputStream lowLevelResponseContent = this.response.getContent(); + if (lowLevelResponseContent != null) { + lowLevelResponseContent.close(); } } @@ -432,8 +435,10 @@ public void ignore() throws IOException { * @since 1.4 */ public void disconnect() throws IOException { - ignore(); + // Close the connection before trying to close the InputStream content. If you are trying to + // disconnect, we shouldn't need to try to read any further content. response.disconnect(); + ignore(); } /** diff --git a/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java b/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java index 338236e9b..835730793 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java @@ -14,17 +14,27 @@ package com.google.api.client.http.javanet; +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpTransport; import com.google.api.client.testing.http.HttpTesting; import com.google.api.client.testing.http.javanet.MockHttpURLConnection; import com.google.api.client.util.ByteArrayStreamingContent; import com.google.api.client.util.StringUtils; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.net.HttpURLConnection; +import java.net.InetSocketAddress; import java.net.URL; import java.security.KeyStore; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import junit.framework.TestCase; +import org.junit.Test; /** * Tests {@link NetHttpTransport}. @@ -159,4 +169,61 @@ private void setContent(NetHttpRequest request, String type, String value) throw request.setContentType(type); request.setContentLength(bytes.length); } + + static class FakeServer implements AutoCloseable { + private final HttpServer server; + private final ExecutorService executorService; + + public FakeServer(HttpHandler httpHandler) throws IOException { + this.server = HttpServer.create(new InetSocketAddress(0), 0); + this.executorService = Executors.newFixedThreadPool(1); + server.setExecutor(this.executorService); + server.createContext("/", httpHandler); + server.start(); + } + + public int getPort() { + return server.getAddress().getPort(); + } + + @Override + public void close() { + this.server.stop(0); + this.executorService.shutdownNow(); + } + } + + @Test(timeout = 10_000L) + public void testDisconnectShouldNotWaitToReadResponse() throws IOException { + // This handler waits for 100s before returning writing content. The test should + // timeout if disconnect waits for the response before closing the connection. + final HttpHandler handler = + new HttpHandler() { + @Override + public void handle(HttpExchange httpExchange) throws IOException { + byte[] response = httpExchange.getRequestURI().toString().getBytes(); + httpExchange.sendResponseHeaders(200, response.length); + + // Sleep for longer than the test timeout + try { + Thread.sleep(100_000); + } catch (InterruptedException e) { + throw new IOException("interrupted", e); + } + try (OutputStream out = httpExchange.getResponseBody()) { + out.write(response); + } + } + }; + + try (FakeServer server = new FakeServer(handler)) { + HttpTransport transport = new NetHttpTransport(); + GenericUrl testUrl = new GenericUrl("http://localhost/foo//bar"); + testUrl.setPort(server.getPort()); + com.google.api.client.http.HttpResponse response = + transport.createRequestFactory().buildGetRequest(testUrl).execute(); + // disconnect should not wait to read the entire content + response.disconnect(); + } + } }