Patch NetHttpResponse to detect abrupt termination

  JDK's HttpUrlConnection implementation fails to notify clients when the
  connection is lost when reading a response, e.g., the server gets restarted.
  This changelist patches the NetHttpResponse to detect abrupt connection
  termination when a fixed content length is available. In the case where
  responses are chunk encoded, connection lost is well handled by JDK.

  Also extend MockHttpUrlConnection for more testing functionalities.

google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpResponse.java

@@ -16,6 +16,7 @@
 
 import com.google.api.client.http.LowLevelHttpResponse;
 
+import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
@@ -70,14 +71,27 @@ public int getStatusCode() {
    * with version 1.17 it returns {@link HttpURLConnection#getInputStream} when it doesn't throw
    * {@link IOException}, otherwise it returns {@link HttpURLConnection#getErrorStream}
    * </p>
+   *
+   * <p>
+   * Upgrade warning: in versions prior to 1.20 {@link #getContent()} returned
+   * {@link HttpURLConnection#getInputStream()} or {@link HttpURLConnection#getErrorStream()}, both
+   * of which silently returned -1 for read() calls when the connection got closed in the middle
+   * of receiving a response. This is highly likely a bug from JDK's {@link HttpURLConnection}.
+   * Since version 1.20, the bytes read off the wire will be checked and an {@link IOException} will
+   * be thrown if the response is not fully delivered when the connection is closed by server for
+   * whatever reason, e.g., server restarts. Note though that this is a best-effort check: when the
+   * response is chunk encoded, we have to rely on the underlying HTTP library to behave correctly.
+   * </p>
    */
   @Override
   public InputStream getContent() throws IOException {
+    InputStream in = null;
     try {
-      return connection.getInputStream();
+      in = connection.getInputStream();
     } catch (IOException ioe) {
-      return connection.getErrorStream();
+      in = connection.getErrorStream();
     }
+    return in == null ? null : new SizeValidatingInputStream(in);
   }
 
   @Override
@@ -131,4 +145,63 @@ public String getHeaderValue(int index) {
   public void disconnect() {
     connection.disconnect();
   }
+
+  /**
+   * A wrapper arround the base {@link InputStream} that validates EOF returned by the read calls.
+   *
+   * @since 1.20
+   */
+  private final class SizeValidatingInputStream extends FilterInputStream {
+
+    private long bytesRead = 0;
+
+    public SizeValidatingInputStream(InputStream in) {
+      super(in);
+    }
+
+    /**
+     * java.io.InputStream#read(byte[], int, int) swallows IOException thrown from read() so we have
+     * to override it.
+     * @see "http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8-b132/java/io/InputStream.java#185"
+     */
+    @Override
+    public int read(byte[] b, int off, int len) throws IOException {
+      int n = in.read(b, off, len);
+      if (n == -1) {
+        throwIfFalseEOF();
+      } else {
+        bytesRead += n;
+      }
+      return n;
+    }
+
+    @Override
+    public int read() throws IOException {
+      int n = in.read();
+      if (n == -1) {
+        throwIfFalseEOF();
+      } else {
+        bytesRead++;
+      }
+      return n;
+    }
+
+    // Throws an IOException if gets an EOF in the middle of a response.
+    private void throwIfFalseEOF() throws IOException {
+      long contentLength = getContentLength();
+      if (contentLength == -1) {
+        // If a Content-Length header is missing, there's nothing we can do.
+        return;
+      }
+      // According to RFC2616, message-body is prohibited in responses to certain requests, e.g.,
+      // HEAD. Nevertheless an entity-header (possibly with non-zero Content-Length) may be present.
+      // Thus we exclude the case where bytesRead == 0.
+      //
+      // See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4 for details.
+      if (bytesRead != 0 && bytesRead < contentLength) {
+        throw new IOException("Connection closed prematurely: bytesRead = " + bytesRead
+            + ", Content-Length = " + contentLength);
+      }
+    }
+  }
 }

google-http-client/src/main/java/com/google/api/client/testing/http/javanet/MockHttpURLConnection.java

@@ -17,14 +17,17 @@
 import com.google.api.client.util.Beta;
 import com.google.api.client.util.Preconditions;
 
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.UnknownServiceException;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 
 /**
  * {@link Beta} <br/>
@@ -52,20 +55,28 @@ public class MockHttpURLConnection extends HttpURLConnection {
   /**
    * The input byte array which represents the content when the status code is less then
    * {@code 400}.
+   *
+   * @deprecated As of 1.20. Use {@link #setInputStream(InputStream)} instead.
    */
+  @Deprecated
   public static final byte[] INPUT_BUF = new byte[1];
 
   /**
    * The error byte array which represents the content when the status code is greater or equal to
    * {@code 400}.
+   *
+   * @deprecated As of 1.20. Use {@link #setErrorStream(InputStream)} instead.
    */
+  @Deprecated
   public static final byte[] ERROR_BUF = new byte[5];
 
   /** The input stream. */
-  private InputStream inputStream = new ByteArrayInputStream(INPUT_BUF);
+  private InputStream inputStream = null;
 
   /** The error stream. */
-  private InputStream errorStream = new ByteArrayInputStream(ERROR_BUF);
+  private InputStream errorStream = null;
+
+  private Map<String, List<String>> headers = new LinkedHashMap<String, List<String>>();
 
   /**
    * @param u the URL or {@code null} for none
@@ -130,6 +141,54 @@ public MockHttpURLConnection setResponseCode(int responseCode) {
     return this;
   }
 
+  /**
+   * Sets a custom response header.
+   *
+   * @since 1.20
+   */
+  public MockHttpURLConnection addHeader(String name, String value) {
+    Preconditions.checkNotNull(name);
+    Preconditions.checkNotNull(value);
+    if (headers.containsKey(name)) {
+      headers.get(name).add(value);
+    } else {
+      List<String> values = new ArrayList<String>();
+      values.add(value);
+      headers.put(name, values);
+    }
+    return this;
+  }
+
+  /**
+   * Sets the input stream.
+   *
+   * <p>To prevent incidental overwrite, only the first non-null assignment is honored.
+   *
+   * @since 1.20
+   */
+  public MockHttpURLConnection setInputStream(InputStream is) {
+    Preconditions.checkNotNull(is);
+    if (inputStream == null) {
+      inputStream = is;
+    }
+    return this;
+  }
+
+  /**
+   * Sets the error stream.
+   *
+   * <p>To prevent incidental overwrite, only the first non-null assignment is honored.
+   *
+   * @since 1.20
+   */
+  public MockHttpURLConnection setErrorStream(InputStream is) {
+    Preconditions.checkNotNull(is);
+    if (errorStream == null) {
+      errorStream = is;
+    }
+    return this;
+  }
+
   @Override
   public InputStream getInputStream() throws IOException {
     if (responseCode < 400) {
@@ -142,4 +201,15 @@ public InputStream getInputStream() throws IOException {
   public InputStream getErrorStream() {
     return errorStream;
   }
+
+  @Override
+  public Map<String, List<String>> getHeaderFields() {
+    return headers;
+  }
+
+  @Override
+  public String getHeaderField(String name) {
+    List<String> values = headers.get(name);
+    return values == null ? null : values.get(0);
+  }
 }

google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpResponseTest.java

@@ -15,10 +15,14 @@
 package com.google.api.client.http.javanet;
 
 import com.google.api.client.testing.http.javanet.MockHttpURLConnection;
+import com.google.api.client.util.StringUtils;
 
 import junit.framework.TestCase;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
 
 /**
  * Tests {@link NetHttpResponse}.
@@ -27,6 +31,9 @@
  */
 public class NetHttpResponseTest extends TestCase {
 
+  private static final String VALID_RESPONSE = "This is a valid response.";
+  private static final String ERROR_RESPONSE = "This is an error response.";
+
   public void testGetStatusCode() throws IOException {
     subtestGetStatusCode(0, -1);
     subtestGetStatusCode(200, 200);
@@ -45,16 +52,48 @@ public void testGetContent() throws IOException {
     subtestGetContent(307);
     subtestGetContent(404);
     subtestGetContent(503);
+
+    subtestGetContentWithShortRead(0);
+    subtestGetContentWithShortRead(200);
+    subtestGetContentWithShortRead(304);
+    subtestGetContentWithShortRead(307);
+    subtestGetContentWithShortRead(404);
+    subtestGetContentWithShortRead(503);
   }
 
   public void subtestGetContent(int responseCode) throws IOException {
     NetHttpResponse response =
-        new NetHttpResponse(new MockHttpURLConnection(null).setResponseCode(responseCode));
-    int bytes = response.getContent().read(new byte[100]);
+        new NetHttpResponse(new MockHttpURLConnection(null).setResponseCode(responseCode)
+            .setInputStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(VALID_RESPONSE)))
+            .setErrorStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(ERROR_RESPONSE))));
+    InputStream is = response.getContent();
+    byte[] buf = new byte[100];
+    int bytes = 0, n = 0;
+    while ((n = is.read(buf)) != -1) {
+      bytes += n;
+    }
+    if (responseCode < 400) {
+      assertEquals(VALID_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
+    } else {
+      assertEquals(ERROR_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
+    }
+  }
+
+  public void subtestGetContentWithShortRead(int responseCode) throws IOException {
+    NetHttpResponse response =
+        new NetHttpResponse(new MockHttpURLConnection(null).setResponseCode(responseCode)
+            .setInputStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(VALID_RESPONSE)))
+            .setErrorStream(new ByteArrayInputStream(StringUtils.getBytesUtf8(ERROR_RESPONSE))));
+    InputStream is = response.getContent();
+    byte[] buf = new byte[100];
+    int bytes = 0, b = 0;
+    while ((b = is.read()) != -1) {
+      buf[bytes++] = (byte) b;
+    }
     if (responseCode < 400) {
-      assertEquals(MockHttpURLConnection.INPUT_BUF.length, bytes);
+      assertEquals(VALID_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
     } else {
-      assertEquals(MockHttpURLConnection.ERROR_BUF.length, bytes);
+      assertEquals(ERROR_RESPONSE, new String(buf, 0, bytes, Charset.forName("UTF-8")));
     }
   }
 }

google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpTransportTest.java

@@ -21,7 +21,9 @@
 
 import junit.framework.TestCase;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
 
@@ -59,6 +61,7 @@ public void testExecute_mock() throws Exception {
     }
   }
 
+
   public void testExecute_methodUnchanged() throws Exception {
     for (String method : METHODS) {
       HttpURLConnection connection =
@@ -75,6 +78,58 @@ public void testExecute_methodUnchanged() throws Exception {
     }
   }
 
+  public void testAbruptTerminationIsNoticedWithContentLength() throws Exception {
+    String incompleteBody = ""
+        + "Fixed size body test.\r\n"
+        + "Incomplete response.";
+    byte[] buf = StringUtils.getBytesUtf8(incompleteBody);
+    MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL))
+        .setResponseCode(200)
+        .addHeader("Content-Length", "205")
+        .setInputStream(new ByteArrayInputStream(buf));
+    connection.setRequestMethod("GET");
+    NetHttpRequest request = new NetHttpRequest(connection);
+    setContent(request, null, "");
+    NetHttpResponse response = (NetHttpResponse) (request.execute());
+
+    InputStream in = response.getContent();
+    boolean thrown = false;
+    try {
+      while (in.read() != -1) {
+        // This space is intentionally left blank.
+      }
+    } catch (IOException ioe) {
+      thrown = true;
+    }
+    assertTrue(thrown);
+  }
+
+  public void testAbruptTerminationIsNoticedWithContentLengthWithReadToBuf() throws Exception {
+    String incompleteBody = ""
+        + "Fixed size body test.\r\n"
+        + "Incomplete response.";
+    byte[] buf = StringUtils.getBytesUtf8(incompleteBody);
+    MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL))
+        .setResponseCode(200)
+        .addHeader("Content-Length", "205")
+        .setInputStream(new ByteArrayInputStream(buf));
+    connection.setRequestMethod("GET");
+    NetHttpRequest request = new NetHttpRequest(connection);
+    setContent(request, null, "");
+    NetHttpResponse response = (NetHttpResponse) (request.execute());
+
+    InputStream in = response.getContent();
+    boolean thrown = false;
+    try {
+      while (in.read(new byte[100]) != -1) {
+        // This space is intentionally left blank.
+      }
+    } catch (IOException ioe) {
+      thrown = true;
+    }
+    assertTrue(thrown);
+  }
+
   private void setContent(NetHttpRequest request, String type, String value) throws Exception {
     byte[] bytes = StringUtils.getBytesUtf8(value);
     request.setStreamingContent(new ByteArrayStreamingContent(bytes));