Changes to support http://codereview.appspot.com/5450097/

Author: rmistry

google-http-client/src/main/java/com/google/api/client/http/AbstractHttpContent.java

@@ -63,7 +63,11 @@ protected long computeLength() throws IOException {
       return -1;
     }
     ByteCountingOutputStream countingStream = new ByteCountingOutputStream();
-    writeTo(countingStream);
+    try {
+      writeTo(countingStream);
+    } finally {
+      countingStream.close();
+    }
     return countingStream.count;
   }
 

google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java

@@ -43,6 +43,12 @@ public abstract class AbstractInputStreamContent implements HttpContent {
   /** Content encoding (for example {@code "gzip"}) or {@code null} for none. */
   private String encoding;
 
+  /**
+   * Whether the input stream should be closed at the end of {@link #writeTo}. Default is
+   * {@code true}.
+   */
+  private boolean closeInputStream = true;
+
   /**
    * @param type Content type or {@code null} for none
    * @since 1.5
@@ -56,14 +62,20 @@ public AbstractInputStreamContent(String type) {
    * {@link AbstractInputStreamContent}. If the specific implementation will return {@code true} for
    * {@link #retrySupported()} this should be a factory function which will create a new
    * {@link InputStream} from the source data whenever invoked.
+   *
+   * <p>
+   * Upgrade warning: in prior version 1.6 {@link #getInputStream} was protected, it is now public.
+   * </p>
+   *
+   * @since 1.7
    */
-  protected abstract InputStream getInputStream() throws IOException;
+  public abstract InputStream getInputStream() throws IOException;
 
   public void writeTo(OutputStream out) throws IOException {
     InputStream inputStream = getInputStream();
     long contentLength = getLength();
     if (contentLength < 0) {
-      copy(inputStream, out);
+      copy(inputStream, out, closeInputStream);
     } else {
       byte[] buffer = new byte[BUFFER_SIZE];
       try {
@@ -78,9 +90,12 @@ public void writeTo(OutputStream out) throws IOException {
           remaining -= read;
         }
       } finally {
-        inputStream.close();
+        if (closeInputStream) {
+          inputStream.close();
+        }
       }
     }
+    out.flush();
   }
 
   public String getEncoding() {
@@ -92,7 +107,18 @@ public String getType() {
   }
 
   /**
-   * Sets the content encoding (for example {@code "gzip"}) or {@code null} for none.
+   * Returns whether the input stream should be closed at the end of {@link #writeTo}. Default is
+   * {@code true}.
+   *
+   * @since 1.7
+   */
+  public final boolean getCloseInputStream() {
+    return closeInputStream;
+  }
+
+  /**
+   * Sets the content encoding (for example {@code "gzip"}) or {@code null} for none. Subclasses
+   * should override by calling super.
    *
    * @since 1.5
    */
@@ -102,7 +128,7 @@ public AbstractInputStreamContent setEncoding(String encoding) {
   }
 
   /**
-   * Sets the content type or {@code null} for none.
+   * Sets the content type or {@code null} for none. Subclasses should override by calling super.
    *
    * @since 1.5
    */
@@ -111,12 +137,25 @@ public AbstractInputStreamContent setType(String type) {
     return this;
   }
 
+  /**
+   * Sets whether the input stream should be closed at the end of {@link #writeTo}. Default is
+   * {@code true}. Subclasses should override by calling super.
+   *
+   * @since 1.7
+   */
+  public AbstractInputStreamContent setCloseInputStream(boolean closeInputStream) {
+    this.closeInputStream = closeInputStream;
+    return this;
+  }
+
   /**
    * Writes the content provided by the given source input stream into the given destination output
    * stream.
+   *
    * <p>
-   * The input stream is guaranteed to be closed at the end of the method.
+   * The input stream is guaranteed to be closed at the end of this method.
    * </p>
+   *
    * <p>
    * Sample use:
    *
@@ -137,14 +176,46 @@ static void downloadMedia(HttpResponse response, File file)
    * @param outputStream destination output stream
    */
   public static void copy(InputStream inputStream, OutputStream outputStream) throws IOException {
+    copy(inputStream, outputStream, true);
+  }
+
+  /**
+   * Writes the content provided by the given source input stream into the given destination output
+   * stream.
+   *
+   * <p>
+   * Sample use:
+   *
+   * <pre><code>
+  static void downloadMedia(HttpResponse response, File file)
+      throws IOException {
+    FileOutputStream out = new FileOutputStream(file);
+    try {
+      AbstractInputStreamContent.copy(response.getContent(), out, true);
+    } finally {
+      out.close();
+    }
+  }
+   * </code></pre>
+   * </p>
+   *
+   * @param inputStream source input stream
+   * @param outputStream destination output stream
+   * @param closeInputStream whether the input stream should be closed at the end of this method
+   * @since 1.7
+   */
+  public static void copy(InputStream inputStream, OutputStream outputStream,
+      boolean closeInputStream) throws IOException {
     try {
       byte[] tmp = new byte[BUFFER_SIZE];
       int bytesRead;
       while ((bytesRead = inputStream.read(tmp)) != -1) {
         outputStream.write(tmp, 0, bytesRead);
       }
     } finally {
-      inputStream.close();
+      if (closeInputStream) {
+        inputStream.close();
+      }
     }
   }
 }

google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java

@@ -115,7 +115,7 @@ public boolean retrySupported() {
   }
 
   @Override
-  protected InputStream getInputStream() {
+  public InputStream getInputStream() {
     return new ByteArrayInputStream(byteArray, offset, length);
   }
 
@@ -128,4 +128,9 @@ public ByteArrayContent setEncoding(String encoding) {
   public ByteArrayContent setType(String type) {
     return (ByteArrayContent) super.setType(type);
   }
+
+  @Override
+  public ByteArrayContent setCloseInputStream(boolean closeInputStream) {
+    return (ByteArrayContent) super.setCloseInputStream(closeInputStream);
+  }
 }

google-http-client/src/main/java/com/google/api/client/http/FileContent.java

@@ -67,7 +67,7 @@ public boolean retrySupported() {
   }
 
   @Override
-  protected InputStream getInputStream() throws FileNotFoundException {
+  public InputStream getInputStream() throws FileNotFoundException {
     return new FileInputStream(file);
   }
 
@@ -89,4 +89,9 @@ public FileContent setEncoding(String encoding) {
   public FileContent setType(String type) {
     return (FileContent) super.setType(type);
   }
+
+  @Override
+  public FileContent setCloseInputStream(boolean closeInputStream) {
+    return (FileContent) super.setCloseInputStream(closeInputStream);
+  }
 }

google-http-client/src/main/java/com/google/api/client/http/GZipContent.java

@@ -41,10 +41,20 @@ final class GZipContent extends AbstractHttpContent {
     this.contentType = contentType;
   }
 
+  /**
+   * Writes the content to the given output stream and closes the output stream.
+   * 
+   * <p>
+   * The output stream is closed by calling {@link GZIPOutputStream#close} to avoid a resource leak
+   * caused due to an instance of {@link java.util.zip.Deflater} being left open. See Bug <a
+   * href='http://code.google.com/p/google-http-java-client/issues/detail?id=61'>61</a> for more
+   * information.
+   * </p>
+   */
   public void writeTo(OutputStream out) throws IOException {
     GZIPOutputStream zipper = new GZIPOutputStream(out);
     httpContent.writeTo(zipper);
-    zipper.finish();
+    zipper.close();
   }
 
   @Override

google-http-client/src/main/java/com/google/api/client/http/HttpContent.java

@@ -40,7 +40,15 @@ public interface HttpContent {
   /** Returns the content type or {@code null} for none. */
   String getType();
 
-  /** Writes the content to the given output stream. */
+  /**
+   * Writes the content to the given output stream.
+   *
+   * <p>
+   * The recommendation for implementations is that they should not close the output stream. Callers
+   * should not assume whether or not the output stream has been closed. Implementations that do not
+   * close the output stream should flush it at the end of the method.
+   * </p>
+   */
   void writeTo(OutputStream out) throws IOException;
 
   /**

google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java

@@ -49,8 +49,8 @@ public final class HttpRequest {
    *
    * @since 1.4
    */
-  public static final String USER_AGENT_SUFFIX =
-      "Google-HTTP-Java-Client/" + Strings.VERSION + " (gzip)";
+  public static final String USER_AGENT_SUFFIX = "Google-HTTP-Java-Client/" + Strings.VERSION
+      + " (gzip)";
 
   /**
    * HTTP request execute interceptor to intercept the start of {@link #execute()} (before executing
@@ -80,6 +80,13 @@ static String executeAndGetValueOfSomeCustomHeader(HttpRequest request) {
    */
   private HttpHeaders responseHeaders = new HttpHeaders();
 
+  /**
+   * Some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1. If this
+   * value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
+   * empty contents. Defaults to {@code true}.
+   */
+  private boolean allowEmptyContent = true;
+
   /**
    * Set the number of retries that will be allowed to execute as the result of an
    * {@link HttpUnsuccessfulResponseHandler} before being terminated or {@code 0} to not retry
@@ -521,6 +528,29 @@ public HttpRequest setUnsuccessfulResponseHandler(
     return this;
   }
 
+  /**
+   * Some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1. If this
+   * value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
+   * empty contents. Defaults to {@code true}.
+   *
+   * @since 1.7
+   */
+  public HttpRequest setAllowEmptyContent(boolean allowEmptyContent) {
+    this.allowEmptyContent = allowEmptyContent;
+    return this;
+  }
+
+  /**
+   * Some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1. If this
+   * value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
+   * empty contents. Defaults to {@code true}.
+   *
+   * @since 1.7
+   */
+  public boolean isAllowEmptyContent() {
+    return allowEmptyContent;
+  }
+
   /**
    * Returns the number of retries that will be allowed to execute as the result of an
    * {@link HttpUnsuccessfulResponseHandler} before being terminated or {@code 0} to not retry
@@ -610,7 +640,7 @@ public boolean getThrowExceptionOnExecuteError() {
   }
 
   /**
-   * Sets whether to throw an exception at the end of {@link #execute()} on an HTTP error code
+   * Sets whether to throw an exception at the end of {@link #execute()} on a HTTP error code
    * (non-2XX) after all retries and response handlers have been exhausted.
    *
    * <p>
@@ -674,13 +704,13 @@ public HttpResponse execute() throws IOException {
           lowLevelHttpRequest = transport.buildGetRequest(urlString);
           break;
         case HEAD:
-          Preconditions.checkArgument(
-              transport.supportsHead(), "HTTP transport doesn't support HEAD");
+          Preconditions.checkArgument(transport.supportsHead(),
+              "HTTP transport doesn't support HEAD");
           lowLevelHttpRequest = transport.buildHeadRequest(urlString);
           break;
         case PATCH:
-          Preconditions.checkArgument(
-              transport.supportsPatch(), "HTTP transport doesn't support PATCH");
+          Preconditions.checkArgument(transport.supportsPatch(),
+              "HTTP transport doesn't support PATCH");
           lowLevelHttpRequest = transport.buildPatchRequest(urlString);
           break;
         case POST:
@@ -726,8 +756,8 @@ public HttpResponse execute() throws IOException {
       }
       // content
       HttpContent content = this.content;
-      // Hack: some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1
-      if ((method == HttpMethod.PUT || method == HttpMethod.POST || method == HttpMethod.PATCH)
+      if (!isAllowEmptyContent()
+          && (method == HttpMethod.PUT || method == HttpMethod.POST || method == HttpMethod.PATCH)
           && (content == null || content.getLength() == 0)) {
         content = ByteArrayContent.fromString(null, " ");
       }
@@ -742,8 +772,9 @@ public HttpResponse execute() throws IOException {
             && LogContent.isTextBasedContentType(contentType)
             && (loggable && !disableContentLogging || logger.isLoggable(Level.ALL))) {
           if (contentLength <= contentLoggingLimit && contentLoggingLimit != 0) {
-            content = new LogContent(content, contentType, contentEncoding, contentLength,
-                contentLoggingLimit);
+            content =
+                new LogContent(content, contentType, contentEncoding, contentLength,
+                    contentLoggingLimit);
           }
         }
         // gzip
@@ -795,8 +826,7 @@ public HttpResponse execute() throws IOException {
             // The unsuccessful request's error could not be handled and it is a redirect request.
             handleRedirect(response);
             redirectRequest = true;
-          } else if (
-              retrySupported && backOffPolicy != null
+          } else if (retrySupported && backOffPolicy != null
               && backOffPolicy.isBackOffRequired(response.getStatusCode())) {
             // The unsuccessful request's error could not be handled and should be backed off before
             // retrying.

google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

@@ -378,8 +378,12 @@ public InputStream getContent() throws IOException {
           !disableContentLogging && logger.isLoggable(Level.CONFIG) || logger.isLoggable(Level.ALL);
       if (loggable) {
         ByteArrayOutputStream debugStream = new ByteArrayOutputStream();
-        AbstractInputStreamContent.copy(content, debugStream);
-        debugContentByteArray = debugStream.toByteArray();
+        try {
+          AbstractInputStreamContent.copy(content, debugStream);
+          debugContentByteArray = debugStream.toByteArray();
+        } finally {
+          debugStream.close();
+        }
         content = new ByteArrayInputStream(debugContentByteArray);
         logger.config("Response size: " + debugContentByteArray.length + " bytes");
       }