diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/resource/HttpContentRangeWriter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/resource/HttpContentRangeWriter.java index 08a5c3805c6..0676fc63b2c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/resource/HttpContentRangeWriter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/resource/HttpContentRangeWriter.java @@ -64,7 +64,7 @@ public class HttpContentRangeWriter if (channel instanceof SeekableByteChannel) { SeekableByteChannel seekableByteChannel = (SeekableByteChannel)channel; - return new SeekableByteChannelRangeWriter(seekableByteChannel); + return new SeekableByteChannelRangeWriter(seekableByteChannel, () -> (SeekableByteChannel)content.getReadableByteChannel()); } if (LOG.isDebugEnabled()) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java index 3388191c611..b43fb8a0a88 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java @@ -28,13 +28,29 @@ import org.eclipse.jetty.util.IO; public class SeekableByteChannelRangeWriter implements RangeWriter { - private final SeekableByteChannel channel; + public static final int NO_PROGRESS_LIMIT = 3; + + public interface ChannelSupplier + { + SeekableByteChannel newSeekableByteChannel() throws IOException; + } + + private final ChannelSupplier channelSupplier; private final int bufSize; private final ByteBuffer buffer; + private SeekableByteChannel channel; + private long pos; + private boolean defaultSeekMode = true; - public SeekableByteChannelRangeWriter(SeekableByteChannel seekableByteChannel) + public SeekableByteChannelRangeWriter(SeekableByteChannelRangeWriter.ChannelSupplier channelSupplier) { - this.channel = seekableByteChannel; + this(null, channelSupplier); + } + + public SeekableByteChannelRangeWriter(SeekableByteChannel initialChannel, SeekableByteChannelRangeWriter.ChannelSupplier channelSupplier) + { + this.channel = initialChannel; + this.channelSupplier = channelSupplier; this.bufSize = IO.bufferSize; this.buffer = BufferUtil.allocate(this.bufSize); } @@ -42,13 +58,16 @@ public class SeekableByteChannelRangeWriter implements RangeWriter @Override public void close() throws IOException { - this.channel.close(); + if (this.channel != null) + { + this.channel.close(); + } } @Override public void writeTo(OutputStream outputStream, long skipTo, long length) throws IOException { - this.channel.position(skipTo); + skipTo(skipTo); // copy from channel to output stream long readTotal = 0; @@ -61,6 +80,87 @@ public class SeekableByteChannelRangeWriter implements RangeWriter BufferUtil.flipToFlush(buffer, 0); BufferUtil.writeTo(buffer, outputStream); readTotal += readLen; + pos += readLen; + } + } + + private void skipTo(long skipTo) throws IOException + { + if (channel == null) + { + channel = channelSupplier.newSeekableByteChannel(); + pos = 0; + } + + if (defaultSeekMode) + { + try + { + if (channel.position() != skipTo) + { + channel.position(skipTo); + pos = skipTo; + return; + } + } + catch (UnsupportedOperationException e) + { + defaultSeekMode = false; + fallbackSkipTo(skipTo); + } + } + else + { + // Fallback mode + fallbackSkipTo(skipTo); + } + } + + private void fallbackSkipTo(long skipTo) throws IOException + { + if (skipTo < pos) + { + channel.close(); + channel = channelSupplier.newSeekableByteChannel(); + pos = 0; + } + + if (pos < skipTo) + { + long skipSoFar = pos; + long actualSkipped; + int noProgressLoopLimit = NO_PROGRESS_LIMIT; + // loop till we reach desired point, break out on lack of progress. + while (noProgressLoopLimit > 0 && skipSoFar < skipTo) + { + BufferUtil.clearToFill(buffer); + int len = (int)Math.min(bufSize, (skipTo - skipSoFar)); + buffer.limit(len); + actualSkipped = channel.read(buffer); + if (actualSkipped == 0) + { + noProgressLoopLimit--; + } + else if (actualSkipped > 0) + { + skipSoFar += actualSkipped; + noProgressLoopLimit = NO_PROGRESS_LIMIT; + } + else + { + // negative values means the stream was closed or reached EOF + // either way, we've hit a state where we can no longer + // fulfill the requested range write. + throw new IOException("EOF reached before SeekableByteChannel skip destination"); + } + } + + if (noProgressLoopLimit <= 0) + { + throw new IOException("No progress made to reach SeekableByteChannel skip position " + (skipTo - pos)); + } + + pos = skipTo; } } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/resource/RangeWriterTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/resource/RangeWriterTest.java index e073fa55734..8312ac038cd 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/resource/RangeWriterTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/resource/RangeWriterTest.java @@ -21,10 +21,16 @@ package org.eclipse.jetty.server.resource; import java.io.BufferedWriter; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.nio.channels.SeekableByteChannel; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.HashMap; +import java.util.Map; import java.util.stream.Stream; import org.eclipse.jetty.toolchain.test.FS; @@ -32,6 +38,7 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.Resource; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -43,6 +50,16 @@ import static org.hamcrest.Matchers.is; public class RangeWriterTest { public static final String DATA = "01234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWZYZ!@#$%^&*()_+/.,[]"; + private static FileSystem zipfs; + + @AfterAll + public static void closeZipFs() throws IOException + { + if (zipfs != null) + { + zipfs.close(); + } + } public static Path initDataFile() throws IOException { @@ -59,21 +76,47 @@ public class RangeWriterTest return dataFile; } - public static Stream impls() throws IOException + private static Path initZipFsDataFile() throws URISyntaxException, IOException { - Resource resource = new PathResource(initDataFile()); + Path exampleJar = MavenTestingUtils.getTestResourcePathFile("example.jar"); + + URI uri = new URI("jar", exampleJar.toUri().toASCIIString(), null); + + Map env = new HashMap<>(); + env.put("multi-release", "runtime"); + + if (zipfs != null) + { + // close prior one + zipfs.close(); + } + + zipfs = FileSystems.newFileSystem(uri, env); + Path rootPath = zipfs.getRootDirectories().iterator().next(); + return rootPath.resolve("data.dat"); + } + + public static Stream impls() throws IOException, URISyntaxException + { + Resource realFileSystemResource = new PathResource(initDataFile()); + Resource nonDefaultFileSystemResource = new PathResource(initZipFsDataFile()); return Stream.of( - Arguments.of(new ByteBufferRangeWriter(BufferUtil.toBuffer(resource, true))), - Arguments.of(new ByteBufferRangeWriter(BufferUtil.toBuffer(resource, false))), - Arguments.of(new SeekableByteChannelRangeWriter((SeekableByteChannel)resource.getReadableByteChannel())), - Arguments.of(new InputStreamRangeWriter(() -> resource.getInputStream())) + Arguments.of("Traditional / Direct Buffer", new ByteBufferRangeWriter(BufferUtil.toBuffer(realFileSystemResource, true))), + Arguments.of("Traditional / Indirect Buffer", new ByteBufferRangeWriter(BufferUtil.toBuffer(realFileSystemResource, false))), + Arguments.of("Traditional / SeekableByteChannel", new SeekableByteChannelRangeWriter(() -> (SeekableByteChannel)realFileSystemResource.getReadableByteChannel())), + Arguments.of("Traditional / InputStream", new InputStreamRangeWriter(() -> realFileSystemResource.getInputStream())), + + Arguments.of("Non-Default FS / Direct Buffer", new ByteBufferRangeWriter(BufferUtil.toBuffer(nonDefaultFileSystemResource, true))), + Arguments.of("Non-Default FS / Indirect Buffer", new ByteBufferRangeWriter(BufferUtil.toBuffer(nonDefaultFileSystemResource, false))), + Arguments.of("Non-Default FS / SeekableByteChannel", new SeekableByteChannelRangeWriter(() -> (SeekableByteChannel)nonDefaultFileSystemResource.getReadableByteChannel())), + Arguments.of("Non-Default FS / InputStream", new InputStreamRangeWriter(() -> nonDefaultFileSystemResource.getInputStream())) ); } - @ParameterizedTest + @ParameterizedTest(name = "[{index}] {0}") @MethodSource("impls") - public void testSimpleRange(RangeWriter rangeWriter) throws IOException + public void testSimpleRange(String description, RangeWriter rangeWriter) throws IOException { ByteArrayOutputStream outputStream; @@ -82,9 +125,9 @@ public class RangeWriterTest assertThat("Range: 10 (len=50)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 60))); } - @ParameterizedTest + @ParameterizedTest(name = "[{index}] {0}") @MethodSource("impls") - public void testSameRange_MultipleTimes(RangeWriter rangeWriter) throws IOException + public void testSameRange_MultipleTimes(String description, RangeWriter rangeWriter) throws IOException { ByteArrayOutputStream outputStream; @@ -97,9 +140,9 @@ public class RangeWriterTest assertThat("Range(b): 10 (len=50)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 60))); } - @ParameterizedTest + @ParameterizedTest(name = "[{index}] {0}") @MethodSource("impls") - public void testMultipleRanges_Ordered(RangeWriter rangeWriter) throws IOException + public void testMultipleRanges_Ordered(String description, RangeWriter rangeWriter) throws IOException { ByteArrayOutputStream outputStream; @@ -116,9 +159,9 @@ public class RangeWriterTest assertThat("Range(b): 55 (len=10)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(55, 55 + 10))); } - @ParameterizedTest + @ParameterizedTest(name = "[{index}] {0}") @MethodSource("impls") - public void testMultipleRanges_Overlapping(RangeWriter rangeWriter) throws IOException + public void testMultipleRanges_Overlapping(String description, RangeWriter rangeWriter) throws IOException { ByteArrayOutputStream outputStream; @@ -135,9 +178,9 @@ public class RangeWriterTest assertThat("Range(b): 20 (len=20)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(20, 20 + 20))); } - @ParameterizedTest + @ParameterizedTest(name = "[{index}] {0}") @MethodSource("impls") - public void testMultipleRanges_ReverseOrder(RangeWriter rangeWriter) throws IOException + public void testMultipleRanges_ReverseOrder(String description, RangeWriter rangeWriter) throws IOException { ByteArrayOutputStream outputStream; diff --git a/jetty-server/src/test/resources/example.jar b/jetty-server/src/test/resources/example.jar new file mode 100644 index 00000000000..cf15a241bf0 Binary files /dev/null and b/jetty-server/src/test/resources/example.jar differ diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 2f1fcc5d669..92b866876fc 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -388,7 +388,6 @@ public class PathResource extends Resource @Override public InputStream getInputStream() throws IOException { - // TODO: investigate if SPARSE use for default FileSystem usages is worth it return Files.newInputStream(path, StandardOpenOption.READ); } @@ -401,7 +400,11 @@ public class PathResource extends Resource @Override public ReadableByteChannel getReadableByteChannel() throws IOException { - // TODO: investigate if SPARSE use for default FileSystem usages is worth it + return newSeekableByteChannel(); + } + + public SeekableByteChannel newSeekableByteChannel() throws IOException + { return Files.newByteChannel(path, StandardOpenOption.READ); } @@ -588,7 +591,7 @@ public class PathResource extends Resource try (SeekableByteChannel channel = Files.newByteChannel(path, StandardOpenOption.READ)) { ByteBuffer buffer = BufferUtil.allocate(IO.bufferSize); - channel.position(start); + skipTo(channel, buffer, start); // copy from channel to output stream long readTotal = 0; @@ -605,6 +608,57 @@ public class PathResource extends Resource } } + private void skipTo(SeekableByteChannel channel, ByteBuffer buffer, long skipTo) throws IOException + { + try + { + if (channel.position() != skipTo) + { + channel.position(skipTo); + } + } + catch (UnsupportedOperationException e) + { + final int NO_PROGRESS_LIMIT = 3; + + if (skipTo > 0) + { + long pos = 0; + long readLen; + int noProgressLoopLimit = NO_PROGRESS_LIMIT; + // loop till we reach desired point, break out on lack of progress. + while (noProgressLoopLimit > 0 && pos < skipTo) + { + BufferUtil.clearToFill(buffer); + int len = (int)Math.min(IO.bufferSize, (skipTo - pos)); + buffer.limit(len); + readLen = channel.read(buffer); + if (readLen == 0) + { + noProgressLoopLimit--; + } + else if (readLen > 0) + { + pos += readLen; + noProgressLoopLimit = NO_PROGRESS_LIMIT; + } + else + { + // negative values means the stream was closed or reached EOF + // either way, we've hit a state where we can no longer + // fulfill the requested range write. + throw new IOException("EOF reached before SeekableByteChannel skip destination"); + } + } + + if (noProgressLoopLimit <= 0) + { + throw new IOException("No progress made to reach SeekableByteChannel skip position " + skipTo); + } + } + } + } + @Override public String toString() { diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 50a8693c95b..e48297480cf 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.util.resource; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -33,6 +34,7 @@ import java.util.Map; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.junit.jupiter.api.Test; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -46,7 +48,6 @@ public class PathResourceTest Path exampleJar = MavenTestingUtils.getTestResourcePathFile("example.jar"); URI uri = new URI("jar", exampleJar.toUri().toASCIIString(), null); - System.err.println("URI = " + uri); Map env = new HashMap<>(); env.put("multi-release", "runtime"); @@ -71,7 +72,6 @@ public class PathResourceTest Path exampleJar = MavenTestingUtils.getTestResourcePathFile("example.jar"); URI uri = new URI("jar", exampleJar.toUri().toASCIIString(), null); - System.err.println("URI = " + uri); Map env = new HashMap<>(); env.put("multi-release", "runtime"); @@ -96,7 +96,6 @@ public class PathResourceTest Path exampleJar = MavenTestingUtils.getTestResourcePathFile("example.jar"); URI uri = new URI("jar", exampleJar.toUri().toASCIIString(), null); - System.err.println("URI = " + uri); Map env = new HashMap<>(); env.put("multi-release", "runtime"); @@ -112,6 +111,32 @@ public class PathResourceTest } } + @Test + public void testNonDefaultFileSystem_WriteTo() throws URISyntaxException, IOException + { + Path exampleJar = MavenTestingUtils.getTestResourcePathFile("example.jar"); + + URI uri = new URI("jar", exampleJar.toUri().toASCIIString(), null); + + Map env = new HashMap<>(); + env.put("multi-release", "runtime"); + + try (FileSystem zipfs = FileSystems.newFileSystem(uri, env)) + { + Path manifestPath = zipfs.getPath("/META-INF/MANIFEST.MF"); + assertThat(manifestPath, is(not(nullValue()))); + + PathResource resource = new PathResource(manifestPath); + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) + { + resource.writeTo(out, 2, 10); + String actual = new String(out.toByteArray(), UTF_8); + String expected = "nifest-Ver"; + assertThat("writeTo(out, 2, 10)", actual, is(expected)); + } + } + } + @Test public void testDefaultFileSystem_GetFile() throws Exception {