[http issue 115] NetHttpTransport drops Content-Length header when posting 0-length data

http://codereview.appspot.com/6463064/

Author: Yaniv Inbar

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

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * 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
+ *
+ * http://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 java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * Empty HTTP content of length zero just to force {@link HttpRequest#execute()} to add the header
+ * {@code Content-Length: 0}.
+ *
+ * <p>
+ * Note that there is no {@code Content-Length} header if the HTTP request content is {@code null} .
+ * However, when making a request like PUT or POST without a {@code Content-Length} header, some
+ * servers may respond with a {@code 411 Length Required} error. Specifying the
+ * {@code Content-Length: 0} header may work around that problem.
+ * </p>
+ *
+ * @since 1.11
+ * @author Yaniv Inbar
+ */
+public class EmptyContent implements HttpContent {
+
+  public long getLength() throws IOException {
+    return 0;
+  }
+
+  public String getEncoding() {
+    return null;
+  }
+
+  public String getType() {
+    return null;
+  }
+
+  public void writeTo(OutputStream out) throws IOException {
+    out.flush();
+  }
+
+  public boolean retrySupported() {
+    return true;
+  }
+}

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

@@ -88,6 +88,7 @@ static String executeAndGetValueOfSomeCustomHeader(HttpRequest request) {
    * value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
    * empty contents. Defaults to {@code true}.
    */
+  @Deprecated
   private boolean allowEmptyContent = true;
 
   /**
@@ -594,9 +595,13 @@ public HttpRequest setUnsuccessfulResponseHandler(
    * empty contents. Defaults to {@code true}.
    *
    * @since 1.7
+   * @deprecated (scheduled to be removed in 1.12) Use {@code setContent(new EmptyContent())}
    */
+  @Deprecated
   public HttpRequest setAllowEmptyContent(boolean allowEmptyContent) {
     this.allowEmptyContent = allowEmptyContent;
+    HttpTransport.LOGGER.warning("setAllowEmptyContent is deprecated and will be removed in 1.12;"
+        + " use setEmptyContent instead (if needed)");
     return this;
   }
 
@@ -606,8 +611,11 @@ public HttpRequest setAllowEmptyContent(boolean allowEmptyContent) {
    * empty contents. Defaults to {@code true}.
    *
    * @since 1.7
+   * @deprecated (scheduled to be removed in 1.12)
    */
+  @Deprecated
   public boolean isAllowEmptyContent() {
+    HttpTransport.LOGGER.warning("isAllowEmptyContent is deprecated and will be removed in 1.12");
     return allowEmptyContent;
   }
 

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

@@ -17,10 +17,12 @@
 import com.google.api.client.http.HttpContent;
 import com.google.api.client.http.LowLevelHttpRequest;
 import com.google.api.client.http.LowLevelHttpResponse;
+import com.google.common.base.Preconditions;
 
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
+import java.net.ProtocolException;
 import java.net.URL;
 
 /**
@@ -32,8 +34,11 @@ final class NetHttpRequest extends LowLevelHttpRequest {
   private HttpContent content;
 
   NetHttpRequest(String requestMethod, String url) throws IOException {
-    HttpURLConnection connection =
-        this.connection = (HttpURLConnection) new URL(url).openConnection();
+    this(requestMethod, (HttpURLConnection) new URL(url).openConnection());
+  }
+
+  NetHttpRequest(String requestMethod, HttpURLConnection connection) throws ProtocolException {
+    this.connection = connection;
     connection.setRequestMethod(requestMethod);
     connection.setUseCaches(false);
     connection.setInstanceFollowRedirects(false);
@@ -67,8 +72,8 @@ public LowLevelHttpResponse execute() throws IOException {
       if (contentLength >= 0) {
         addHeader("Content-Length", Long.toString(contentLength));
       }
-      if (contentLength != 0) {
-        // setDoOutput(true) will change a GET method to POST, so only if contentLength != 0
+      String requestMethod = connection.getRequestMethod();
+      if ("POST".equals(requestMethod) || "PUT".equals(requestMethod)) {
         connection.setDoOutput(true);
         // see http://developer.android.com/reference/java/net/HttpURLConnection.html
         if (contentLength >= 0 && contentLength <= Integer.MAX_VALUE) {
@@ -82,6 +87,11 @@ public LowLevelHttpResponse execute() throws IOException {
         } finally {
           out.close();
         }
+      } else {
+        // cannot call setDoOutput(true) because it would change a GET method to POST
+        // for HEAD, OPTIONS, DELETE, or TRACE it would throw an exceptions
+        Preconditions.checkArgument(
+            contentLength == 0, "%s with non-zero content length is not supported", requestMethod);
       }
     }
     // connect

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

@@ -33,6 +33,12 @@
  * globally-shared instance of the HTTP transport.
  * </p>
  *
+ * <p>
+ * Upgrade warning: in prior version 1.10 when using GET method with non-zero content, it
+ * automatically changed the method to POST. However, starting with version 1.11 it now throws an
+ * {@link IllegalArgumentException} instead.  Instead, use POST.
+ * </p>
+ *
  * @since 1.0
  * @author Yaniv Inbar
  */

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

@@ -0,0 +1,109 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * 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
+ *
+ * http://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.testing.http.javanet;
+
+import com.google.common.base.Preconditions;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.UnknownServiceException;
+
+/**
+ * Mock for {@link HttpURLConnection}.
+ *
+ * <p>
+ * Implementation is not thread-safe.
+ * </p>
+ *
+ * @since 1.11
+ * @author Yaniv Inbar
+ */
+public class MockHttpURLConnection extends HttpURLConnection {
+
+  /** Whether {@link #doOutput} was called. */
+  private boolean doOutputCalled;
+
+  /**
+   * Output stream or {@code null} to throw an {@link UnknownServiceException} when
+   * {@link #getOutputStream()} is called.
+   */
+  private OutputStream outputStream = new ByteArrayOutputStream(0);
+
+  /**
+   * @param u the URL or {@code null} for none
+   */
+  public MockHttpURLConnection(URL u) {
+    super(u);
+  }
+
+  @Override
+  public void disconnect() {
+  }
+
+  @Override
+  public boolean usingProxy() {
+    return false;
+  }
+
+  @Override
+  public void connect() throws IOException {
+  }
+
+  @Override
+  public int getResponseCode() throws IOException {
+    return responseCode;
+  }
+
+  @Override
+  public void setDoOutput(boolean dooutput) {
+    doOutputCalled = true;
+  }
+
+  @Override
+  public OutputStream getOutputStream() throws IOException {
+    if (outputStream != null) {
+      return outputStream;
+    }
+    return super.getOutputStream();
+  }
+
+  /** Returns whether {@link #doOutput} was called. */
+  public final boolean doOutputCalled() {
+    return doOutputCalled;
+  }
+
+  /**
+   * Sets the output stream or {@code null} to throw an {@link UnknownServiceException} when
+   * {@link #getOutputStream()} is called.
+   *
+   * <p>
+   * By default it is {@code null}.
+   * </p>
+   */
+  public MockHttpURLConnection setOutputStream(OutputStream outputStream) {
+    this.outputStream = outputStream;
+    return this;
+  }
+
+  /** Sets the HTTP response status code. */
+  public MockHttpURLConnection setResponseCode(int responseCode) {
+    Preconditions.checkArgument(responseCode >= -1);
+    this.responseCode = responseCode;
+    return this;
+  }
+}

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

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * 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
+ *
+ * http://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.
+ */
+
+/**
+ * Testing utilities used for writing tests based on the {@code java.net} package.
+ *
+ * <p>
+ * <b>Warning: this package is experimental, and its content may be changed in incompatible ways or
+ * possibly entirely removed in a future version of the library</b>
+ * </p>
+ *
+ * @since 1.11
+ * @author Yaniv Inbar
+ */
+
+package com.google.api.client.testing.http.javanet;
+

google-http-client/src/test/java/com/google/api/client/http/EmptyContentTest.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * 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
+ *
+ * http://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 junit.framework.TestCase;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+/**
+ * Tests {@link EmptyContent}.
+ *
+ * @author Yaniv Inbar
+ */
+public class EmptyContentTest extends TestCase {
+
+  public void test() throws IOException {
+    EmptyContent content = new EmptyContent();
+    assertEquals(0L, content.getLength());
+    assertNull(content.getEncoding());
+    assertNull(content.getType());
+    assertTrue(content.retrySupported());
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    content.writeTo(out);
+    assertEquals(0, out.size());
+  }
+}

google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java

@@ -805,7 +805,8 @@ public void testUserAgent() {
     assertTrue(HttpRequest.USER_AGENT_SUFFIX.contains("gzip"));
   }
 
-  public void testExecute_emptyContent() throws IOException {
+  @Deprecated
+  public void testExecute_allowEmptyContent() throws IOException {
     class MyTransport extends MockHttpTransport {
       String expectedContent;
 

google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java

@@ -244,6 +244,7 @@ class DisconnectLowLevelHttpResponse extends MockLowLevelHttpResponse {
     boolean disconnectCalled;
     DisconnectByteArrayInputStream content;
 
+    @SuppressWarnings("resource")
     @Override
     public MockLowLevelHttpResponse setContent(String stringContent) {
       content = stringContent == null ? null : new DisconnectByteArrayInputStream(StringUtils