DefaultResponseErrorHandler delegate methods declared as protected

Also revises copyToByteArray/String in FileCopyUtils/StreamUtils for lenient null handling.

Issue: SPR-15329

spring-core/src/main/java/org/springframework/util/FileCopyUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -60,8 +60,9 @@ public abstract class FileCopyUtils {
 	public static int copy(File in, File out) throws IOException {
 		Assert.notNull(in, "No input File specified");
 		Assert.notNull(out, "No output File specified");
+
 		return copy(new BufferedInputStream(new FileInputStream(in)),
-			new BufferedOutputStream(new FileOutputStream(out)));
+				new BufferedOutputStream(new FileOutputStream(out)));
 	}
 
 	/**
@@ -73,6 +74,7 @@ public abstract class FileCopyUtils {
 	public static void copy(byte[] in, File out) throws IOException {
 		Assert.notNull(in, "No input byte array specified");
 		Assert.notNull(out, "No output File specified");
+
 		ByteArrayInputStream inStream = new ByteArrayInputStream(in);
 		OutputStream outStream = new BufferedOutputStream(new FileOutputStream(out));
 		copy(inStream, outStream);
@@ -86,6 +88,7 @@ public abstract class FileCopyUtils {
 	 */
 	public static byte[] copyToByteArray(File in) throws IOException {
 		Assert.notNull(in, "No input File specified");
+
 		return copyToByteArray(new BufferedInputStream(new FileInputStream(in)));
 	}
 
