Set empty request body if it was null (#1778)

Co-authored-by: Kevin Davis <[email protected]>

core/src/main/java/feign/Client.java

@@ -199,8 +199,19 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
         connection.addRequestProperty("Accept", "*/*");
       }
 
-      if (request.body() != null) {
-        if (disableRequestBuffering) {
+      boolean hasEmptyBody = false;
+      byte[] body = request.body();
+      if (body == null && request.httpMethod().isWithBody()) {
+        body = new byte[0];
+        hasEmptyBody = true;
+      }
+
+      if (body != null) {
+        /*
+         * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal
+         * retry logic applies to such requests.
+         */
+        if (disableRequestBuffering && !hasEmptyBody) {
           if (contentLength != null) {
             connection.setFixedLengthStreamingMode(contentLength);
           } else {
@@ -215,7 +226,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
           out = new DeflaterOutputStream(out);
         }
         try {
-          out.write(request.body());
+          out.write(body);
         } finally {
           try {
             out.close();

core/src/main/java/feign/Request.java

@@ -31,13 +31,27 @@ public final class Request implements Serializable {
   public enum HttpMethod {
     GET,
     HEAD,
-    POST,
-    PUT,
+    POST(true),
+    PUT(true),
     DELETE,
     CONNECT,
     OPTIONS,
     TRACE,
-    PATCH
+    PATCH(true);
+
+    private final boolean withBody;
+
+    HttpMethod() {
+      this(false);
+    }
+
+    HttpMethod(boolean withBody) {
+      this.withBody = withBody;
+    }
+
+    public boolean isWithBody() {
+      return this.withBody;
+    }
   }
 
   public enum ProtocolVersion {

core/src/test/java/feign/client/AbstractClientTest.java

@@ -210,7 +210,7 @@ protected void log(String configKey, String format, Object... args) {}
   }
 
   @Test
-  public void noResponseBodyForPost() {
+  public void noResponseBodyForPost() throws Exception {
     server.enqueue(new MockResponse());
 
     TestInterface api =
@@ -220,7 +220,7 @@ public void noResponseBodyForPost() {
   }
 
   @Test
-  public void noResponseBodyForPut() {
+  public void noResponseBodyForPut() throws Exception {
     server.enqueue(new MockResponse());
 
     TestInterface api =

core/src/test/java/feign/client/DefaultClientTest.java

@@ -14,6 +14,7 @@
 package feign.client;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 import static org.hamcrest.core.Is.isA;
 import static org.junit.Assert.assertEquals;
 
@@ -22,6 +23,7 @@
 import feign.Feign;
 import feign.Feign.Builder;
 import feign.RetryableException;
+import feign.assertj.MockWebServerAssertions;
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.InetSocketAddress;
@@ -30,6 +32,7 @@
 import java.net.Proxy.Type;
 import java.net.SocketAddress;
 import java.net.URL;
+import java.util.Collections;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
 import okhttp3.mockwebserver.MockResponse;
@@ -92,6 +95,22 @@ public void testPatch() throws Exception {
     super.testPatch();
   }
 
+  @Override
+  public void noResponseBodyForPost() throws Exception {
+    super.noResponseBodyForPost();
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasMethod("POST")
+        .hasHeaders(entry("Content-Length", Collections.singletonList("0")));
+  }
+
+  @Override
+  public void noResponseBodyForPut() throws Exception {
+    super.noResponseBodyForPut();
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasMethod("PUT")
+        .hasHeaders(entry("Content-Length", Collections.singletonList("0")));
+  }
+
   @Test
   @Override
   public void noResponseBodyForPatch() {

java11/pom.xml

@@ -51,16 +51,6 @@
       <scope>test</scope>
     </dependency>
 
-    <dependency>
-      <groupId>org.assertj</groupId>
-      <artifactId>assertj-core</artifactId>
-      <scope>test</scope>
-    </dependency>
-    <dependency>
-      <groupId>junit</groupId>
-      <artifactId>junit</artifactId>
-      <scope>test</scope>
-    </dependency>
     <dependency>
       <groupId>io.github.openfeign</groupId>
       <artifactId>feign-core</artifactId>

jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java

@@ -50,7 +50,7 @@ public void testPatch() throws Exception {
   }
 
   @Override
-  public void noResponseBodyForPut() {
+  public void noResponseBodyForPut() throws Exception {
     try {
       super.noResponseBodyForPut();
     } catch (final IllegalStateException e) {

okhttp/src/main/java/feign/okhttp/OkHttpClient.java

@@ -17,7 +17,6 @@
 
 import feign.AsyncClient;
 import feign.Client;
-import feign.Request.HttpMethod;
 import feign.Request.ProtocolVersion;
 import java.io.IOException;
 import java.io.InputStream;
@@ -78,11 +77,7 @@ static Request toOkHttpRequest(feign.Request input) {
     }
 
     byte[] inputBody = input.body();
-    boolean isMethodWithBody =
-        HttpMethod.POST == input.httpMethod()
-            || HttpMethod.PUT == input.httpMethod()
-            || HttpMethod.PATCH == input.httpMethod();
-    if (isMethodWithBody) {
+    if (input.httpMethod().isWithBody()) {
       requestBuilder.removeHeader("Content-Type");
       if (inputBody == null) {
         // write an empty BODY to conform with okhttp 2.4.0+

pom.xml

@@ -517,13 +517,9 @@
         </dependencies>
       </plugin>
 
-      <!-- Ensures checksums are added to published jars -->
       <plugin>
         <artifactId>maven-install-plugin</artifactId>
         <version>${maven-install-plugin.version}</version>
-        <configuration>
-          <createChecksum>true</createChecksum>
-        </configuration>
       </plugin>
 
       <plugin>