Reuse InputStream in ResourceRegionHttpMessageConverter

The converter now tries to keep reading from the same InputStream which
should be possible with ordered and non-overlapping regions. When
necessary the InputStream is re-opened.

Closes gh-24214

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -170,7 +170,7 @@ public abstract class StreamUtils {
 		}
 
 		long bytesToCopy = end - start + 1;
-		byte[] buffer = new byte[StreamUtils.BUFFER_SIZE];
+		byte[] buffer = new byte[(int) Math.min(StreamUtils.BUFFER_SIZE, bytesToCopy)];
 		while (bytesToCopy > 0) {
 			int bytesRead = in.read(buffer);
 			if (bytesRead == -1) {

spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java

@@ -179,11 +179,23 @@ public class ResourceRegionHttpMessageConverter extends AbstractGenericHttpMessa
 		responseHeaders.set(HttpHeaders.CONTENT_TYPE, "multipart/byteranges; boundary=" + boundaryString);
 		OutputStream out = outputMessage.getBody();
 
-		for (ResourceRegion region : resourceRegions) {
+		Resource resource = null;
-			long start = region.getPosition();
+		InputStream in = null;
-			long end = start + region.getCount() - 1;
+		long inputStreamPosition = 0;
-			InputStream in = region.getResource().getInputStream();
+
-			try {
+		try {
+			for (ResourceRegion region : resourceRegions) {
+				long start = region.getPosition() - inputStreamPosition;
+				if (start < 0 || resource != region.getResource()) {
+					if (in != null) {
+						in.close();
+					}
+					resource = region.getResource();
+					in = resource.getInputStream();
+					inputStreamPosition = 0;
+					start = region.getPosition();
+				}
+				long end = start + region.getCount() - 1;
 				// Writing MIME header.
 				println(out);
 				print(out, "--" + boundaryString);
@@ -193,20 +205,25 @@ public class ResourceRegionHttpMessageConverter extends AbstractGenericHttpMessa
 					println(out);
 				}
 				Long resourceLength = region.getResource().contentLength();
-				end = Math.min(end, resourceLength - 1);
+				end = Math.min(end, resourceLength - inputStreamPosition - 1);
-				print(out, "Content-Range: bytes " + start + '-' + end + '/' + resourceLength);
+				print(out, "Content-Range: bytes " +
+						region.getPosition() + '-' + (region.getPosition() + region.getCount() - 1) +
+						'/' + resourceLength);
 				println(out);
 				println(out);
 				// Printing content
 				StreamUtils.copyRange(in, out, start, end);
+				inputStreamPosition += (end + 1);
 			}
-			finally {
+		}
-				try {
+		finally {
+			try {
+				if (in != null) {
 					in.close();
 				}
-				catch (IOException ex) {
+			}
-					// ignore
+			catch (IOException ex) {
-				}
+				// ignore
 			}
 		}
 

spring-web/src/test/java/org/springframework/http/converter/ResourceRegionHttpMessageConverterTests.java

@@ -144,6 +144,45 @@ public class ResourceRegionHttpMessageConverterTests {
 		assertThat(ranges[15]).isEqualTo("resource content.");
 	}
 
+	@Test
+	public void partialContentMultipleByteRangesInRandomOrderAndOverlapping() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource body = new ClassPathResource("byterangeresource.txt", getClass());
+		List<HttpRange> rangeList = HttpRange.parseRanges("bytes=7-15,0-5,17-20,20-29");
+		List<ResourceRegion> regions = new ArrayList<>();
+		for(HttpRange range : rangeList) {
+			regions.add(range.toResourceRegion(body));
+		}
+
+		converter.write(regions, MediaType.TEXT_PLAIN, outputMessage);
+
+		HttpHeaders headers = outputMessage.getHeaders();
+		assertThat(headers.getContentType().toString()).startsWith("multipart/byteranges;boundary=");
+		String boundary = "--" + headers.getContentType().toString().substring(30);
+		String content = outputMessage.getBodyAsString(StandardCharsets.UTF_8);
+		String[] ranges = StringUtils.tokenizeToStringArray(content, "\r\n", false, true);
+
+		assertThat(ranges[0]).isEqualTo(boundary);
+		assertThat(ranges[1]).isEqualTo("Content-Type: text/plain");
+		assertThat(ranges[2]).isEqualTo("Content-Range: bytes 7-15/39");
+		assertThat(ranges[3]).isEqualTo("Framework");
+
+		assertThat(ranges[4]).isEqualTo(boundary);
+		assertThat(ranges[5]).isEqualTo("Content-Type: text/plain");
+		assertThat(ranges[6]).isEqualTo("Content-Range: bytes 0-5/39");
+		assertThat(ranges[7]).isEqualTo("Spring");
+
+		assertThat(ranges[8]).isEqualTo(boundary);
+		assertThat(ranges[9]).isEqualTo("Content-Type: text/plain");
+		assertThat(ranges[10]).isEqualTo("Content-Range: bytes 17-20/39");
+		assertThat(ranges[11]).isEqualTo("test");
+
+		assertThat(ranges[12]).isEqualTo(boundary);
+		assertThat(ranges[13]).isEqualTo("Content-Type: text/plain");
+		assertThat(ranges[14]).isEqualTo("Content-Range: bytes 20-29/39");
+		assertThat(ranges[15]).isEqualTo("t resource");
+	}
+
 	@Test // SPR-15041
 	public void applicationOctetStreamDefaultContentType() throws Exception {
 		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();