diff --git a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java index 546fde34d11..b45d36e3295 100644 --- a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java +++ b/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> entry : headers.entrySet()) { String headerName = entry.getKey(); for (String headerValue : entry.getValue()) { builder.addHeader(headerName, headerValue); } } - return builder.build(); } diff --git a/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java index c0cf802a98f..ce9e8c39ad2 100644 --- a/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java +++ b/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> entry : headers.entrySet()) { String headerName = entry.getKey(); for (String headerValue : entry.getValue()) { builder.addHeader(headerName, headerValue); } } - return builder.build(); } diff --git a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java index 18493412e14..4c58bbb7aa2 100644 --- a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java +++ b/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()); + } + } + } + } diff --git a/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java b/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java index 93015c18292..3ab4d3734eb 100644 --- a/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java +++ b/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 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 {