#10330 - Fix broken EE9 DefaultServlet range requests

Signed-off-by: Ludovic Orban <lorban@bitronix.be>

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java

@@ -21,6 +21,7 @@ import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Enumeration;
@@ -232,9 +233,16 @@ public class ResourceService
             pathInfo = request.getPathInfo();
 
             // Is this a Range request?
-            reqRanges = request.getHeaders(HttpHeader.RANGE.asString());
+            if (_acceptRanges)
-            if (!hasDefinedRange(reqRanges))
+            {
-                reqRanges = null;
+                reqRanges = request.getHeaders(HttpHeader.RANGE.asString());
+                if (!hasDefinedRange(reqRanges))
+                    reqRanges = null;
+            }
+            else if (request.getHeader(HttpHeader.RANGE.asString()) != null)
+            {
+                response.setHeader(HttpHeader.ACCEPT_RANGES.asString(), "none");
+            }
         }
 
         String pathInContext = URIUtil.addPaths(servletPath, pathInfo);
@@ -864,28 +872,37 @@ public class ResourceService
 
     private static void writeContent(HttpContent content, OutputStream out, long start, long contentLength) throws IOException
     {
-        // Is the write for the whole content?
+        // attempt efficient ByteBuffer based write
-        if (start == 0 && content.getResource().length() == contentLength)
+        ByteBuffer buffer = content.getByteBuffer();
+        if (buffer != null)
         {
-            // attempt efficient ByteBuffer based write for whole content
+            // no need to modify buffer pointers when whole content is requested
-            ByteBuffer buffer = content.getByteBuffer();
+            if (start != 0 || content.getResource().length() != contentLength)
-            if (buffer != null)
             {
-                BufferUtil.writeTo(buffer, out);
+                buffer = buffer.asReadOnlyBuffer();
-                return;
+                buffer.position((int)(buffer.position() + start));
-            }
+                buffer.limit((int)(buffer.position() + contentLength));
-
-            try (InputStream input = content.getResource().newInputStream())
-            {
-                IO.copy(input, out);
-                return;
             }
+            BufferUtil.writeTo(buffer, out);
+            return;
         }
 
-        // Use a ranged writer
+        // Use a ranged writer if resource backed by path
-        try (SeekableByteChannelRangeWriter rangeWriter = new SeekableByteChannelRangeWriter(() -> Files.newByteChannel(content.getResource().getPath())))
+        Path path = content.getResource().getPath();
+        if (path != null)
         {
-            rangeWriter.writeTo(out, start, contentLength);
+            try (SeekableByteChannelRangeWriter rangeWriter = new SeekableByteChannelRangeWriter(() -> Files.newByteChannel(path)))
+            {
+                rangeWriter.writeTo(out, start, contentLength);
+            }
+            return;
+        }
+
+        // Perform ranged write
+        try (InputStream input = content.getResource().newInputStream())
+        {
+            input.skipNBytes(start);
+            IO.copy(input, out, contentLength);
         }
     }
 

jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/HttpContentRangeWriter.java

@@ -13,29 +13,25 @@
 
 package org.eclipse.jetty.ee9.nested.resource;
 
-import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Objects;
 
 import org.eclipse.jetty.http.content.HttpContent;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Range Writer selection for HttpContent
  */
 public class HttpContentRangeWriter
 {
-    private static final Logger LOG = LoggerFactory.getLogger(HttpContentRangeWriter.class);
-
     /**
      * Obtain a new RangeWriter for the supplied HttpContent.
      *
      * @param content the HttpContent to base RangeWriter on
      * @return the RangeWriter best suited for the supplied HttpContent
      */
-    public static RangeWriter newRangeWriter(HttpContent content) throws IOException
+    public static RangeWriter newRangeWriter(HttpContent content)
     {
         Objects.requireNonNull(content, "HttpContent");
 
@@ -44,6 +40,12 @@ public class HttpContentRangeWriter
         if (buffer != null)
             return new ByteBufferRangeWriter(buffer);
 
-        return new SeekableByteChannelRangeWriter(() -> Files.newByteChannel(content.getResource().getPath()));
+        // Try path's SeekableByteChannel
+        Path path = content.getResource().getPath();
+        if (path != null)
+            return new SeekableByteChannelRangeWriter(() -> Files.newByteChannel(path));
+
+        // Fallback to InputStream
+        return new InputStreamRangeWriter(() -> content.getResource().newInputStream());
     }
 }

jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java

@@ -245,35 +245,39 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory, Welc
         if (cc != null)
             _resourceService.setCacheControl(new PreEncodedHttpField(HttpHeader.CACHE_CONTROL, cc));
 