@@ -105,6 +108,7 @@ public abstract class FileCopyUtils {
 	public static int copy(InputStream in, OutputStream out) throws IOException {
 		Assert.notNull(in, "No InputStream specified");
 		Assert.notNull(out, "No OutputStream specified");
+
 		try {
 			return StreamUtils.copy(in, out);
 		}
@@ -132,6 +136,7 @@ public abstract class FileCopyUtils {
 	public static void copy(byte[] in, OutputStream out) throws IOException {
 		Assert.notNull(in, "No input byte array specified");
 		Assert.notNull(out, "No OutputStream specified");
+
 		try {
 			out.write(in);
 		}
@@ -147,11 +152,15 @@ public abstract class FileCopyUtils {
 	/**
 	 * Copy the contents of the given InputStream into a new byte array.
 	 * Closes the stream when done.
-	 * @param in the stream to copy from
-	 * @return the new byte array that has been copied to
+	 * @param in the stream to copy from (may be {@code null} or empty)
+	 * @return the new byte array that has been copied to (possibly empty)
 	 * @throws IOException in case of I/O errors
 	 */
 	public static byte[] copyToByteArray(InputStream in) throws IOException {
+		if (in == null) {
+			return new byte[0];
+		}
+
 		ByteArrayOutputStream out = new ByteArrayOutputStream(BUFFER_SIZE);
 		copy(in, out);
 		return out.toByteArray();
@@ -173,6 +182,7 @@ public abstract class FileCopyUtils {
 	public static int copy(Reader in, Writer out) throws IOException {
 		Assert.notNull(in, "No Reader specified");
 		Assert.notNull(out, "No Writer specified");
+
 		try {
 			int byteCount = 0;
 			char[] buffer = new char[BUFFER_SIZE];
@@ -208,6 +218,7 @@ public abstract class FileCopyUtils {
 	public static void copy(String in, Writer out) throws IOException {
 		Assert.notNull(in, "No input String specified");
 		Assert.notNull(out, "No Writer specified");
+
 		try {
 			out.write(in);
 		}
@@ -223,11 +234,15 @@ public abstract class FileCopyUtils {
 	/**
 	 * Copy the contents of the given Reader into a String.
 	 * Closes the reader when done.
-	 * @param in the reader to copy from
-	 * @return the String that has been copied to
+	 * @param in the reader to copy from (may be {@code null} or empty)
+	 * @return the String that has been copied to (possibly empty)
 	 * @throws IOException in case of I/O errors
 	 */
 	public static String copyToString(Reader in) throws IOException {
+		if (in == null) {
+			return "";
+		}
+
 		StringWriter out = new StringWriter();
 		copy(in, out);
 		return out.toString();

spring-core/src/main/java/org/springframework/util/StreamUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -51,11 +51,15 @@ public abstract class StreamUtils {
 	/**
 	 * Copy the contents of the given InputStream into a new byte array.
 	 * Leaves the stream open when done.
-	 * @param in the stream to copy from
-	 * @return the new byte array that has been copied to
+	 * @param in the stream to copy from (may be {@code null} or empty)
+	 * @return the new byte array that has been copied to (possibly empty)
 	 * @throws IOException in case of I/O errors
 	 */
 	public static byte[] copyToByteArray(InputStream in) throws IOException {
+		if (in == null) {
+			return new byte[0];
+		}
+
 		ByteArrayOutputStream out = new ByteArrayOutputStream(BUFFER_SIZE);
 		copy(in, out);
 		return out.toByteArray();
@@ -64,12 +68,15 @@ public abstract class StreamUtils {
 	/**
 	 * Copy the contents of the given InputStream into a String.
 	 * Leaves the stream open when done.
-	 * @param in the InputStream to copy from
-	 * @return the String that has been copied to
+	 * @param in the InputStream to copy from (may be {@code null} or empty)
+	 * @return the String that has been copied to (possibly empty)
 	 * @throws IOException in case of I/O errors
 	 */
 	public static String copyToString(InputStream in, Charset charset) throws IOException {
-		Assert.notNull(in, "No InputStream specified");
+		if (in == null) {
+			return "";
+		}
+
 		StringBuilder out = new StringBuilder();
 		InputStreamReader reader = new InputStreamReader(in, charset);
 		char[] buffer = new char[BUFFER_SIZE];
@@ -90,6 +97,7 @@ public abstract class StreamUtils {
 	public static void copy(byte[] in, OutputStream out) throws IOException {
 		Assert.notNull(in, "No input byte array specified");
 		Assert.notNull(out, "No OutputStream specified");
+
 		out.write(in);
 	}
 
@@ -105,6 +113,7 @@ public abstract class StreamUtils {
 		Assert.notNull(in, "No input String specified");
 		Assert.notNull(charset, "No charset specified");
 		Assert.notNull(out, "No OutputStream specified");
+
 		Writer writer = new OutputStreamWriter(out, charset);
 		writer.write(in);
 		writer.flush();
@@ -121,6 +130,7 @@ public abstract class StreamUtils {
 	public static int copy(InputStream in, OutputStream out) throws IOException {
 		Assert.notNull(in, "No InputStream specified");
 		Assert.notNull(out, "No OutputStream specified");
+
 		int byteCount = 0;
 		byte[] buffer = new byte[BUFFER_SIZE];
 		int bytesRead = -1;
@@ -146,10 +156,14 @@ public abstract class StreamUtils {
 	 * @since 4.3
 	 */
 	public static long copyRange(InputStream in, OutputStream out, long start, long end) throws IOException {
+		Assert.notNull(in, "No InputStream specified");
+		Assert.notNull(out, "No OutputStream specified");
+
 		long skipped = in.skip(start);
 		if (skipped < start) {
-			throw new IOException("Skipped only " + skipped + " bytes out of " + start + " required.");
+			throw new IOException("Skipped only " + skipped + " bytes out of " + start + " required");
 		}
+
 		long bytesToCopy = end - start + 1;
 		byte buffer[] = new byte[StreamUtils.BUFFER_SIZE];
 		while (bytesToCopy > 0) {
@@ -166,7 +180,7 @@ public abstract class StreamUtils {
 				bytesToCopy = 0;
 			}
 		}
-		return end - start + 1 - bytesToCopy;
+		return (end - start + 1 - bytesToCopy);
 	}
 
 	/**
@@ -248,4 +262,5 @@ public abstract class StreamUtils {
 		public void close() throws IOException {
 		}
 	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -17,7 +17,6 @@
 package org.springframework.web.client;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.nio.charset.Charset;
 
 import org.springframework.http.HttpHeaders;
@@ -27,13 +26,12 @@ import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.util.FileCopyUtils;
 
 /**
- * Default implementation of the {@link ResponseErrorHandler} interface.
+ * Spring's default implementation of the {@link ResponseErrorHandler} interface.
  *
- * <p>This error handler checks for the status code on the {@link ClientHttpResponse}: any
- * code with series {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR} or
+ * <p>This error handler checks for the status code on the {@link ClientHttpResponse}:
+ * Any code with series {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR} or
  * {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR} is considered to be an
- * error. This behavior can be changed by overriding the {@link #hasError(HttpStatus)}
- * method.
+ * error. This behavior can be changed by overriding the {@link #hasError(HttpStatus)} method.
  *
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
@@ -50,16 +48,45 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 		return hasError(getHttpStatusCode(response));
 	}
 
-	private HttpStatus getHttpStatusCode(ClientHttpResponse response) throws IOException {
-		HttpStatus statusCode;
+	/**
+	 * This default implementation throws a {@link HttpClientErrorException} if the response status code
+	 * is {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR}, a {@link HttpServerErrorException}
+	 * if it is {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR},
+	 * and a {@link RestClientException} in other cases.
+	 */
+	@Override
+	public void handleError(ClientHttpResponse response) throws IOException {
+		HttpStatus statusCode = getHttpStatusCode(response);
+		switch (statusCode.series()) {
+			case CLIENT_ERROR:
+				throw new HttpClientErrorException(statusCode, response.getStatusText(),
+						response.getHeaders(), getResponseBody(response), getCharset(response));
+			case SERVER_ERROR:
+				throw new HttpServerErrorException(statusCode, response.getStatusText(),
+						response.getHeaders(), getResponseBody(response), getCharset(response));
+			default:
+				throw new RestClientException("Unknown status code [" + statusCode + "]");
+		}
+	}
+
+
+	/**
+	 * Determine the HTTP status of the given response.
+	 * @param response the response to inspect
+	 * @return the associated HTTP status
+	 * @throws IOException in case of I/O errors
+	 * @throws UnknownHttpStatusCodeException in case of an unknown status code
+	 * that cannot be represented with the {@link HttpStatus} enum
+	 * @since 4.3.8
+	 */
+	protected HttpStatus getHttpStatusCode(ClientHttpResponse response) throws IOException {
 		try {
-			statusCode = response.getStatusCode();
+			return response.getStatusCode();
 		}
 		catch (IllegalArgumentException ex) {
-			throw new UnknownHttpStatusCodeException(response.getRawStatusCode(),
-					response.getStatusText(), response.getHeaders(), getResponseBody(response), getCharset(response));
+			throw new UnknownHttpStatusCodeException(response.getRawStatusCode(), response.getStatusText(),
+					response.getHeaders(), getResponseBody(response), getCharset(response));
 		}
-		return statusCode;
 	}
 
 	/**
@@ -70,6 +97,7 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	 * Can be overridden in subclasses.
 	 * @param statusCode the HTTP status code
 	 * @return {@code true} if the response has an error; {@code false} otherwise
+	 * @see #getHttpStatusCode(ClientHttpResponse)
 	 */
 	protected boolean hasError(HttpStatus statusCode) {
 		return (statusCode.series() == HttpStatus.Series.CLIENT_ERROR ||
@@ -77,32 +105,27 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 	}
 
 	/**
-	 * This default implementation throws a {@link HttpClientErrorException} if the response status code
-	 * is {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR}, a {@link HttpServerErrorException}
-	 * if it is {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR},
-	 * and a {@link RestClientException} in other cases.
+	 * Determine the charset of the response (for inclusion in a status exception).
+	 * @param response the response to inspect
+	 * @return the associated charset, or {@code null} if none
+	 * @since 4.3.8
 	 */
-	@Override
-	public void handleError(ClientHttpResponse response) throws IOException {
-		HttpStatus statusCode = getHttpStatusCode(response);
-		switch (statusCode.series()) {
-			case CLIENT_ERROR:
-				throw new HttpClientErrorException(statusCode, response.getStatusText(),
-						response.getHeaders(), getResponseBody(response), getCharset(response));
-			case SERVER_ERROR:
-				throw new HttpServerErrorException(statusCode, response.getStatusText(),
-						response.getHeaders(), getResponseBody(response), getCharset(response));
-			default:
-				throw new RestClientException("Unknown status code [" + statusCode + "]");
-		}
+	protected Charset getCharset(ClientHttpResponse response) {
+		HttpHeaders headers = response.getHeaders();
+		MediaType contentType = headers.getContentType();
+		return (contentType != null ? contentType.getCharset() : null);
 	}
 
-	private byte[] getResponseBody(ClientHttpResponse response) {
+	/**
+	 * Read the body of the given response (for inclusion in a status exception).
+	 * @param response the response to inspect
+	 * @return the response body as a byte array,
+	 * or an empty byte array if the body could not be read
+	 * @since 4.3.8
+	 */
+	protected byte[] getResponseBody(ClientHttpResponse response) {
 		try {
-			InputStream responseBody = response.getBody();
-			if (responseBody != null) {
-				return FileCopyUtils.copyToByteArray(responseBody);
-			}
+			return FileCopyUtils.copyToByteArray(response.getBody());
 		}
 		catch (IOException ex) {
 			// ignore
@@ -110,10 +133,4 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 		return new byte[0];
 	}
 
-	private Charset getCharset(ClientHttpResponse response) {
-		HttpHeaders headers = response.getHeaders();
-		MediaType contentType = headers.getContentType();
-		return contentType != null ? contentType.getCharset() : null;
-	}
-
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -19,7 +19,6 @@ package org.springframework.web.client;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.http.HttpHeaders;
@@ -37,15 +36,10 @@ import static org.mockito.BDDMockito.*;
  */
 public class DefaultResponseErrorHandlerTests {
 
-	private DefaultResponseErrorHandler handler;
+	private final DefaultResponseErrorHandler handler = new DefaultResponseErrorHandler();
 
-	private ClientHttpResponse response;
+	private final ClientHttpResponse response = mock(ClientHttpResponse.class);
 
-	@Before
-	public void setUp() throws Exception {
-		handler = new DefaultResponseErrorHandler();
-		response = mock(ClientHttpResponse.class);
-	}
 
 	@Test
 	public void hasErrorTrue() throws Exception {
@@ -103,9 +97,7 @@ public class DefaultResponseErrorHandlerTests {
 		handler.handleError(response);
 	}
 
-	// SPR-9406
-
-	@Test(expected = UnknownHttpStatusCodeException.class)
+	@Test(expected = UnknownHttpStatusCodeException.class)  // SPR-9406
 	public void unknownStatusCode() throws Exception {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.TEXT_PLAIN);
@@ -117,4 +109,5 @@ public class DefaultResponseErrorHandlerTests {
 
 		handler.handleError(response);
 	}
+
 }