Catch EOFException in IntrospectingClientHttpResponse

Prior to this commit, the `IntrospectingClientHttpResponse` would try
and read the HTTP response stream in order to check for the presence of
a non-empty message body.

Developers reported that in some cases, an `EOFException` is thrown
instead of returning -1 from the `read()` method. This commit ensures
that this case is taken into account and that we report the response as
an empty body in these cases.

Closes gh-35361

spring-web/src/main/java/org/springframework/web/client/IntrospectingClientHttpResponse.java

@@ -16,6 +16,7 @@
 
 package org.springframework.web.client;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PushbackInputStream;
@@ -33,7 +34,6 @@ import org.springframework.lang.Nullable;
  * @author Brian Clozel
  * @author Rossen Stoyanchev
  * @since 4.1.5
- * @see <a href="https://tools.ietf.org/html/rfc7230#section-3.3.3">RFC 7230 Section 3.3.3</a>
  */
 class IntrospectingClientHttpResponse extends ClientHttpResponseDecorator {
 
@@ -47,14 +47,18 @@ class IntrospectingClientHttpResponse extends ClientHttpResponseDecorator {
 
 
 	/**
-	 * Indicates whether the response has a message body.
+	 * Indicates whether the response might have a message body.
 	 * <p>Implementation returns {@code false} for:
 	 * <ul>
 	 * <li>a response status of {@code 1XX}, {@code 204} or {@code 304}</li>
 	 * <li>a {@code Content-Length} header of {@code 0}</li>
 	 * </ul>
-	 * @return {@code true} if the response has a message body, {@code false} otherwise
+	 * <p>In other cases, the server could use a {@code Transfer-Encoding} header or just
+	 * write the body and close the response. Reading the message body is then the only way
+	 * to check for the presence of a body.
+	 * @return {@code true} if the response might have a message body, {@code false} otherwise
 	 * @throws IOException in case of I/O errors
+	 * @see <a href="https://tools.ietf.org/html/rfc7230#section-3.3.3">RFC 7230 Section 3.3.3</a>
 	 */
 	public boolean hasMessageBody() throws IOException {
 		HttpStatusCode statusCode = getStatusCode();
@@ -62,10 +66,7 @@ class IntrospectingClientHttpResponse extends ClientHttpResponseDecorator {
 				statusCode == HttpStatus.NOT_MODIFIED) {
 			return false;
 		}
-		if (getHeaders().getContentLength() == 0) {
-			return false;
-		}
-		return true;
+		return getHeaders().getContentLength() != 0;
 	}
 
 	/**
@@ -73,6 +74,7 @@ class IntrospectingClientHttpResponse extends ClientHttpResponseDecorator {
 	 * <p>Implementation tries to read the first bytes of the response stream:
 	 * <ul>
 	 * <li>if no bytes are available, the message body is empty</li>
+	 * <li>if an {@link EOFException} is thrown, the body is considered empty</li>
 	 * <li>otherwise it is not empty and the stream is reset to its start for further reading</li>
 	 * </ul>
 	 * @return {@code true} if the response has a zero-length message body, {@code false} otherwise
@@ -85,26 +87,31 @@ class IntrospectingClientHttpResponse extends ClientHttpResponseDecorator {
 		if (body == null) {
 			return true;
 		}
-		if (body.markSupported()) {
-			body.mark(1);
-			if (body.read() == -1) {
-				return true;
+		try {
+			if (body.markSupported()) {
+				body.mark(1);
+				if (body.read() == -1) {
+					return true;
+				}
+				else {
+					body.reset();
+					return false;
+				}
 			}
 			else {
-				body.reset();
-				return false;
+				this.pushbackInputStream = new PushbackInputStream(body);
+				int b = this.pushbackInputStream.read();
+				if (b == -1) {
+					return true;
+				}
+				else {
+					this.pushbackInputStream.unread(b);
+					return false;
+				}
 			}
 		}
-		else {
-			this.pushbackInputStream = new PushbackInputStream(body);
-			int b = this.pushbackInputStream.read();
-			if (b == -1) {
-				return true;
-			}
-			else {
-				this.pushbackInputStream.unread(b);
-				return false;
-			}
+		catch (EOFException exc) {
+			return true;
 		}
 	}
 

spring-web/src/test/java/org/springframework/web/client/IntrospectingClientHttpResponseTests.java

@@ -17,11 +17,18 @@
 package org.springframework.web.client;
 
 import java.io.ByteArrayInputStream;
+import java.io.EOFException;
 import java.io.InputStream;
+import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 
+import org.springframework.http.HttpStatus;
+import org.springframework.http.HttpStatusCode;
 import org.springframework.http.client.ClientHttpResponse;
+import org.springframework.web.testfixture.http.client.MockClientHttpResponse;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.BDDMockito.given;
@@ -30,27 +37,57 @@ import static org.mockito.Mockito.mock;
 /**
  * Tests for {@link IntrospectingClientHttpResponse}.
  *
- * @since 5.3.10
- * @author Yin-Jui Liao
+ * @author Brian Clozel
  */
 class IntrospectingClientHttpResponseTests {
 
-	private final ClientHttpResponse response = mock();
 
-	private final IntrospectingClientHttpResponse wrappedResponse = new IntrospectingClientHttpResponse(response);
+	@ParameterizedTest
+	@MethodSource("noBodyHttpStatus")
+	void noMessageBodyWhenStatus(HttpStatus status) throws Exception {
+		var response = new MockClientHttpResponse(new byte[0], status);
+		var wrapped = new IntrospectingClientHttpResponse(response);
 
+		assertThat(wrapped.hasMessageBody()).isFalse();
+	}
+
+	static Stream<HttpStatusCode> noBodyHttpStatus() {
+		return Stream.of(HttpStatus.NO_CONTENT, HttpStatus.EARLY_HINTS, HttpStatus.NOT_MODIFIED);
+	}
+
+	@Test
+	void noMessageBodyWhenContentLength0() throws Exception {
+		var response = new MockClientHttpResponse(new byte[0], HttpStatus.OK);
+		response.getHeaders().setContentLength(0);
+		var wrapped = new IntrospectingClientHttpResponse(response);
+
+		assertThat(wrapped.hasMessageBody()).isFalse();
+	}
 
 	@Test
-	void messageBodyDoesNotExist() throws Exception {
-		given(response.getBody()).willReturn(null);
-		assertThat(wrappedResponse.hasEmptyMessageBody()).isTrue();
+	void emptyMessageWhenNullInputStream() throws Exception {
+		ClientHttpResponse mockResponse = mock();
+		given(mockResponse.getBody()).willReturn(null);
+		var wrappedMock = new IntrospectingClientHttpResponse(mockResponse);
+		assertThat(wrappedMock.hasEmptyMessageBody()).isTrue();
 	}
 
 	@Test
 	void messageBodyExists() throws Exception {
-		InputStream stream = new ByteArrayInputStream("content".getBytes());
-		given(response.getBody()).willReturn(stream);
-		assertThat(wrappedResponse.hasEmptyMessageBody()).isFalse();
+		var stream = new ByteArrayInputStream("content".getBytes());
+		var response = new MockClientHttpResponse(stream, HttpStatus.OK);
+		var wrapped = new IntrospectingClientHttpResponse(response);
+		assertThat(wrapped.hasEmptyMessageBody()).isFalse();
+	}
+
+	@Test
+	void emptyMessageWhenEOFException() throws Exception {
+		ClientHttpResponse mockResponse = mock();
+		InputStream stream = mock();
+		given(mockResponse.getBody()).willReturn(stream);
+		given(stream.read()).willThrow(new EOFException());
+		var wrappedMock = new IntrospectingClientHttpResponse(mockResponse);
+		assertThat(wrappedMock.hasEmptyMessageBody()).isTrue();
 	}
 
 }