-        // Try to get factory from ServletContext attribute.
+        // Create HttpContentFactory if none already set
-        HttpContent.Factory contentFactory = (HttpContent.Factory)getServletContext().getAttribute(HttpContent.Factory.class.getName());
+        if (_resourceService.getHttpContentFactory() == null)
-        if (contentFactory == null)
         {
-            contentFactory = new ResourceHttpContentFactory(this, _mimeTypes);
+            // Try to get factory from ServletContext attribute.
-            if (_useFileMappedBuffer)
+            HttpContent.Factory contentFactory = (HttpContent.Factory)getServletContext().getAttribute(HttpContent.Factory.class.getName());
-                contentFactory = new FileMappingHttpContentFactory(contentFactory);
+            if (contentFactory == null)
-            contentFactory = new VirtualHttpContentFactory(contentFactory, _styleSheet, "text/css");
-            contentFactory = new PreCompressedHttpContentFactory(contentFactory, _resourceService.getPrecompressedFormats());
-
-            int maxCacheSize = getInitInt("maxCacheSize", -2);
-            int maxCachedFileSize = getInitInt("maxCachedFileSize", -2);
-            int maxCachedFiles = getInitInt("maxCachedFiles", -2);
-            long cacheValidationTime = getInitParameter("cacheValidationTime") != null ? Long.parseLong(getInitParameter("cacheValidationTime")) : -2;
-            if (maxCachedFiles != -2 || maxCacheSize != -2 || maxCachedFileSize != -2 || cacheValidationTime != -2)
             {
-                ByteBufferPool bufferPool = getByteBufferPool(_contextHandler);
+                contentFactory = new ResourceHttpContentFactory(this, _mimeTypes);
-                _cachingContentFactory = new ValidatingCachingHttpContentFactory(contentFactory,
+                if (_useFileMappedBuffer)
-                    (cacheValidationTime > -2) ? cacheValidationTime : Duration.ofSeconds(1).toMillis(), bufferPool);
+                    contentFactory = new FileMappingHttpContentFactory(contentFactory);
-                contentFactory = _cachingContentFactory;
+                contentFactory = new VirtualHttpContentFactory(contentFactory, _styleSheet, "text/css");
-                if (maxCacheSize >= 0)
+                contentFactory = new PreCompressedHttpContentFactory(contentFactory, _resourceService.getPrecompressedFormats());
-                    _cachingContentFactory.setMaxCacheSize(maxCacheSize);
+
-                if (maxCachedFileSize >= -1)
+                int maxCacheSize = getInitInt("maxCacheSize", -2);
-                    _cachingContentFactory.setMaxCachedFileSize(maxCachedFileSize);
+                int maxCachedFileSize = getInitInt("maxCachedFileSize", -2);
-                if (maxCachedFiles >= -1)
+                int maxCachedFiles = getInitInt("maxCachedFiles", -2);
-                    _cachingContentFactory.setMaxCachedFiles(maxCachedFiles);
+                long cacheValidationTime = getInitParameter("cacheValidationTime") != null ? Long.parseLong(getInitParameter("cacheValidationTime")) : -2;
+                if (maxCachedFiles != -2 || maxCacheSize != -2 || maxCachedFileSize != -2 || cacheValidationTime != -2)
+                {
+                    ByteBufferPool bufferPool = getByteBufferPool(_contextHandler);
+                    _cachingContentFactory = new ValidatingCachingHttpContentFactory(contentFactory,
+                        (cacheValidationTime > -2) ? cacheValidationTime : Duration.ofSeconds(1).toMillis(), bufferPool);
+                    contentFactory = _cachingContentFactory;
+                    if (maxCacheSize >= 0)
+                        _cachingContentFactory.setMaxCacheSize(maxCacheSize);
+                    if (maxCachedFileSize >= -1)
+                        _cachingContentFactory.setMaxCachedFileSize(maxCachedFileSize);
+                    if (maxCachedFiles >= -1)
+                        _cachingContentFactory.setMaxCachedFiles(maxCachedFiles);
+                }
             }
+            _resourceService.setHttpContentFactory(contentFactory);
         }
-        _resourceService.setHttpContentFactory(contentFactory);
         _resourceService.setWelcomeFactory(this);
 
         List<String> gzipEquivalentFileExtensions = new ArrayList<>();

jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java

@@ -18,6 +18,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.nio.ByteBuffer;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
@@ -45,6 +46,7 @@ import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.http.content.ResourceHttpContent;
 import org.eclipse.jetty.logging.StacklessLogging;
 import org.eclipse.jetty.server.AllowedResourceAliasChecker;
 import org.eclipse.jetty.server.HttpConfiguration;
@@ -55,9 +57,12 @@ import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
+import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.resource.FileSystemPool;
+import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.resource.ResourceFactory;
+import org.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
@@ -2267,6 +2272,142 @@ public class DefaultServletTest
         assertThat(response.get(HttpHeader.ALLOW), is("GET, HEAD, OPTIONS"));
     }
 
