OkHttpClientHttpRequestFactory allows POST request without body

Issue: SPR-15015

spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java

@@ -19,7 +19,6 @@ package org.springframework.http.client;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URI;
-import java.net.URL;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -73,7 +72,7 @@ public class OkHttp3ClientHttpRequestFactory
 	/**
 	 * Sets the underlying read timeout in milliseconds.
 	 * A value of 0 specifies an infinite timeout.
-	 * @see okhttp3.OkHttpClient.Builder#readTimeout(long, TimeUnit)
+	 * @see OkHttpClient.Builder#readTimeout(long, TimeUnit)
 	 */
 	public void setReadTimeout(int readTimeout) {
 		this.client = this.client.newBuilder()
@@ -84,7 +83,7 @@ public class OkHttp3ClientHttpRequestFactory
 	/**
 	 * Sets the underlying write timeout in milliseconds.
 	 * A value of 0 specifies an infinite timeout.
-	 * @see okhttp3.OkHttpClient.Builder#writeTimeout(long, TimeUnit)
+	 * @see OkHttpClient.Builder#writeTimeout(long, TimeUnit)
 	 */
 	public void setWriteTimeout(int writeTimeout) {
 		this.client = this.client.newBuilder()
@@ -95,7 +94,7 @@ public class OkHttp3ClientHttpRequestFactory
 	/**
 	 * Sets the underlying connect timeout in milliseconds.
 	 * A value of 0 specifies an infinite timeout.
-	 * @see okhttp3.OkHttpClient.Builder#connectTimeout(long, TimeUnit)
+	 * @see OkHttpClient.Builder#connectTimeout(long, TimeUnit)
 	 */
 	public void setConnectTimeout(int connectTimeout) {
 		this.client = this.client.newBuilder()
@@ -127,23 +126,21 @@ public class OkHttp3ClientHttpRequestFactory
 	}
 
 
-	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri,
-			HttpMethod method) throws MalformedURLException {
+	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
+			throws MalformedURLException {
 
 		okhttp3.MediaType contentType = getContentType(headers);
-		RequestBody body = (content.length > 0 ? RequestBody.create(contentType, content) : null);
-
-		URL url = uri.toURL();
-		String methodName = method.name();
-		Request.Builder builder = new Request.Builder().url(url).method(methodName, body);
+		RequestBody body = (content.length > 0 ||
+				okhttp3.internal.http.HttpMethod.requiresRequestBody(method.name()) ?
+				RequestBody.create(contentType, content) : null);
 
+		Request.Builder builder = new Request.Builder().url(uri.toURL()).method(method.name(), body);
 		for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
 			String headerName = entry.getKey();
 			for (String headerValue : entry.getValue()) {
 				builder.addHeader(headerName, headerValue);
 			}
 		}
-
 		return builder.build();
 	}
 

spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java

@@ -19,7 +19,6 @@ package org.springframework.http.client;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URI;
-import java.net.URL;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -123,23 +122,21 @@ public class OkHttpClientHttpRequestFactory
 	}
 
 
