Revisit empty body response support in HTTP client

Prior to this commit, HTTP responses without body (response status 204
or 304, Content-Length: 0) were handled properly by RestTemplates. But
some other cases were not properly managed, throwing exceptions for
valid HTTP responses.

This commit better handles HTTP responses, using a response wrapper that
can tell if a response:

* has no message body (HTTP status 1XX, 204, 304 or Content-Length:0)
* has an empty message body

This covers rfc7230 Section 3.3.3.

Issue: SPR-8016

spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpResponseWrapper.java

@@ -25,7 +25,7 @@ import org.springframework.http.HttpStatus;
 import org.springframework.util.StreamUtils;
 
 /**
- * Simple implementation of {@link ClientHttpResponse} that reads the request's body into memory,
+ * Simple implementation of {@link ClientHttpResponse} that reads the response's body into memory,
  * thus allowing for multiple invocations of {@link #getBody()}.
  *
  * @author Arjen Poutsma

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

@@ -23,7 +23,6 @@ import java.util.List;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.client.ClientHttpResponse;
@@ -80,10 +79,12 @@ public class HttpMessageConverterExtractor<T> implements ResponseExtractor<T> {
 	@Override
 	@SuppressWarnings({ "unchecked", "rawtypes" })
 	public T extractData(ClientHttpResponse response) throws IOException {
-		if (!hasMessageBody(response)) {
+
+		MessageBodyClientHttpResponseWrapper responseWrapper = new MessageBodyClientHttpResponseWrapper(response);
+		if(!responseWrapper.hasMessageBody() || responseWrapper.hasEmptyMessageBody()) {
 			return null;
 		}
-		MediaType contentType = getContentType(response);
+		MediaType contentType = getContentType(responseWrapper);
 
 		for (HttpMessageConverter<?> messageConverter : this.messageConverters) {
 			if (messageConverter instanceof GenericHttpMessageConverter) {
@@ -93,7 +94,7 @@ public class HttpMessageConverterExtractor<T> implements ResponseExtractor<T> {
 						logger.debug("Reading [" + this.responseType + "] as \"" +
 								contentType + "\" using [" + messageConverter + "]");
 					}
-					return (T) genericMessageConverter.read(this.responseType, null, response);
+					return (T) genericMessageConverter.read(this.responseType, null, responseWrapper);
 				}
 			}
 			if (this.responseClass != null) {
@@ -102,7 +103,7 @@ public class HttpMessageConverterExtractor<T> implements ResponseExtractor<T> {
 						logger.debug("Reading [" + this.responseClass.getName() + "] as \"" +
 								contentType + "\" using [" + messageConverter + "]");
 					}
-					return (T) messageConverter.read((Class) this.responseClass, response);
+					return (T) messageConverter.read((Class) this.responseClass, responseWrapper);
 				}
 			}
 		}
@@ -122,39 +123,4 @@ public class HttpMessageConverterExtractor<T> implements ResponseExtractor<T> {
 		return contentType;
 	}
 
-	/**
-	 * Indicates whether the given response has a message body.
-	 * <p>Default implementation returns {@code false} for:
-	 * <ul>
-	 *     <li>a response status of {@code 204} or {@code 304}</li>
-	 *     <li>a {@code Content-Length} of {@code 0}</li>
-	 *     <li>no indication of content (no {@code Content-Length} nor {@code Transfer-encoding: chunked}) and
-	 *     a ({@code Connection: closed}) header. See rfc7230 section 3.4</li>
-	 * </ul>
-	 *
-	 * @param response the response to check for a message body
-	 * @return {@code true} if the response has a body, {@code false} otherwise
-	 * @throws IOException in case of I/O errors
-	 */
-	protected boolean hasMessageBody(ClientHttpResponse response) throws IOException {
-		HttpStatus responseStatus = response.getStatusCode();
-		if (responseStatus == HttpStatus.NO_CONTENT ||
-				responseStatus == HttpStatus.NOT_MODIFIED) {
-			return false;
-		}
-		HttpHeaders headers = response.getHeaders();
-		long contentLength = headers.getContentLength();
-		if(contentLength == 0) {
-			return false;
-		}
-		boolean chunked = headers.containsKey(HttpHeaders.TRANSFER_ENCODING)
-				&& headers.get(HttpHeaders.TRANSFER_ENCODING).contains("chunked");
-		boolean closed = headers.containsKey(HttpHeaders.CONNECTION)
-				&& headers.getConnection().contains("close");
-		if(!chunked && contentLength == -1 && closed) {
-			return false;
-		}
-		return true;
-	}
-
 }

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

@@ -0,0 +1,122 @@
+package org.springframework.web.client;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.client.ClientHttpResponse;
+
+/**
+ * Implementation of {@link ClientHttpResponse} that can not only check if the response
+ * has a message body, but also if its length is 0 (i.e. empty) by actually reading the input stream.
+ *
+ * @author Brian Clozel
+ * @since 4.1
+ * @see <a href="http://tools.ietf.org/html/rfc7230#section-3.3.3">rfc7230 Section 3.3.3</a>
+ */
+class MessageBodyClientHttpResponseWrapper implements ClientHttpResponse {
+
+	private PushbackInputStream pushbackInputStream;
+
+	private final ClientHttpResponse response;
+
+	public MessageBodyClientHttpResponseWrapper(ClientHttpResponse response) throws IOException {
+		this.response = response;
+	}
+
+	/**
+	 * Indicates whether the response has 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
+	 * @throws IOException in case of I/O errors
+	 */
+	public boolean hasMessageBody() throws IOException {
+		HttpStatus responseStatus = this.getStatusCode();
+		if (responseStatus.is1xxInformational() || responseStatus == HttpStatus.NO_CONTENT ||
+				responseStatus == HttpStatus.NOT_MODIFIED) {
+			return false;
+		}
+		else if(this.getHeaders().getContentLength() == 0) {
+			return false;
+		}
+		return true;
+	}
+
+	/**
+	 * Indicates whether the response has an empty message body.
+	 *
+	 * <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>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
+	 * @throws IOException in case of I/O errors
+	 */
+	public boolean hasEmptyMessageBody() throws IOException {
+		InputStream body = this.response.getBody();
+		if (body == null) {
+			return true;
+		}
+		else if (body.markSupported()) {
+			body.mark(1);
+			if (body.read() == -1) {
+				return true;
+			}
+			else {
+				body.reset();
+				return false;
+			}
+		}
+		else {
+			this.pushbackInputStream = new PushbackInputStream(body);
+			int b = pushbackInputStream.read();
+			if (b == -1) {
+				return true;
+			}
+			else {
+				pushbackInputStream.unread(b);
+				return false;
+			}
+		}
+	}
+
+	@Override
+	public HttpStatus getStatusCode() throws IOException {
+		return response.getStatusCode();
+	}
+
+	@Override
+	public int getRawStatusCode() throws IOException {
+		return response.getRawStatusCode();
+	}
+
+	@Override
+	public String getStatusText() throws IOException {
+		return response.getStatusText();
+	}
+
+	@Override
+	public void close() {
+		response.close();
+	}
+
+	@Override
+	public InputStream getBody() throws IOException {
+		return this.pushbackInputStream != null ? this.pushbackInputStream : response.getBody();
+	}
+
+	@Override
+	public HttpHeaders getHeaders() {
+		return response.getHeaders();
+	}
+}