OkHttpClientHttpRequestFactory allows POST request without body

Issue: SPR-15015
(cherry picked from commit 2954385)

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,
+	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
-			HttpMethod method) throws MalformedURLException {
+			throws MalformedURLException {
 
 		okhttp3.MediaType contentType = getContentType(headers);
-		RequestBody body = (content.length > 0 ? RequestBody.create(contentType, content) : null);
+		RequestBody body = (content.length > 0 ||
-
+				okhttp3.internal.http.HttpMethod.requiresRequestBody(method.name()) ?
-		URL url = uri.toURL();
+				RequestBody.create(contentType, content) : null);
-		String methodName = method.name();
-		Request.Builder builder = new Request.Builder().url(url).method(methodName, body);
 
+		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;
@@ -121,23 +120,21 @@ public class OkHttpClientHttpRequestFactory
 	}
 
 
-	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri,
+	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
-			HttpMethod method) throws MalformedURLException {
+			throws MalformedURLException {
 
 		com.squareup.okhttp.MediaType contentType = getContentType(headers);
-		RequestBody body = (content.length > 0 ? RequestBody.create(contentType, content) : null);
+		RequestBody body = (content.length > 0 ||
-
+				com.squareup.okhttp.internal.http.HttpMethod.requiresRequestBody(method.name()) ?
-		URL url = uri.toURL();
+				RequestBody.create(contentType, content) : null);
-		String methodName = method.name();
-		Request.Builder builder = new Request.Builder().url(url).method(methodName, body);
 
+		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;
@@ -15,21 +31,18 @@ import org.junit.Before;
 
 import org.springframework.http.MediaType;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author Brian Clozel
  */
 public class AbstractMockWebServerTestCase {
 
-	protected static final String helloWorld = "H\u00e9llo W\u00f6rld";
+	private static final Charset UTF_8 = Charset.forName("UTF-8");
 
-  private static final Charset UTF_8 = Charset.forName("UTF-8");
+	private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1");
 
-  private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1");
+	protected static final String helloWorld = "H\u00e9llo W\u00f6rld";
 
 	protected static final MediaType textContentType =
 			new MediaType("text", "plain", Collections.singletonMap("charset", "UTF-8"));
@@ -40,6 +53,7 @@ public class AbstractMockWebServerTestCase {
 
 	protected String baseUrl;
 
+
 	@Before
 	public void setUp() throws Exception {
 		this.server = new MockWebServer();
@@ -54,68 +68,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(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();
@@ -138,7 +93,7 @@ public class AbstractMockWebServerTestCase {
 		String requestContentType = request.getHeader("Content-Type");
 		assertNotNull("No content-type", requestContentType);
 		Charset charset = ISO_8859_1;
-		if(requestContentType.indexOf("charset=") > -1) {
+		if (requestContentType.contains("charset=")) {
 			String charsetName = requestContentType.split("charset=")[1];
 			charset = Charset.forName(charsetName);
 		}
@@ -154,10 +109,11 @@ public class AbstractMockWebServerTestCase {
 	}
 
 	private MockResponse jsonPostRequest(RecordedRequest request, String location, String contentType) {
-
+		if (request.getBodySize() > 0) {
-		assertTrue("Invalid request content-length",
+			assertTrue("Invalid request content-length",
-				Integer.parseInt(request.getHeader("Content-Length")) > 0);
+					Integer.parseInt(request.getHeader("Content-Length")) > 0);
-		assertNotNull("No content-type", request.getHeader("Content-Type"));
+			assertNotNull("No content-type", request.getHeader("Content-Type"));
+		}
 		return new MockResponse()
 				.setHeader("Location", baseUrl + location)
 				.setHeader("Content-Type", contentType)
@@ -177,8 +133,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) {
+		catch (EOFException ex) {
-			throw new RuntimeException(e);
+			throw new IllegalStateException(ex);
 		}
 		return new MockResponse().setResponseCode(200);
 	}
@@ -221,13 +177,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 = ISO_8859_1;
-		if(requestContentType.indexOf("charset=") > -1) {
+		if (requestContentType.contains("charset=")) {
 			String charsetName = requestContentType.split("charset=")[1];
 			charset = Charset.forName(charsetName);
 		}
@@ -246,7 +203,7 @@ public class AbstractMockWebServerTestCase {
 		String requestContentType = request.getHeader("Content-Type");
 		assertNotNull("No content-type", requestContentType);
 		Charset charset = ISO_8859_1;
-		if(requestContentType.indexOf("charset=") > -1) {
+		if (requestContentType.contains("charset=")) {
 			String charsetName = requestContentType.split("charset=")[1];
 			charset = Charset.forName(charsetName);
 		}
@@ -254,4 +211,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(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,6 +50,7 @@ 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;
@@ -72,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()
 		);
 	}
 
@@ -141,7 +143,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 	public void postForLocationEntity() throws URISyntaxException {
 		HttpHeaders entityHeaders = new HttpHeaders();
 		entityHeaders.setContentType(new MediaType("text", "plain", Charset.forName("ISO-8859-15")));
-		HttpEntity<String> entity = new HttpEntity<String>(helloWorld, entityHeaders);
+		HttpEntity<String> entity = new HttpEntity<>(helloWorld, entityHeaders);
 		URI location = template.postForLocation(baseUrl + "/{method}", entity, "post");
 		assertEquals("Invalid location", new URI(baseUrl + "/post/1"), location);
 	}
@@ -199,7 +201,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 
 	@Test
 	public void multipart() throws UnsupportedEncodingException {
-		MultiValueMap<String, Object> parts = new LinkedMultiValueMap<String, Object>();
+		MultiValueMap<String, Object> parts = new LinkedMultiValueMap<>();
 		parts.add("name 1", "value 1");
 		parts.add("name 2", "value 2+1");
 		parts.add("name 2", "value 2+2");
@@ -211,7 +213,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 
 	@Test
 	public void form() throws UnsupportedEncodingException {
-		MultiValueMap<String, String> form = new LinkedMultiValueMap<String, String>();
+		MultiValueMap<String, String> form = new LinkedMultiValueMap<>();
 		form.add("name 1", "value 1");
 		form.add("name 2", "value 2+1");
 		form.add("name 2", "value 2+2");
@@ -223,7 +225,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 	public void exchangeGet() throws Exception {
 		HttpHeaders requestHeaders = new HttpHeaders();
 		requestHeaders.set("MyHeader", "MyValue");
-		HttpEntity<String> requestEntity = new HttpEntity<String>(requestHeaders);
+		HttpEntity<String> requestEntity = new HttpEntity<>(requestHeaders);
 		ResponseEntity<String> response =
 				template.exchange(baseUrl + "/{method}", HttpMethod.GET, requestEntity, String.class, "get");
 		assertEquals("Invalid content", helloWorld, response.getBody());
@@ -234,7 +236,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 		HttpHeaders requestHeaders = new HttpHeaders();
 		requestHeaders.set("MyHeader", "MyValue");
 		requestHeaders.setContentType(MediaType.TEXT_PLAIN);
-		HttpEntity<String> requestEntity = new HttpEntity<String>(helloWorld, requestHeaders);
+		HttpEntity<String> requestEntity = new HttpEntity<>(helloWorld, requestHeaders);
 		HttpEntity<Void> result = template.exchange(baseUrl + "/{method}", HttpMethod.POST, requestEntity, Void.class, "post");
 		assertEquals("Invalid location", new URI(baseUrl + "/post/1"), result.getHeaders().getLocation());
 		assertFalse(result.hasBody());
@@ -248,7 +250,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 		bean.setWith1("with");
 		bean.setWith2("with");
 		bean.setWithout("without");
-		HttpEntity<MySampleBean> entity = new HttpEntity<MySampleBean>(bean, entityHeaders);
+		HttpEntity<MySampleBean> entity = new HttpEntity<>(bean, entityHeaders);
 		String s = template.postForObject(baseUrl + "/jsonpost", entity, String.class);
 		assertTrue(s.contains("\"with1\":\"with\""));
 		assertTrue(s.contains("\"with2\":\"with\""));
@@ -262,7 +264,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase
 		MySampleBean bean = new MySampleBean("with", "with", "without");
 		MappingJacksonValue jacksonValue = new MappingJacksonValue(bean);
 		jacksonValue.setSerializationView(MyJacksonView1.class);
-		HttpEntity<MappingJacksonValue> entity = new HttpEntity<MappingJacksonValue>(jacksonValue, entityHeaders);
+		HttpEntity<MappingJacksonValue> entity = new HttpEntity<>(jacksonValue, entityHeaders);
 		String s = template.postForObject(baseUrl + "/jsonpost", entity, String.class);
 		assertTrue(s.contains("\"with1\":\"with\""));
 		assertFalse(s.contains("\"with2\":\"with\""));
@@ -290,10 +292,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 {