+    @Test
+    public void testMemoryResourceRange() throws Exception
+    {
+        Resource memResource = ResourceFactory.root().newMemoryResource(getClass().getResource("/contextResources/test.txt"));
+        ResourceService resourceService = new ResourceService();
+        resourceService.setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain"));
+        DefaultServlet defaultServlet = new DefaultServlet(resourceService);
+        context.addServlet(new ServletHolder(defaultServlet), "/");
+
+        String rawResponse = connector.getResponse("""
+            GET /context/ HTTP/1.1\r
+            Host: local\r
+            Range: bytes=10-12\r
+            Connection: close\r
+            \r
+            """);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.PARTIAL_CONTENT_206));
+        assertThat(response.get(HttpHeader.CONTENT_LENGTH), is("3"));
+        assertThat(response.getContent(), is("too"));
+    }
+
+    @Test
+    public void testMemoryResourceMultipleRanges() throws Exception
+    {
+        Resource memResource = ResourceFactory.root().newMemoryResource(getClass().getResource("/contextResources/test.txt"));
+        ResourceService resourceService = new ResourceService();
+        resourceService.setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain"));
+        DefaultServlet defaultServlet = new DefaultServlet(resourceService);
+        context.addServlet(new ServletHolder(defaultServlet), "/");
+
+        String rawResponse = connector.getResponse("""
+            GET /context/ HTTP/1.1\r
+            Host: local\r
+            Range: bytes=5-8, 10-12\r
+            Connection: close\r
+            \r
+            """);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.PARTIAL_CONTENT_206));
+        assertThat(response.get(HttpHeader.CONTENT_LENGTH), notNullValue());
+        assertThat(response.getContent(), Matchers.stringContainsInOrder(
+            "Content-Type: text/plain", "Content-Range: bytes 5-8/17", "2 to",
+            "Content-Type: text/plain", "Content-Range: bytes 10-12/17", "too")
+        );
+    }
+
+    @Test
+    public void testMemoryResourceRangeUsingBufferedHttpContent() throws Exception
+    {
+        Resource memResource = ResourceFactory.root().newMemoryResource(getClass().getResource("/contextResources/test.txt"));
+        ResourceService resourceService = new ResourceService();
+        resourceService.setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain")
+        {
+            final ByteBuffer buffer = BufferUtil.toBuffer(getResource(), false);
+
+            @Override
+            public ByteBuffer getByteBuffer()
+            {
+                return buffer;
+            }
+        });
+        DefaultServlet defaultServlet = new DefaultServlet(resourceService);
+        context.addServlet(new ServletHolder(defaultServlet), "/");
+
+        String rawResponse = connector.getResponse("""
+            GET /context/ HTTP/1.1\r
+            Host: local\r
+            Range: bytes=10-12\r
+            Connection: close\r
+            \r
+            """);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.PARTIAL_CONTENT_206));
+        assertThat(response.get(HttpHeader.CONTENT_LENGTH), is("3"));
+        assertThat(response.getContent(), is("too"));
+    }
+
+    @Test
+    public void testMemoryResourceMultipleRangesUsingBufferedHttpContent() throws Exception
+    {
+        Resource memResource = ResourceFactory.root().newMemoryResource(getClass().getResource("/contextResources/test.txt"));
+        ResourceService resourceService = new ResourceService();
+        resourceService.setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain")
+        {
+            final ByteBuffer buffer = BufferUtil.toBuffer(getResource(), false);
+
+            @Override
+            public ByteBuffer getByteBuffer()
+            {
+                return buffer;
+            }
+        });
+        DefaultServlet defaultServlet = new DefaultServlet(resourceService);
+        context.addServlet(new ServletHolder(defaultServlet), "/");
+
+        String rawResponse = connector.getResponse("""
+            GET /context/ HTTP/1.1\r
+            Host: local\r
+            Range: bytes=5-8, 10-12\r
+            Connection: close\r
+            \r
+            """);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.PARTIAL_CONTENT_206));
+        assertThat(response.get(HttpHeader.CONTENT_LENGTH), notNullValue());
+        assertThat(response.getContent(), Matchers.stringContainsInOrder(
+            "Content-Type: text/plain", "Content-Range: bytes 5-8/17", "2 to",
+            "Content-Type: text/plain", "Content-Range: bytes 10-12/17", "too")
+        );
+    }
+
+    @Test
+    public void testNotAcceptRanges() throws Exception
+    {
+        Resource memResource = ResourceFactory.root().newMemoryResource(getClass().getResource("/contextResources/test.txt"));
+        ResourceService resourceService = new ResourceService();
+        resourceService.setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain"));
+        resourceService.setAcceptRanges(false);
+        DefaultServlet defaultServlet = new DefaultServlet(resourceService);
+        context.addServlet(new ServletHolder(defaultServlet), "/");
+
+        String rawResponse = connector.getResponse("""
+            GET /context/ HTTP/1.1\r
+            Host: local\r
+            Range: bytes=10-12\r
+            Connection: close\r
+            \r
+            """);
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+        assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
+        assertThat(response.get(HttpHeader.CONTENT_LENGTH), is("17"));
+        assertThat(response.get(HttpHeader.ACCEPT_RANGES), is("none"));
+        assertThat(response.getContent(), is("Test 2 to too two"));
+    }
+
     public static class OutputFilter implements Filter
     {
         @Override