Ensure HTTP classes don't close streams

Prior to this commit several HTTP classes made use of FileCopyUtils
when reading from or writing to streams. This has the unfortunate
side effect of closing streams that should really be left open.

The problem is particularly noticeable when dealing with a
FormHttpMessageConverter that is writing a multi-part response.

Relevant HTTP classes have now been refactored to make use of a new
StreamUtils class that works in a similar way FileCopyUtils but does
not close streams.

The NonClosingOutputStream class from SimpleStreamingClientHttpRequest
has also been refactored to a StreamUtils method.

Issue: SPR-10095

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -31,19 +31,19 @@
 import java.io.Writer;
 
 /**
- * Simple utility methods for file and stream copying.
- * All copy methods use a block size of 4096 bytes,
- * and close all affected streams when done.
+ * Simple utility methods for file and stream copying. All copy methods use a block size
+ * of 4096 bytes, and close all affected streams when done. A variation of the copy
+ * methods from this class that leave streams open can be found in {@link StreamUtils}.
  *
- * <p>Mainly for use within the framework,
- * but also useful for application code.
+ * <p>Mainly for use within the framework, but also useful for application code.
  *
  * @author Juergen Hoeller
  * @since 06.10.2003
+ * @see StreamUtils
  */
 public abstract class FileCopyUtils {
 
-	public static final int BUFFER_SIZE = 4096;
+	public static final int BUFFER_SIZE = StreamUtils.BUFFER_SIZE;
 
 
 	//---------------------------------------------------------------------
@@ -106,15 +106,7 @@ public static int copy(InputStream in, OutputStream out) throws IOException {
 		Assert.notNull(in, "No InputStream specified");
 		Assert.notNull(out, "No OutputStream specified");
 		try {
-			int byteCount = 0;
-			byte[] buffer = new byte[BUFFER_SIZE];
-			int bytesRead = -1;
-			while ((bytesRead = in.read(buffer)) != -1) {
-				out.write(buffer, 0, bytesRead);
-				byteCount += bytesRead;
-			}
-			out.flush();
-			return byteCount;
+			return StreamUtils.copy(in, out);
 		}
 		finally {
 			try {
@@ -208,7 +200,7 @@ public static int copy(Reader in, Writer out) throws IOException {
 
 	/**
 	 * Copy the contents of the given String to the given output Writer.
-	 * Closes the write when done.
+	 * Closes the writer when done.
 	 * @param in the String to copy from
 	 * @param out the Writer to copy to
 	 * @throws IOException in case of I/O errors

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

@@ -0,0 +1,183 @@
+/*
+ * Copyright 2002-2013 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.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.util;
+
+import java.io.ByteArrayOutputStream;
+import java.io.FilterInputStream;
+import java.io.FilterOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.Charset;
+
+
+/**
+ * Simple utility methods for dealing with streams. The copy methods of this class are
+ * similar to those defined in {@link FileCopyUtils} except that all affected streams are
+ * left open when done. All copy methods use a block size of 4096 bytes.
+ *
+ * <p>Mainly for use within the framework, but also useful for application code.
+ *
+ * @author Juergen Hoeller
+ * @author Phillip Webb
+ * @since 3.2
+ * @see FileCopyUtils
+ */
+public abstract class StreamUtils {
+
+	public static final int BUFFER_SIZE = 4096;
+
+
+	/**
+	 * 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
+	 * @throws IOException in case of I/O errors
+	 */
+	public static byte[] copyToByteArray(InputStream in) throws IOException {
+		ByteArrayOutputStream out = new ByteArrayOutputStream(BUFFER_SIZE);
+		copy(in, out);
+		return out.toByteArray();
+	}
+
+	/**
+	 * 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
+	 * @throws IOException in case of I/O errors
+	 */
+	public static String copyToString(InputStream in, Charset charset) throws IOException {
+		Assert.notNull(in, "No InputStream specified");
+		StringBuilder out = new StringBuilder();
+		InputStreamReader reader = new InputStreamReader(in, charset);
+		char[] buffer = new char[BUFFER_SIZE];
+		int bytesRead = -1;
+		while ((bytesRead = reader.read(buffer)) != -1) {
+			out.append(buffer, 0, bytesRead);
+		}
+		return out.toString();
+	}
+
+	/**
+	 * Copy the contents of the given byte array to the given OutputStream.
+	 * Leaves the stream open when done.
+	 * @param in the byte array to copy from
+	 * @param out the OutputStream to copy to
+	 * @throws IOException in case of I/O errors
+	 */
+	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);
+	}
+
+	/**
+	 * Copy the contents of the given String to the given output OutputStream.
+	 * Leaves the stream open when done.
+	 * @param in the String to copy from
+	 * @param charset the Charset
+	 * @param out the OutputStream to copy to
+	 * @throws IOException in case of I/O errors
+	 */
+	public static void copy(String in, Charset charset, OutputStream out) throws IOException {
+		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();
+	}
+
+	/**
+	 * Copy the contents of the given InputStream to the given OutputStream.
+	 * Leaves both streams open when done.
+	 * @param in the InputStream to copy from
+	 * @param out the OutputStream to copy to
+	 * @return the number of bytes copied
+	 * @throws IOException in case of I/O errors
+	 */
+	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;
+		while ((bytesRead = in.read(buffer)) != -1) {
+			out.write(buffer, 0, bytesRead);
+			byteCount += bytesRead;
+		}
+		out.flush();
+		return byteCount;
+	}
+
+	/**
+	 * Returns a variant of the given {@link InputStream} where calling
+	 * {@link InputStream#close() close()} has no effect.
+	 * @param in the InputStream to decorate
+	 * @return a version of the InputStream that ignores calls to close
+	 */
+	public static InputStream nonClosing(InputStream in) {
+		Assert.notNull(in, "No InputStream specified");
+		return new NonClosingInputStream(in);
+	}
+
+	/**
+	 * Returns a variant of the given {@link OutputStream} where calling
+	 * {@link OutputStream#close() close()} has no effect.
+	 * @param in the OutputStream to decorate
+	 * @return a version of the OutputStream that ignores calls to close
+	 */
+	public static OutputStream nonClosing(OutputStream out) {
+		Assert.notNull(out, "No OutputStream specified");
+		return new NonClosingOutputStream(out);
+	}
+
+
+	private static class NonClosingInputStream extends FilterInputStream {
+
+		public NonClosingInputStream(InputStream in) {
+			super(in);
+		}
+
+		@Override
+		public void close() throws IOException {
+		}
+	}
+
+
+	private static class NonClosingOutputStream extends FilterOutputStream {
+
+		public NonClosingOutputStream(OutputStream out) {
+			super(out);
+		}
+
+		@Override
+		public void write(byte[] b, int off, int let) throws IOException {
+			// It is critical that we override this method for performance
+			out.write(b, off, let);
+		}
+
+		@Override
+		public void close() throws IOException {
+		}
+	}
+}

spring-core/src/test/java/org/springframework/util/StreamUtilsTests.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright 2002-2013 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.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.util;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.charset.Charset;
+import java.util.Random;
+import java.util.UUID;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InOrder;
+
+/**
+ * Tests for {@link StreamUtils}.
+ *
+ * @author Phillip Webb
+ */
+public class StreamUtilsTests {
+
+	private byte[] bytes = new byte[StreamUtils.BUFFER_SIZE + 10];
+
+	private String string = "";
+
+	@Before
+	public void setup() {
+		new Random().nextBytes(bytes);
+		while (string.length() < StreamUtils.BUFFER_SIZE + 10) {
+			string += UUID.randomUUID().toString();
+		}
+	}
+
+	@Test
+	public void copyToByteArray() throws Exception {
+		InputStream inputStream = spy(new ByteArrayInputStream(bytes));
+		byte[] actual = StreamUtils.copyToByteArray(inputStream);
+		assertThat(actual, equalTo(bytes));
+		verify(inputStream, never()).close();
+	}
+
+	@Test
+	public void copyToString() throws Exception {
+		Charset charset = Charset.defaultCharset();
+		InputStream inputStream = spy(new ByteArrayInputStream(string.getBytes(charset)));
+		String actual = StreamUtils.copyToString(inputStream, charset);
+		assertThat(actual, equalTo(string));
+		verify(inputStream, never()).close();
+	}
+
+	@Test
+	public void copyBytes() throws Exception {
+		ByteArrayOutputStream out = spy(new ByteArrayOutputStream());
+		StreamUtils.copy(bytes, out);
+		assertThat(out.toByteArray(), equalTo(bytes));
+		verify(out, never()).close();
+	}
+
+	@Test
+	public void copyString() throws Exception {
+		Charset charset = Charset.defaultCharset();
+		ByteArrayOutputStream out = spy(new ByteArrayOutputStream());
+		StreamUtils.copy(string, charset, out);
+		assertThat(out.toByteArray(), equalTo(string.getBytes(charset)));
+		verify(out, never()).close();
+	}
+
+	@Test
+	public void copyStream() throws Exception {
+		ByteArrayOutputStream out = spy(new ByteArrayOutputStream());
+		StreamUtils.copy(new ByteArrayInputStream(bytes), out);
+		assertThat(out.toByteArray(), equalTo(bytes));
+		verify(out, never()).close();
+	}
+
+	@Test
+	public void nonClosingInputStream() throws Exception {
+		InputStream source = mock(InputStream.class);
+		InputStream nonClosing = StreamUtils.nonClosing(source);
+		nonClosing.read();
+		nonClosing.read(bytes);
+		nonClosing.read(bytes, 1, 2);
+		nonClosing.close();
+		InOrder ordered = inOrder(source);
+		ordered.verify(source).read();
+		ordered.verify(source).read(bytes, 0, bytes.length);
+		ordered.verify(source).read(bytes, 1, 2);
+		ordered.verify(source, never()).close();
+	}
+
+	@Test
+	public void nonClosingOutputStream() throws Exception {
+		OutputStream source = mock(OutputStream.class);
+		OutputStream nonClosing = StreamUtils.nonClosing(source);
+		nonClosing.write(1);
+		nonClosing.write(bytes);
+		nonClosing.write(bytes, 1, 2);
+		nonClosing.close();
+		InOrder ordered = inOrder(source);
+		ordered.verify(source).write(1);
+		ordered.verify(source).write(bytes, 0, bytes.length);
+		ordered.verify(source).write(bytes, 1, 2);
+		ordered.verify(source, never()).close();
+	}
+}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -23,7 +23,7 @@
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.util.Assert;
-import org.springframework.util.FileCopyUtils;
+import org.springframework.util.StreamUtils;
 
 /**
  * Simple implementation of {@link ClientHttpRequest} that wraps another request.
@@ -53,8 +53,7 @@ public URI getURI() {
 	@Override
 	protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] bufferedOutput) throws IOException {
 		this.request.getHeaders().putAll(headers);
-		OutputStream body = this.request.getBody();
-		FileCopyUtils.copy(bufferedOutput, body);
+		StreamUtils.copy(bufferedOutput, this.request.getBody());
 		ClientHttpResponse response = this.request.execute();
 		return new BufferingClientHttpResponseWrapper(response);
 	}