SimpleClientHttpResponse catches any Exception on close

jhoeller · jhoeller · commit d74a2730ec43 · 2018-05-02T16:07:22.000+02:00
Issue: SPR-16773 (cherry picked from commit 21fad8e)
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
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -96,7 +96,7 @@ public void close() {
 				StreamUtils.drain(this.responseStream);
 				this.responseStream.close();
 			}
-			catch (IOException ex) {
+			catch (Exception ex) {
 				// ignore
 			}
 		}
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
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -22,31 +22,25 @@
 import java.net.HttpURLConnection;
 import java.nio.charset.StandardCharsets;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.util.StreamUtils;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.BDDMockito.any;
 import static org.mockito.BDDMockito.*;
 
 /**
  * @author Brian Clozel
+ * @author Juergen Hoeller
  */
 public class SimpleClientHttpResponseTests {
 
-	private SimpleClientHttpResponse response;
+	private final HttpURLConnection connection = mock(HttpURLConnection.class);
 
-	private HttpURLConnection connection;
-
-
-	@Before
-	public void setup() throws Exception {
-		this.connection = mock(HttpURLConnection.class);
-		this.response = new SimpleClientHttpResponse(this.connection);
-	}
+	private final SimpleClientHttpResponse response = new SimpleClientHttpResponse(this.connection);
 
 
 	@Test  // SPR-14040
@@ -98,8 +92,22 @@ public void shouldDrainErrorStreamWhenResponseClosed() throws Exception {
 		verify(this.connection, never()).disconnect();
 	}
 
+	@Test  // SPR-16773
+	public void shouldNotDrainWhenErrorStreamClosed() throws Exception {
+		InputStream is = mock(InputStream.class);
+		given(this.connection.getErrorStream()).willReturn(is);
+		doNothing().when(is).close();
+		given(is.read(any())).willThrow(new NullPointerException("from HttpURLConnection#ErrorStream"));
+
+		InputStream responseStream = this.response.getBody();
+		responseStream.close();
+		this.response.close();
+
+		verify(is).close();
+	}
+
 
-	class TestByteArrayInputStream extends ByteArrayInputStream {
+	private static class TestByteArrayInputStream extends ByteArrayInputStream {
 
 		private boolean closed;
 

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2017 the original author or authors.`
	`2`	`+ * Copyright 2002-2018 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -96,7 +96,7 @@ public void close() {`
`96`	`96`	`StreamUtils.drain(this.responseStream);`
`97`	`97`	`this.responseStream.close();`
`98`	`98`	`}`
`99`		`- catch (IOException ex) {`
	`99`	`+ catch (Exception ex) {`
`100`	`100`	`// ignore`
`101`	`101`	`}`
`102`	`102`	`}`