diff --git a/core/src/main/java/feign/Client.java b/core/src/main/java/feign/Client.java index 885fac861..de6404ff8 100644 --- a/core/src/main/java/feign/Client.java +++ b/core/src/main/java/feign/Client.java @@ -65,7 +65,7 @@ class Default implements Client { /** * Disable the request body internal buffering for {@code HttpURLConnection}. - * + * * @see HttpURLConnection#setFixedLengthStreamingMode(int) * @see HttpURLConnection#setFixedLengthStreamingMode(long) * @see HttpURLConnection#setChunkedStreamingMode(int) @@ -74,7 +74,7 @@ class Default implements Client { /** * Create a new client, which disable request buffering by default. - * + * * @param sslContextFactory SSLSocketFactory for secure https URL connections. * @param hostnameVerifier the host name verifier. */ @@ -86,7 +86,7 @@ public Default(SSLSocketFactory sslContextFactory, HostnameVerifier hostnameVeri /** * Create a new client. - * + * * @param sslContextFactory SSLSocketFactory for secure https URL connections. * @param hostnameVerifier the host name verifier. * @param disableRequestBuffering Disable the request body internal buffering for @@ -196,8 +196,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 { @@ -212,7 +223,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(); diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index c9987d10f..a974e467c 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -30,7 +30,21 @@ public final class Request implements Serializable { public enum HttpMethod { - GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, PATCH + GET, HEAD, POST(true), PUT(true), DELETE, CONNECT, OPTIONS, TRACE, PATCH(true); + + private final boolean withBody; + + HttpMethod() { + this(false); + } + + HttpMethod(boolean withBody) { + this.withBody = withBody; + } + + public boolean isWithBody() { + return this.withBody; + } } public enum ProtocolVersion { diff --git a/core/src/test/java/feign/client/AbstractClientTest.java b/core/src/test/java/feign/client/AbstractClientTest.java index e9790eedb..44e59a695 100644 --- a/core/src/test/java/feign/client/AbstractClientTest.java +++ b/core/src/test/java/feign/client/AbstractClientTest.java @@ -203,7 +203,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 = newBuilder() @@ -213,7 +213,7 @@ public void noResponseBodyForPost() { } @Test - public void noResponseBodyForPut() { + public void noResponseBodyForPut() throws Exception { server.enqueue(new MockResponse()); TestInterface api = newBuilder() diff --git a/core/src/test/java/feign/client/DefaultClientTest.java b/core/src/test/java/feign/client/DefaultClientTest.java index d41ea6962..3bd77716d 100644 --- a/core/src/test/java/feign/client/DefaultClientTest.java +++ b/core/src/test/java/feign/client/DefaultClientTest.java @@ -13,11 +13,17 @@ */ package feign.client; -import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.core.Is.isA; -import static org.junit.Assert.assertEquals; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; +import feign.Client; +import feign.Client.Proxied; +import feign.Feign; +import feign.Feign.Builder; +import feign.RetryableException; +import feign.assertj.MockWebServerAssertions; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.SocketPolicy; +import org.junit.Test; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLSession; import java.io.IOException; import java.net.HttpURLConnection; import java.net.InetSocketAddress; @@ -26,20 +32,11 @@ import java.net.Proxy.Type; import java.net.SocketAddress; import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.util.zip.DeflaterOutputStream; -import java.util.zip.GZIPOutputStream; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLSession; -import okio.Buffer; -import org.junit.Test; -import feign.Client; -import feign.Client.Proxied; -import feign.Feign; -import feign.Feign.Builder; -import feign.RetryableException; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.SocketPolicy; +import java.util.Collections; +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; /** * Tests client-specific behavior, such as ensuring Content-Length is sent when specified. @@ -97,6 +94,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() { diff --git a/java11/pom.xml b/java11/pom.xml index 09a09e8da..0ffd0952f 100644 --- a/java11/pom.xml +++ b/java11/pom.xml @@ -51,16 +51,6 @@ test - - org.assertj - assertj-core - test - - - junit - junit - test - io.github.openfeign feign-core diff --git a/jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java b/jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java index 1016391b2..c8c4e62bd 100644 --- a/jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java +++ b/jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java @@ -51,7 +51,7 @@ public void testPatch() throws Exception { } @Override - public void noResponseBodyForPut() { + public void noResponseBodyForPut() throws Exception { try { super.noResponseBodyForPut(); } catch (final IllegalStateException e) { diff --git a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java index 221cf44ef..26cd726f1 100644 --- a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java +++ b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java @@ -77,10 +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+ diff --git a/pom.xml b/pom.xml index 7a6ed49e0..e8bb1999f 100644 --- a/pom.xml +++ b/pom.xml @@ -517,13 +517,9 @@ - maven-install-plugin ${maven-install-plugin.version} - - true -