-	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri,
-			HttpMethod method) throws MalformedURLException {
+	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
+			throws MalformedURLException {
 
 		com.squareup.okhttp.MediaType contentType = getContentType(headers);
-		RequestBody body = (content.length > 0 ? RequestBody.create(contentType, content) : null);
-
-		URL url = uri.toURL();
-		String methodName = method.name();
-		Request.Builder builder = new Request.Builder().url(url).method(methodName, body);
+		RequestBody body = (content.length > 0 ||
+				com.squareup.okhttp.internal.http.HttpMethod.requiresRequestBody(method.name()) ?
+				RequestBody.create(contentType, content) : null);
 
+		Request.Builder builder = new Request.Builder().url(uri.toURL()).method(method.name(), body);
 		for (Map.Entry<String, List<String>> entry : headers.entrySet()) {
 			String headerName = entry.getKey();
 			for (String headerValue : entry.getValue()) {
 				builder.addHeader(headerName, headerValue);
 			}
 		}
-
 		return builder.build();
 	}
 

spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java

@@ -1,3 +1,19 @@
+/*
+ * Copyright 2002-2016 the original author or authors.
+ *
+ * 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 org.springframework.web.client;
 
 import java.io.EOFException;
@@ -16,16 +32,16 @@ import org.junit.Before;
 
 import org.springframework.http.MediaType;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 /**
  * @author Brian Clozel
  */
 public class AbstractMockWebServerTestCase {
 
+	protected static final MediaType textContentType =
+			new MediaType("text", "plain", Collections.singletonMap("charset", "UTF-8"));
+
 	protected static final String helloWorld = "H\u00e9llo W\u00f6rld";
 
 	private MockWebServer server;
@@ -34,8 +50,6 @@ public class AbstractMockWebServerTestCase {
 
 	protected String baseUrl;
 
-	protected static final MediaType textContentType =
-			new MediaType("text", "plain", Collections.singletonMap("charset", "UTF-8"));
 
 	@Before
 	public void setUp() throws Exception {
@@ -51,68 +65,9 @@ public class AbstractMockWebServerTestCase {
 		this.server.shutdown();
 	}
 
-	protected class TestDispatcher extends Dispatcher {
-		@Override
-		public MockResponse dispatch(RecordedRequest request) throws InterruptedException {
-			try {
-				byte[] helloWorldBytes = helloWorld.getBytes(StandardCharsets.UTF_8);
-
-				if (request.getPath().equals("/get")) {
-					return getRequest(request, helloWorldBytes, textContentType.toString());
-				}
-				else if (request.getPath().equals("/get/nothing")) {
-					return getRequest(request, new byte[0], textContentType.toString());
-				}
-				else if (request.getPath().equals("/get/nocontenttype")) {
-					return getRequest(request, helloWorldBytes, null);
-				}
-				else if (request.getPath().equals("/post")) {
-					return postRequest(request, helloWorld, "/post/1", textContentType.toString(), helloWorldBytes);
-				}
-				else if (request.getPath().equals("/jsonpost")) {
-					return jsonPostRequest(request, "/jsonpost/1", "application/json; charset=utf-8");
-				}
-				else if (request.getPath().equals("/status/nocontent")) {
-					return new MockResponse().setResponseCode(204);
-				}
-				else if (request.getPath().equals("/status/notmodified")) {
-					return new MockResponse().setResponseCode(304);
-				}
-				else if (request.getPath().equals("/status/notfound")) {
-					return new MockResponse().setResponseCode(404);
-				}
-				else if (request.getPath().equals("/status/server")) {
-					return new MockResponse().setResponseCode(500);
-				}
-				else if (request.getPath().contains("/uri/")) {
-					return new MockResponse().setBody(request.getPath()).setHeader("Content-Type", "text/plain");
-				}
-				else if (request.getPath().equals("/multipart")) {
-					return multipartRequest(request);
-				}
-				else if (request.getPath().equals("/form")) {
-					return formRequest(request);
-				}
-				else if (request.getPath().equals("/delete")) {
-					return new MockResponse().setResponseCode(200);
-				}
-				else if (request.getPath().equals("/patch")) {
-					return patchRequest(request, helloWorld, textContentType.toString(), helloWorldBytes);
-				}
-				else if (request.getPath().equals("/put")) {
-					return putRequest(request, helloWorld);
-				}
-				return new MockResponse().setResponseCode(404);
-			}
-			catch (Throwable exc) {
-				return new MockResponse().setResponseCode(500).setBody(exc.toString());
-			}
-		}
-	}
-
 
 	private MockResponse getRequest(RecordedRequest request, byte[] body, String contentType) {
-		if(request.getMethod().equals("OPTIONS")) {
+		if (request.getMethod().equals("OPTIONS")) {
 			return new MockResponse().setResponseCode(200).setHeader("Allow", "GET, OPTIONS, HEAD, TRACE");
 		}
 		Buffer buf = new Buffer();
@@ -135,7 +90,7 @@ public class AbstractMockWebServerTestCase {
 		String requestContentType = request.getHeader("Content-Type");
 		assertNotNull("No content-type", requestContentType);
 		Charset charset = StandardCharsets.ISO_8859_1;
-		if(requestContentType.indexOf("charset=") > -1) {
+		if (requestContentType.contains("charset=")) {
 			String charsetName = requestContentType.split("charset=")[1];
 			charset = Charset.forName(charsetName);
 		}
@@ -151,10 +106,11 @@ public class AbstractMockWebServerTestCase {
 	}
 
 	private MockResponse jsonPostRequest(RecordedRequest request, String location, String contentType) {
-
-		assertTrue("Invalid request content-length",
-				Integer.parseInt(request.getHeader("Content-Length")) > 0);
-		assertNotNull("No content-type", request.getHeader("Content-Type"));
+		if (request.getBodySize() > 0) {
+			assertTrue("Invalid request content-length",
+					Integer.parseInt(request.getHeader("Content-Length")) > 0);
+			assertNotNull("No content-type", request.getHeader("Content-Type"));
+		}
 		return new MockResponse()
 				.setHeader("Location", baseUrl + location)
 				.setHeader("Content-Type", contentType)
@@ -174,8 +130,8 @@ public class AbstractMockWebServerTestCase {
 			assertPart(body, "form-data", boundary, "name 2", "text/plain", "value 2+2");
 			assertFilePart(body, "form-data", boundary, "logo", "logo.jpg", "image/jpeg");
 		}
-		catch (EOFException e) {
-			throw new RuntimeException(e);
+		catch (EOFException ex) {
+			throw new IllegalStateException(ex);
 		}
 		return new MockResponse().setResponseCode(200);
 	}
@@ -218,13 +174,14 @@ public class AbstractMockWebServerTestCase {
 
 	private MockResponse patchRequest(RecordedRequest request, String expectedRequestContent,
 			String contentType, byte[] responseBody) {
+
 		assertEquals("PATCH", request.getMethod());
 		assertTrue("Invalid request content-length",
 				Integer.parseInt(request.getHeader("Content-Length")) > 0);
 		String requestContentType = request.getHeader("Content-Type");
 		assertNotNull("No content-type", requestContentType);
 		Charset charset = StandardCharsets.ISO_8859_1;
-		if(requestContentType.indexOf("charset=") > -1) {
+		if (requestContentType.contains("charset=")) {
 			String charsetName = requestContentType.split("charset=")[1];
 			charset = Charset.forName(charsetName);
 		}
@@ -243,7 +200,7 @@ public class AbstractMockWebServerTestCase {
 		String requestContentType = request.getHeader("Content-Type");
 		assertNotNull("No content-type", requestContentType);
 		Charset charset = StandardCharsets.ISO_8859_1;
-		if(requestContentType.indexOf("charset=") > -1) {
+		if (requestContentType.contains("charset=")) {
 			String charsetName = requestContentType.split("charset=")[1];
 			charset = Charset.forName(charsetName);
 		}
@@ -251,4 +208,65 @@ public class AbstractMockWebServerTestCase {
 		return new MockResponse().setResponseCode(202);
 	}
 
+
+	protected class TestDispatcher extends Dispatcher {
+
+		@Override
+		public MockResponse dispatch(RecordedRequest request) throws InterruptedException {
+			try {
+				byte[] helloWorldBytes = helloWorld.getBytes(StandardCharsets.UTF_8);
+
+				if (request.getPath().equals("/get")) {
+					return getRequest(request, helloWorldBytes, textContentType.toString());
+				}
+				else if (request.getPath().equals("/get/nothing")) {
+					return getRequest(request, new byte[0], textContentType.toString());
+				}
+				else if (request.getPath().equals("/get/nocontenttype")) {
+					return getRequest(request, helloWorldBytes, null);
+				}
+				else if (request.getPath().equals("/post")) {
+					return postRequest(request, helloWorld, "/post/1", textContentType.toString(), helloWorldBytes);
+				}
+				else if (request.getPath().equals("/jsonpost")) {
+					return jsonPostRequest(request, "/jsonpost/1", "application/json; charset=utf-8");
+				}
+				else if (request.getPath().equals("/status/nocontent")) {
+					return new MockResponse().setResponseCode(204);
+				}
+				else if (request.getPath().equals("/status/notmodified")) {
+					return new MockResponse().setResponseCode(304);
+				}
+				else if (request.getPath().equals("/status/notfound")) {
+					return new MockResponse().setResponseCode(404);
+				}
+				else if (request.getPath().equals("/status/server")) {
+					return new MockResponse().setResponseCode(500);
+				}
+				else if (request.getPath().contains("/uri/")) {
+					return new MockResponse().setBody(request.getPath()).setHeader("Content-Type", "text/plain");
+				}
+				else if (request.getPath().equals("/multipart")) {
+					return multipartRequest(request);
+				}
+				else if (request.getPath().equals("/form")) {
+					return formRequest(request);
+				}
+				else if (request.getPath().equals("/delete")) {
+					return new MockResponse().setResponseCode(200);
+				}
+				else if (request.getPath().equals("/patch")) {
+					return patchRequest(request, helloWorld, textContentType.toString(), helloWorldBytes);
+				}
+				else if (request.getPath().equals("/put")) {
+					return putRequest(request, helloWorld);
+				}
+				return new MockResponse().setResponseCode(404);
+			}
+			catch (Throwable ex) {
+				return new MockResponse().setResponseCode(500).setBody(ex.toString());
+			}
+		}
+	}
+
 }

spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java

@@ -50,18 +50,13 @@ import org.springframework.http.client.ClientHttpRequestFactory;
 import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
 import org.springframework.http.client.Netty4ClientHttpRequestFactory;
 import org.springframework.http.client.OkHttp3ClientHttpRequestFactory;
+import org.springframework.http.client.OkHttpClientHttpRequestFactory;
 import org.springframework.http.client.SimpleClientHttpRequestFactory;
 import org.springframework.http.converter.json.MappingJacksonValue;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 /**
  * @author Arjen Poutsma
@@ -78,10 +73,11 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 	@Parameterized.Parameters
 	public static Iterable<? extends ClientHttpRequestFactory> data() {
 		return Arrays.asList(
+				new SimpleClientHttpRequestFactory(),
 				new HttpComponentsClientHttpRequestFactory(),
 				new Netty4ClientHttpRequestFactory(),
 				new OkHttp3ClientHttpRequestFactory(),
-				new SimpleClientHttpRequestFactory()
+				new OkHttpClientHttpRequestFactory()
 		);
 	}
 
@@ -305,10 +301,15 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 		assertTrue(content.contains("\"type\":\"bar\""));
 	}
 
+	@Test  // SPR-15015
+	public void postWithoutBody() throws Exception {
+		assertNull(template.postForObject(baseUrl + "/jsonpost", null, String.class));
+	}
+
 
-	public interface MyJacksonView1 {};
+	public interface MyJacksonView1 {}
 
-	public interface MyJacksonView2 {};
+	public interface MyJacksonView2 {}
 
 
 	public static class MySampleBean {