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,
+	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;
@@ -123,23 +122,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;
@@ -16,16 +32,16 @@ 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 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,7 +65,152 @@ public class AbstractMockWebServerTestCase {
 		this.server.shutdown();
 	}
 
+
+	private MockResponse getRequest(RecordedRequest request, byte[] body, String contentType) {
+		if (request.getMethod().equals("OPTIONS")) {
+			return new MockResponse().setResponseCode(200).setHeader("Allow", "GET, OPTIONS, HEAD, TRACE");
+		}
+		Buffer buf = new Buffer();
+		buf.write(body);
+		MockResponse response = new MockResponse()
+				.setHeader("Content-Length", body.length)
+				.setBody(buf)
+				.setResponseCode(200);
+		if (contentType != null) {
+			response = response.setHeader("Content-Type", contentType);
+		}
+		return response;
+	}
+
+	private MockResponse postRequest(RecordedRequest request, String expectedRequestContent,
+			String location, String contentType, byte[] responseBody) {
+
+		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.contains("charset=")) {
+			String charsetName = requestContentType.split("charset=")[1];
+			charset = Charset.forName(charsetName);
+		}
+		assertEquals("Invalid request body", expectedRequestContent, request.getBody().readString(charset));
+		Buffer buf = new Buffer();
+		buf.write(responseBody);
+		return new MockResponse()
+				.setHeader("Location", baseUrl + location)
+				.setHeader("Content-Type", contentType)
+				.setHeader("Content-Length", responseBody.length)
+				.setBody(buf)
+				.setResponseCode(201);
+	}
+
+	private MockResponse jsonPostRequest(RecordedRequest request, String location, String contentType) {
+		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)
+				.setHeader("Content-Length", request.getBody().size())
+				.setBody(request.getBody())
+				.setResponseCode(201);
+	}
+
+	private MockResponse multipartRequest(RecordedRequest request) {
+		String contentType = request.getHeader("Content-Type");
+		assertTrue(contentType.startsWith("multipart/form-data"));
+		String boundary = contentType.split("boundary=")[1];
+		Buffer body = request.getBody();
+		try {
+			assertPart(body, "form-data", boundary, "name 1", "text/plain", "value 1");
+			assertPart(body, "form-data", boundary, "name 2", "text/plain", "value 2+1");
+			assertPart(body, "form-data", boundary, "name 2", "text/plain", "value 2+2");
+			assertFilePart(body, "form-data", boundary, "logo", "logo.jpg", "image/jpeg");
+		}
+		catch (EOFException ex) {
+			throw new IllegalStateException(ex);
+		}
+		return new MockResponse().setResponseCode(200);
+	}
+
+	private void assertPart(Buffer buffer, String disposition, String boundary, String name,
+			String contentType, String value) throws EOFException {
+
+		assertTrue(buffer.readUtf8Line().contains("--" + boundary));
+		String line = buffer.readUtf8Line();
+		assertTrue(line.contains("Content-Disposition: "+ disposition));
+		assertTrue(line.contains("name=\""+ name + "\""));
+		assertTrue(buffer.readUtf8Line().startsWith("Content-Type: "+contentType));
+		assertTrue(buffer.readUtf8Line().equals("Content-Length: " + value.length()));
+		assertTrue(buffer.readUtf8Line().equals(""));
+		assertTrue(buffer.readUtf8Line().equals(value));
+	}
+
+	private void assertFilePart(Buffer buffer, String disposition, String boundary, String name,
+			String filename, String contentType) throws EOFException {
+
+		assertTrue(buffer.readUtf8Line().contains("--" + boundary));
+		String line = buffer.readUtf8Line();
+		assertTrue(line.contains("Content-Disposition: "+ disposition));
+		assertTrue(line.contains("name=\""+ name + "\""));
+		assertTrue(line.contains("filename=\""+ filename + "\""));
+		assertTrue(buffer.readUtf8Line().startsWith("Content-Type: "+contentType));
+		assertTrue(buffer.readUtf8Line().startsWith("Content-Length: "));
+		assertTrue(buffer.readUtf8Line().equals(""));
+		assertNotNull(buffer.readUtf8Line());
+	}
+
+	private MockResponse formRequest(RecordedRequest request) {
+		assertEquals("application/x-www-form-urlencoded", request.getHeader("Content-Type"));
+		String body = request.getBody().readUtf8();
+		assertThat(body, Matchers.containsString("name+1=value+1"));
+		assertThat(body, Matchers.containsString("name+2=value+2%2B1"));
+		assertThat(body, Matchers.containsString("name+2=value+2%2B2"));
+		return new MockResponse().setResponseCode(200);
+	}
+
+	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.contains("charset=")) {
+			String charsetName = requestContentType.split("charset=")[1];
+			charset = Charset.forName(charsetName);
+		}
+		assertEquals("Invalid request body", expectedRequestContent, request.getBody().readString(charset));
+		Buffer buf = new Buffer();
+		buf.write(responseBody);
+		return new MockResponse().setResponseCode(201)
+				.setHeader("Content-Length", responseBody.length)
+				.setHeader("Content-Type", contentType)
+				.setBody(buf);
+	}
+
+	private MockResponse putRequest(RecordedRequest request, String expectedRequestContent) {
+		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.contains("charset=")) {
+			String charsetName = requestContentType.split("charset=")[1];
+			charset = Charset.forName(charsetName);
+		}
+		assertEquals("Invalid request body", expectedRequestContent, request.getBody().readString(charset));
+		return new MockResponse().setResponseCode(202);
+	}
+
+
 	protected class TestDispatcher extends Dispatcher {
+
 		@Override
 		public MockResponse dispatch(RecordedRequest request) throws InterruptedException {
 			try {
@@ -104,151 +263,10 @@ public class AbstractMockWebServerTestCase {
 				}
 				return new MockResponse().setResponseCode(404);
 			}
-			catch (Throwable exc) {
+			catch (Throwable ex) {
-				return new MockResponse().setResponseCode(500).setBody(exc.toString());
+				return new MockResponse().setResponseCode(500).setBody(ex.toString());
 			}
 		}
 	}
 
-
-	private MockResponse getRequest(RecordedRequest request, byte[] body, String contentType) {
-		if(request.getMethod().equals("OPTIONS")) {
-			return new MockResponse().setResponseCode(200).setHeader("Allow", "GET, OPTIONS, HEAD, TRACE");
-		}
-		Buffer buf = new Buffer();
-		buf.write(body);
-		MockResponse response = new MockResponse()
-				.setHeader("Content-Length", body.length)
-				.setBody(buf)
-				.setResponseCode(200);
-		if (contentType != null) {
-			response = response.setHeader("Content-Type", contentType);
-		}
-		return response;
-	}
-
-	private MockResponse postRequest(RecordedRequest request, String expectedRequestContent,
-			String location, String contentType, byte[] responseBody) {
-
-		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) {
-			String charsetName = requestContentType.split("charset=")[1];
-			charset = Charset.forName(charsetName);
-		}
-		assertEquals("Invalid request body", expectedRequestContent, request.getBody().readString(charset));
-		Buffer buf = new Buffer();
-		buf.write(responseBody);
-		return new MockResponse()
-				.setHeader("Location", baseUrl + location)
-				.setHeader("Content-Type", contentType)
-				.setHeader("Content-Length", responseBody.length)
-				.setBody(buf)
-				.setResponseCode(201);
-	}
-
-	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"));
-		return new MockResponse()
-				.setHeader("Location", baseUrl + location)
-				.setHeader("Content-Type", contentType)
-				.setHeader("Content-Length", request.getBody().size())
-				.setBody(request.getBody())
-				.setResponseCode(201);
-	}
-
-	private MockResponse multipartRequest(RecordedRequest request) {
-		String contentType = request.getHeader("Content-Type");
-		assertTrue(contentType.startsWith("multipart/form-data"));
-		String boundary = contentType.split("boundary=")[1];
-		Buffer body = request.getBody();
-		try {
-			assertPart(body, "form-data", boundary, "name 1", "text/plain", "value 1");
-			assertPart(body, "form-data", boundary, "name 2", "text/plain", "value 2+1");
-			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);
-		}
-		return new MockResponse().setResponseCode(200);
-	}
-
-	private void assertPart(Buffer buffer, String disposition, String boundary, String name,
-			String contentType, String value) throws EOFException {
-
-		assertTrue(buffer.readUtf8Line().contains("--" + boundary));
-		String line = buffer.readUtf8Line();
-		assertTrue(line.contains("Content-Disposition: "+ disposition));
-		assertTrue(line.contains("name=\""+ name + "\""));
-		assertTrue(buffer.readUtf8Line().startsWith("Content-Type: "+contentType));
-		assertTrue(buffer.readUtf8Line().equals("Content-Length: " + value.length()));
-		assertTrue(buffer.readUtf8Line().equals(""));
-		assertTrue(buffer.readUtf8Line().equals(value));
-	}
-
-	private void assertFilePart(Buffer buffer, String disposition, String boundary, String name,
-			String filename, String contentType) throws EOFException {
-
-		assertTrue(buffer.readUtf8Line().contains("--" + boundary));
-		String line = buffer.readUtf8Line();
-		assertTrue(line.contains("Content-Disposition: "+ disposition));
-		assertTrue(line.contains("name=\""+ name + "\""));
-		assertTrue(line.contains("filename=\""+ filename + "\""));
-		assertTrue(buffer.readUtf8Line().startsWith("Content-Type: "+contentType));
-		assertTrue(buffer.readUtf8Line().startsWith("Content-Length: "));
-		assertTrue(buffer.readUtf8Line().equals(""));
-		assertNotNull(buffer.readUtf8Line());
-	}
-
-	private MockResponse formRequest(RecordedRequest request) {
-		assertEquals("application/x-www-form-urlencoded", request.getHeader("Content-Type"));
-		String body = request.getBody().readUtf8();
-		assertThat(body, Matchers.containsString("name+1=value+1"));
-		assertThat(body, Matchers.containsString("name+2=value+2%2B1"));
-		assertThat(body, Matchers.containsString("name+2=value+2%2B2"));
-		return new MockResponse().setResponseCode(200);
-	}
-
-	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) {
-			String charsetName = requestContentType.split("charset=")[1];
-			charset = Charset.forName(charsetName);
-		}
-		assertEquals("Invalid request body", expectedRequestContent, request.getBody().readString(charset));
-		Buffer buf = new Buffer();
-		buf.write(responseBody);
-		return new MockResponse().setResponseCode(201)
-				.setHeader("Content-Length", responseBody.length)
-				.setHeader("Content-Type", contentType)
-				.setBody(buf);
-	}
-
-	private MockResponse putRequest(RecordedRequest request, String expectedRequestContent) {
-		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) {
-			String charsetName = requestContentType.split("charset=")[1];
-			charset = Charset.forName(charsetName);
-		}
-		assertEquals("Invalid request body", expectedRequestContent, request.getBody().readString(charset));
-		return new MockResponse().setResponseCode(202);
-	}
-
 }

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.*;
-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;
 
 /**
  * @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 MyJacksonView2 {};
+	public interface MyJacksonView1 {}
+
+	public interface MyJacksonView2 {}
 
 
 	public static class MySampleBean {