From 5efe7a2eea434093ae955dd7e4c1725fba60dfc9 Mon Sep 17 00:00:00 2001 From: Randomnicode Date: Sat, 14 Dec 2019 10:45:33 -0800 Subject: [PATCH 1/2] Reuse InputStream for ResourceRegionHttpMessageConverter Dynamically generated input streams cannot be closed and reopened multiple times. This fix reuses the same InputStream across multiple ResourceRegion writes. This is a proposal for and closes #24210 --- .../ResourceRegionHttpMessageConverter.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java index be98db9122a1..39a74e8b5f2b 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java @@ -22,7 +22,11 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourceRegion; @@ -178,12 +182,28 @@ private void writeResourceRegionCollection(Collection resourceRe String boundaryString = MimeTypeUtils.generateMultipartBoundaryString(); responseHeaders.set(HttpHeaders.CONTENT_TYPE, "multipart/byteranges; boundary=" + boundaryString); OutputStream out = outputMessage.getBody(); + // Allows reuse of streams + Map cleanup = new HashMap<>(); + List exs = new ArrayList<>(); + + try { + for (ResourceRegion region : resourceRegions) { + long start = region.getPosition(); + long end = start + region.getCount() - 1; + + InputStream in = cleanup.computeIfAbsent(region.getResource(), r -> { + try { + return r.getInputStream(); + } catch (IOException e) { + exs.add(e); + return null; + } + }); + + if (!exs.isEmpty()) { + throw exs.get(0); + } - for (ResourceRegion region : resourceRegions) { - long start = region.getPosition(); - long end = start + region.getCount() - 1; - InputStream in = region.getResource().getInputStream(); - try { // Writing MIME header. println(out); print(out, "--" + boundaryString); @@ -200,11 +220,11 @@ private void writeResourceRegionCollection(Collection resourceRe // Printing content StreamUtils.copyRange(in, out, start, end); } - finally { + } finally { + for (InputStream in : cleanup) { try { in.close(); - } - catch (IOException ex) { + } catch (IOException ex) { // ignore } } From a3c965782d5f4c9f602574b15b460b54d54ce32e Mon Sep 17 00:00:00 2001 From: Randomnicode Date: Mon, 16 Dec 2019 15:51:10 -0800 Subject: [PATCH 2/2] Remember stream position and account for out-of-order ranges --- .../ResourceRegionHttpMessageConverter.java | 48 +++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java index 39a74e8b5f2b..ac2b3c45f0fc 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java @@ -22,11 +22,13 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; +import java.util.AbstractMap.SimpleEntry; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourceRegion; @@ -183,17 +185,15 @@ private void writeResourceRegionCollection(Collection resourceRe responseHeaders.set(HttpHeaders.CONTENT_TYPE, "multipart/byteranges; boundary=" + boundaryString); OutputStream out = outputMessage.getBody(); // Allows reuse of streams - Map cleanup = new HashMap<>(); + Map> currStreams = new HashMap<>(); List exs = new ArrayList<>(); try { for (ResourceRegion region : resourceRegions) { - long start = region.getPosition(); - long end = start + region.getCount() - 1; - - InputStream in = cleanup.computeIfAbsent(region.getResource(), r -> { + // retrieve an existing stream or generate a new one + Entry input = currStreams.computeIfAbsent(region.getResource(), r -> { try { - return r.getInputStream(); + return new SimpleEntry<>(r.getInputStream(), 0L); } catch (IOException e) { exs.add(e); return null; @@ -204,6 +204,33 @@ private void writeResourceRegionCollection(Collection resourceRe throw exs.get(0); } + // offset the existing stream location from the byte start so appropriate number of bytes are skipped + long start = region.getPosition() - input.getValue(); + + // check if range is out of order (stream pointer has advanced beyond what the range is requesting) + if (start < 0) { + // close existing + input.getKey().close(); + // open new stream + input = currStreams.computeIfPresent(region.getResource(), (r, i) -> { + try { + return new SimpleEntry<>(r.getInputStream(), 0L); + } catch (IOException e) { + exs.add(e); + return null; + } + }); + + if (!exs.isEmpty()) { + throw exs.get(0); + } + + // reset start + start = region.getPosition(); + } + + long end = start + region.getCount() - 1; + // Writing MIME header. println(out); print(out, "--" + boundaryString); @@ -218,16 +245,19 @@ private void writeResourceRegionCollection(Collection resourceRe println(out); println(out); // Printing content - StreamUtils.copyRange(in, out, start, end); + StreamUtils.copyRange(input.getKey(), out, start, end); + + //set stream position to reuse for next time + input.setValue(end + 1); } } finally { - for (InputStream in : cleanup) { + currStreams.values().stream().map(e -> e.getKey()).forEach(in -> { try { in.close(); } catch (IOException ex) { // ignore } - } + }); } println(out);