From 23fc6f6b1d9a77a5b391502572d137e3bdd7e5f4 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 15 Aug 2018 20:52:40 +0200 Subject: [PATCH] Drain JDK HTTP client response body in all cases Prior to this commit, when using the `SimpleClientHttpRequestFactory` as a driver for `RestTemplate`, the HTTP response body would only be drained if there was an attempt to read it in the first place. This commit ensures that, even if there's no attempt at reading the response body, it is properly drained when the response is closed to make sure that the connection is released in a proper state and can be put back in the connection pool for reuse. Issue: SPR-17181 --- .../http/client/SimpleClientHttpResponse.java | 15 ++++++++------- .../AbstractHttpRequestFactoryTestCase.java | 2 +- .../client/SimpleClientHttpResponseTests.java | 12 ++++++++++++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java index 70f169c2a5..22cf7b59c1 100644 --- a/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java @@ -91,14 +91,15 @@ final class SimpleClientHttpResponse extends AbstractClientHttpResponse { @Override public void close() { - if (this.responseStream != null) { - try { - StreamUtils.drain(this.responseStream); - this.responseStream.close(); - } - catch (Exception ex) { - // ignore + try { + if (this.responseStream == null) { + getBody(); } + StreamUtils.drain(this.responseStream); + this.responseStream.close(); + } + catch (Exception ex) { + // ignore } } diff --git a/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java b/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java index 954d0ed5b0..af862c971a 100644 --- a/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java +++ b/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java @@ -135,7 +135,7 @@ public abstract class AbstractHttpRequestFactoryTestCase extends AbstractMockWeb @Test(expected = UnsupportedOperationException.class) public void headersAfterExecute() throws Exception { - ClientHttpRequest request = factory.createRequest(new URI(baseUrl + "/echo"), HttpMethod.POST); + ClientHttpRequest request = factory.createRequest(new URI(baseUrl + "/status/ok"), HttpMethod.POST); request.getHeaders().add("MyHeader", "value"); byte[] body = "Hello World".getBytes("UTF-8"); diff --git a/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpResponseTests.java b/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpResponseTests.java index 9025b3d9ea..c4157f6389 100644 --- a/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpResponseTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpResponseTests.java @@ -106,6 +106,18 @@ public class SimpleClientHttpResponseTests { verify(is).close(); } + @Test // SPR-17181 + public void shouldDrainResponseEvenIfResponseNotRead() throws Exception { + TestByteArrayInputStream is = new TestByteArrayInputStream("SpringSpring".getBytes(StandardCharsets.UTF_8)); + given(this.connection.getErrorStream()).willReturn(null); + given(this.connection.getInputStream()).willReturn(is); + + this.response.close(); + assertThat(is.available(), is(0)); + assertTrue(is.isClosed()); + verify(this.connection, never()).disconnect(); + } + private static class TestByteArrayInputStream extends ByteArrayInputStream {