From 3627f5e07b6f510ad8c3cb56d5fe829f7e5f5aaa Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 8 Jan 2025 13:38:57 -0600 Subject: [PATCH 01/12] Issue #12680 - Test of 2GB+ files on ResourceHandler. --- .../server/handler/ResourceHandlerTest.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index 2fda299cec03..ea8ff94109ff 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -18,6 +18,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.channels.SeekableByteChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -25,6 +27,7 @@ import java.nio.file.StandardOpenOption; import java.nio.file.attribute.FileTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.function.Consumer; import java.util.regex.Matcher; @@ -760,6 +763,28 @@ public void testBigger() throws Exception assertThat(response.getContent(), containsString(" 400\tThis is a big file\n")); } + @Test + public void testOver2GBFile() throws Exception + { + long hugeLength = (long)Integer.MAX_VALUE + 10L; + + generateFile(docRoot.resolve("huge.mkv"), hugeLength); + + HttpTester.Response response = HttpTester.parseResponse( + _local.getResponse(""" + GET /context/huge.mkv HTTP/1.1\r + Host: local\r + Connection: close\r + \r + """)); + + System.err.println(response); + + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + long responseContentLength = response.getLongField(CONTENT_LENGTH); + assertThat(responseContentLength, is(hugeLength)); + } + @Test public void testBrotliInitialCompressed() throws Exception { @@ -3939,6 +3964,40 @@ private void setupBigFiles(Path base) throws Exception } } + private void generateFile(Path staticFile, long size) throws Exception + { + byte[] buf = new byte[(int)(1024 * 1024)]; // about 1 MB + Arrays.fill(buf, (byte)'x'); + ByteBuffer src = ByteBuffer.wrap(buf); + + if (Files.exists(staticFile) && Files.size(staticFile) == size) + { + // all done, nothing left to do. + System.err.printf("File Exists Already: %s (%,d bytes)%n", staticFile, Files.size(staticFile)); + return; + } + + System.err.printf("Creating %,d byte file: %s ...%n", size, staticFile); + try (SeekableByteChannel channel = Files.newByteChannel(staticFile, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) + { + long remaining = size; + while (remaining > 0) + { + ByteBuffer slice = src.slice(); + int len = buf.length; + if (remaining < Integer.MAX_VALUE) + { + len = Math.min(buf.length, (int)remaining); + slice.limit(len); + } + + channel.write(slice); + remaining -= len; + } + } + System.err.println(" Done"); + } + private void setupQuestionMarkDir(Path base) throws IOException { boolean filesystemSupportsQuestionMarkDir = false; From 7d44d720d2707edb442ad8c1af212819b3328a02 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 9 Jan 2025 11:27:18 +0100 Subject: [PATCH 02/12] #12680 fix rounding error in buffer count calculation Signed-off-by: Ludovic Orban --- .../FileMappingHttpContentFactory.java | 6 ++++++ .../FileMappingHttpContentFactoryTest.java | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java index 21efb969a523..d4b10c6ef178 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactory.java @@ -163,6 +163,12 @@ private MultiBufferFileMappedHttpContent(HttpContent content, int maxBufferSize) long contentLength = content.getContentLengthValue(); int bufferCount = Math.toIntExact(contentLength / maxBufferSize); + if (contentLength % maxBufferSize != 0) + { + if (bufferCount == Integer.MAX_VALUE) + throw new IOException("Cannot memory map Content as that would require over Integer.MAX_VALUE buffers: " + content); + bufferCount++; + } _buffers = new ByteBuffer[bufferCount]; long currentPos = 0L; long total = 0L; diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java index bbc52c0e30a5..bdb20a08ecff 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/content/FileMappingHttpContentFactoryTest.java @@ -28,6 +28,8 @@ import org.eclipse.jetty.util.resource.ResourceFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -39,7 +41,7 @@ public class FileMappingHttpContentFactoryTest public WorkDir workDir; @Test - public void testMultiBufferFileMapped() throws Exception + public void testMultiBufferFileMappedOffsetAndLength() throws Exception { Path file = Files.writeString(workDir.getEmptyPathDir().resolve("file.txt"), "0123456789abcdefghijABCDEFGHIJ"); FileMappingHttpContentFactory fileMappingHttpContentFactory = new FileMappingHttpContentFactory( @@ -76,6 +78,22 @@ public void testMultiBufferFileMapped() throws Exception assertThat(writeToString(content, 25, -1), is("FGHIJ")); } + @ParameterizedTest + @ValueSource(ints = {8, 10}) + public void testMultiBufferFileMappedMaxBufferSizeRounding(int maxBufferSize) throws Exception + { + Path file = Files.writeString(workDir.getEmptyPathDir().resolve("file.txt"), "0123456789abcdefghijABCDEFGHIJ"); + FileMappingHttpContentFactory fileMappingHttpContentFactory = new FileMappingHttpContentFactory( + new ResourceHttpContentFactory(ResourceFactory.root().newResource(file.getParent()), MimeTypes.DEFAULTS, ByteBufferPool.SIZED_NON_POOLING), + 0, maxBufferSize); + + HttpContent content = fileMappingHttpContentFactory.getContent("file.txt"); + + assertThat(content.getContentLength().getValue(), is("30")); + assertThat(content.getContentLengthValue(), is(30L)); + assertThat(writeToString(content, 0, -1), is("0123456789abcdefghijABCDEFGHIJ")); + } + private static String writeToString(HttpContent content, long offset, long length) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); From 8ec628375e57ed20cf5c22b837e134c3544f5970 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 9 Jan 2025 14:31:28 +0100 Subject: [PATCH 03/12] #12680 remove superfluous System.err prints Signed-off-by: Ludovic Orban --- .../eclipse/jetty/server/handler/ResourceHandlerTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index ea8ff94109ff..2e0cd48c04d2 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -778,8 +778,6 @@ public void testOver2GBFile() throws Exception \r """)); - System.err.println(response); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); long responseContentLength = response.getLongField(CONTENT_LENGTH); assertThat(responseContentLength, is(hugeLength)); @@ -3973,11 +3971,9 @@ private void generateFile(Path staticFile, long size) throws Exception if (Files.exists(staticFile) && Files.size(staticFile) == size) { // all done, nothing left to do. - System.err.printf("File Exists Already: %s (%,d bytes)%n", staticFile, Files.size(staticFile)); return; } - System.err.printf("Creating %,d byte file: %s ...%n", size, staticFile); try (SeekableByteChannel channel = Files.newByteChannel(staticFile, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { long remaining = size; @@ -3995,7 +3991,6 @@ private void generateFile(Path staticFile, long size) throws Exception remaining -= len; } } - System.err.println(" Done"); } private void setupQuestionMarkDir(Path base) throws IOException From 386c282b89abbf55bf147cf4925faf5a5c494f05 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 9 Jan 2025 18:33:18 +0000 Subject: [PATCH 04/12] Bump org.apache.mina:mina-core from 2.2.3 to 2.2.4 Bumps [org.apache.mina:mina-core](https://github.com/apache/mina) from 2.2.3 to 2.2.4. - [Commits](https://github.com/apache/mina/compare/2.2.3...2.2.4) --- updated-dependencies: - dependency-name: org.apache.mina:mina-core dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7fbddcbd5b65..b70c6d0ebbe2 100644 --- a/pom.xml +++ b/pom.xml @@ -276,7 +276,7 @@ 3.3.0 3.9.9 3.4.0 - 2.2.3 + 2.2.4 5.0.26 5.1.3 4.1.115.Final From cbbd0fed019861d8e4e51cb9a4c8490b8a92e459 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 9 Jan 2025 18:33:39 +0000 Subject: [PATCH 05/12] Bump ch.qos.logback:logback-core from 1.5.12 to 1.5.13 Bumps [ch.qos.logback:logback-core](https://github.com/qos-ch/logback) from 1.5.12 to 1.5.13. - [Commits](https://github.com/qos-ch/logback/compare/v_1.5.12...v_1.5.13) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-core dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7fbddcbd5b65..0822e6ce0c46 100644 --- a/pom.xml +++ b/pom.xml @@ -241,7 +241,7 @@ 4.5 ${project.build.directory}/local-repo 2.24.2 - 1.5.12 + 1.5.13 9.9.2 10.3.6 3.5.1 From 6b48a1918d64a4f269cace188dfa56fc6eaf4f70 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 10 Jan 2025 09:13:10 +1100 Subject: [PATCH 06/12] Issue #12668 Fix log line for jetty.home.bundle property (#12677) --- .../jetty/osgi/JettyBootstrapActivator.java | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/jetty-core/jetty-osgi/src/main/java/org/eclipse/jetty/osgi/JettyBootstrapActivator.java b/jetty-core/jetty-osgi/src/main/java/org/eclipse/jetty/osgi/JettyBootstrapActivator.java index d8f4593a3ef1..8e3037bc03a4 100644 --- a/jetty-core/jetty-osgi/src/main/java/org/eclipse/jetty/osgi/JettyBootstrapActivator.java +++ b/jetty-core/jetty-osgi/src/main/java/org/eclipse/jetty/osgi/JettyBootstrapActivator.java @@ -65,7 +65,6 @@ public class JettyBootstrapActivator implements BundleActivator public static final String DEFAULT_JETTYHOME = "/jettyhome"; private ServiceRegistration _registeredServer; - /* private PackageAdminServiceTracker _packageAdminServiceTracker;*/ /** * Setup a new jetty Server, register it as a service. @@ -75,10 +74,6 @@ public class JettyBootstrapActivator implements BundleActivator @Override public void start(final BundleContext context) throws Exception { - // track other bundles and fragments attached to this bundle that we - // should activate, as OSGi will not call activators for them. - /* _packageAdminServiceTracker = new PackageAdminServiceTracker(context);*/ - ServiceReference[] references = context.getAllServiceReferences("org.eclipse.jetty.http.HttpFieldPreEncoder", null); if (references == null || references.length == 0) @@ -96,14 +91,6 @@ public void start(final BundleContext context) throws Exception @Override public void stop(BundleContext context) throws Exception { - - /* if (_packageAdminServiceTracker != null) - { - _packageAdminServiceTracker.stop(); - context.removeServiceListener(_packageAdminServiceTracker); - _packageAdminServiceTracker = null; - } - */ try { if (_registeredServer != null) @@ -194,7 +181,7 @@ else if (jettyHomeBundleSysProp != null) } if (jettyHomeBundle == null) { - LOG.warn("Unable to find the jetty.home.bundle named {}", jettyHomeSysProp); + LOG.warn("Unable to find the jetty.home.bundle named {}", jettyHomeBundleSysProp); return; } } From 9761d61d8739130be1f20a9b48d70b582d662f6f Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 10 Jan 2025 10:05:41 +1100 Subject: [PATCH 07/12] Issue #12683 Fix cross context dispatch to root context. (#12684) --- .../servlet/CrossContextServletContext.java | 2 +- .../servlet/CrossContextDispatcherTest.java | 102 +++++++++++++++++- .../nested/CrossContextServletContext.java | 2 +- .../servlet/CrossContextDispatcherTest.java | 101 ++++++++++++++++- .../src/test/resources/docroot/empty | 0 5 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 jetty-ee9/jetty-ee9-servlet/src/test/resources/docroot/empty diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/CrossContextServletContext.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/CrossContextServletContext.java index f83f4c6b49ff..78db368d1f34 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/CrossContextServletContext.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/CrossContextServletContext.java @@ -141,7 +141,7 @@ public RequestDispatcher getRequestDispatcher(String uriInContext) if (StringUtil.isEmpty(encodedPathInContext)) return null; - if (!StringUtil.isEmpty(contextPath)) + if (!StringUtil.isEmpty(contextPath) && !contextPath.equals("/")) { uri.path(URIUtil.addPaths(contextPath, uri.getPath())); encodedPathInContext = uri.getCanonicalPath().substring(contextPath.length()); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/CrossContextDispatcherTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/CrossContextDispatcherTest.java index 3dc4cb685d68..7ebd4b6f73d1 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/CrossContextDispatcherTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/CrossContextDispatcherTest.java @@ -115,8 +115,8 @@ public class CrossContextDispatcherTest private Server _server; private LocalConnector _connector; private ServletContextHandler _contextHandler; - private ServletContextHandler _targetServletContextHandler; + private ServletContextHandler _rootContextHandler; @BeforeEach public void init() throws Exception @@ -145,6 +145,13 @@ public void init() throws Exception resourceContextHandler.setHandler(resourceHandler); resourceContextHandler.setCrossContextDispatchSupported(true); contextCollection.addHandler(resourceContextHandler); + + _rootContextHandler = new ServletContextHandler(); + _rootContextHandler.setContextPath("/"); + _rootContextHandler.setBaseResourceAsPath(MavenTestingUtils.getTestResourcePathDir("docroot")); + _rootContextHandler.setCrossContextDispatchSupported(true); + contextCollection.addHandler(_rootContextHandler); + _server.setHandler(contextCollection); _server.addConnector(_connector); @@ -158,6 +165,91 @@ public void destroy() throws Exception _server.join(); } + @Test + public void testForwardToRoot() throws Exception + { + _rootContextHandler.addServlet(VerifyForwardServlet.class, "/verify/*"); + _contextHandler.addServlet(CrossContextDispatchServlet.class, "/dispatch/*"); + + String rawResponse = _connector.getResponse(""" + GET /context/dispatch/?forward=/verify&ctx=/ HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + String content = response.getContent(); + String[] contentLines = content.split("\\n"); + + //verify forward attributes + assertThat(content, containsString("Verified!")); + assertThat(content, containsString("jakarta.servlet.forward.context_path=/context")); + assertThat(content, containsString("jakarta.servlet.forward.servlet_path=/dispatch")); + assertThat(content, containsString("jakarta.servlet.forward.path_info=/")); + + String forwardMapping = extractLine(contentLines, "jakarta.servlet.forward.mapping="); + assertNotNull(forwardMapping); + assertThat(forwardMapping, containsString("CrossContextDispatchServlet")); + assertThat(content, containsString("jakarta.servlet.forward.query_string=forward=/verify&ctx=/")); + assertThat(content, containsString("jakarta.servlet.forward.request_uri=/context/dispatch/")); + //verify request values + assertThat(content, containsString("REQUEST_URL=http://localhost/")); + assertThat(content, containsString("CONTEXT_PATH=")); + assertThat(content, containsString("SERVLET_PATH=/verify")); + assertThat(content, containsString("PATH_INFO=/pinfo")); + String mapping = extractLine(contentLines, "MAPPING="); + assertNotNull(mapping); + assertThat(mapping, containsString("VerifyForwardServlet")); + String params = extractLine(contentLines, "PARAMS="); + assertNotNull(params); + params = params.substring(params.indexOf("=") + 1); + params = params.substring(1, params.length() - 1); //dump leading, trailing [ ] + assertThat(Arrays.asList(StringUtil.csvSplit(params)), containsInAnyOrder("a", "forward", "ctx")); + assertThat(content, containsString("REQUEST_URI=/verify/pinfo")); + } + + @Test + public void testIncludeToRoot() throws Exception + { + _rootContextHandler.addServlet(VerifyIncludeServlet.class, "/verify/*"); + _contextHandler.addServlet(CrossContextDispatchServlet.class, "/dispatch/*"); + + String rawResponse = _connector.getResponse(""" + GET /context/dispatch/?include=/verify&ctx=/ HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + String content = response.getContent(); + String[] contentLines = content.split("\\n"); + + //verify include attributes + assertThat(content, containsString("Verified!")); + assertThat(content, containsString("jakarta.servlet.include.context_path=/")); + assertThat(content, containsString("jakarta.servlet.include.servlet_path=/verify")); + assertThat(content, containsString("jakarta.servlet.include.path_info=/pinfo")); + String includeMapping = extractLine(contentLines, "jakarta.servlet.include.mapping="); + assertThat(includeMapping, containsString("VerifyIncludeServlet")); + assertThat(content, containsString("jakarta.servlet.include.request_uri=/verify/pinfo")); + //verify request values + assertThat(content, containsString("CONTEXT_PATH=/context")); + assertThat(content, containsString("SERVLET_PATH=/dispatch")); + assertThat(content, containsString("PATH_INFO=/")); + String mapping = extractLine(contentLines, "MAPPING="); + assertThat(mapping, containsString("CrossContextDispatchServlet")); + assertThat(content, containsString("QUERY_STRING=include=/verify")); + assertThat(content, containsString("REQUEST_URI=/context/dispatch/")); + String params = extractLine(contentLines, "PARAMS="); + assertNotNull(params); + params = params.substring(params.indexOf("=") + 1); + params = params.substring(1, params.length() - 1); //dump leading, trailing [ ] + assertThat(Arrays.asList(StringUtil.csvSplit(params)), containsInAnyOrder("a", "include", "ctx")); + } + @Test public void testSimpleCrossContextForward() throws Exception { @@ -732,10 +824,12 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { RequestDispatcher dispatcher; - + String ctx = request.getParameter("ctx"); + if (StringUtil.isBlank(ctx)) + ctx = "/foreign"; if (request.getParameter("forward") != null) { - ServletContext foreign = getServletContext().getContext("/foreign"); + ServletContext foreign = getServletContext().getContext(ctx); assertNotNull(foreign); dispatcher = foreign.getRequestDispatcher(request.getParameter("forward") + "/pinfo?a=b"); @@ -746,7 +840,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t } else if (request.getParameter("include") != null) { - ServletContext foreign = getServletContext().getContext("/foreign"); + ServletContext foreign = getServletContext().getContext(ctx); assertNotNull(foreign); dispatcher = foreign.getRequestDispatcher(request.getParameter("include") + "/pinfo?a=b"); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CrossContextServletContext.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CrossContextServletContext.java index aaa6981de4bf..da8359144fc9 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CrossContextServletContext.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CrossContextServletContext.java @@ -169,7 +169,7 @@ public RequestDispatcher getRequestDispatcher(String uriInContext) if (StringUtil.isEmpty(encodedPathInContext)) return null; - if (!StringUtil.isEmpty(contextPath)) + if (!StringUtil.isEmpty(contextPath) && !contextPath.equals("/")) { uri.path(URIUtil.addPaths(contextPath, uri.getPath())); encodedPathInContext = uri.getCanonicalPath().substring(contextPath.length()); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/CrossContextDispatcherTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/CrossContextDispatcherTest.java index 50379d428506..6155caa40930 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/CrossContextDispatcherTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/CrossContextDispatcherTest.java @@ -116,8 +116,8 @@ public class CrossContextDispatcherTest private Server _server; private LocalConnector _connector; private ServletContextHandler _contextHandler; - private ServletContextHandler _targetServletContextHandler; + private ServletContextHandler _rootContextHandler; @BeforeEach public void init() throws Exception @@ -140,6 +140,12 @@ public void init() throws Exception _targetServletContextHandler.setCrossContextDispatchSupported(true); contextCollection.addHandler(_targetServletContextHandler); + _rootContextHandler = new ServletContextHandler(); + _rootContextHandler.setContextPath("/"); + _rootContextHandler.setBaseResource(ResourceFactory.root().newResource(MavenPaths.findTestResourceDir("docroot"))); + _rootContextHandler.setCrossContextDispatchSupported(true); + contextCollection.addHandler(_rootContextHandler); + ResourceHandler resourceHandler = new ResourceHandler(); resourceHandler.setBaseResource(ResourceFactory.root().newResource(MavenPaths.findTestResourceDir("dispatchResourceTest"))); ContextHandler resourceContextHandler = new ContextHandler("/resource"); @@ -159,6 +165,91 @@ public void destroy() throws Exception _server.join(); } + @Test + public void testForwardToRoot() throws Exception + { + _rootContextHandler.addServlet(VerifyForwardServlet.class, "/verify/*"); + _contextHandler.addServlet(CrossContextDispatchServlet.class, "/dispatch/*"); + + String rawResponse = _connector.getResponse(""" + GET /context/dispatch/?forward=/verify&ctx=/ HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + String content = response.getContent(); + String[] contentLines = content.split("\\n"); + + //verify forward attributes + assertThat(content, containsString("Verified!")); + assertThat(content, containsString("jakarta.servlet.forward.context_path=/context")); + assertThat(content, containsString("jakarta.servlet.forward.servlet_path=/dispatch")); + assertThat(content, containsString("jakarta.servlet.forward.path_info=/")); + + String forwardMapping = extractLine(contentLines, "jakarta.servlet.forward.mapping="); + assertNotNull(forwardMapping); + assertThat(forwardMapping, containsString("CrossContextDispatchServlet")); + assertThat(content, containsString("jakarta.servlet.forward.query_string=forward=/verify&ctx=/")); + assertThat(content, containsString("jakarta.servlet.forward.request_uri=/context/dispatch/")); + //verify request values + assertThat(content, containsString("REQUEST_URL=http://localhost/")); + assertThat(content, containsString("CONTEXT_PATH=")); + assertThat(content, containsString("SERVLET_PATH=/verify")); + assertThat(content, containsString("PATH_INFO=/pinfo")); + String mapping = extractLine(contentLines, "MAPPING="); + assertNotNull(mapping); + assertThat(mapping, containsString("VerifyForwardServlet")); + String params = extractLine(contentLines, "PARAMS="); + assertNotNull(params); + params = params.substring(params.indexOf("=") + 1); + params = params.substring(1, params.length() - 1); //dump leading, trailing [ ] + assertThat(Arrays.asList(StringUtil.csvSplit(params)), containsInAnyOrder("a", "forward", "ctx")); + assertThat(content, containsString("REQUEST_URI=/verify/pinfo")); + } + + @Test + public void testIncludeToRoot() throws Exception + { + _rootContextHandler.addServlet(VerifyIncludeServlet.class, "/verify/*"); + _contextHandler.addServlet(CrossContextDispatchServlet.class, "/dispatch/*"); + + String rawResponse = _connector.getResponse(""" + GET /context/dispatch/?include=/verify&ctx=/ HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """); + + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + String content = response.getContent(); + String[] contentLines = content.split("\\n"); + + //verify include attributes + assertThat(content, containsString("Verified!")); + assertThat(content, containsString("jakarta.servlet.include.context_path=")); + assertThat(content, containsString("jakarta.servlet.include.servlet_path=/verify")); + assertThat(content, containsString("jakarta.servlet.include.path_info=/pinfo")); + String includeMapping = extractLine(contentLines, "jakarta.servlet.include.mapping="); + assertThat(includeMapping, containsString("VerifyIncludeServlet")); + assertThat(content, containsString("jakarta.servlet.include.request_uri=/verify/pinfo")); + //verify request values + assertThat(content, containsString("CONTEXT_PATH=/context")); + assertThat(content, containsString("SERVLET_PATH=/dispatch")); + assertThat(content, containsString("PATH_INFO=/")); + String mapping = extractLine(contentLines, "MAPPING="); + assertThat(mapping, containsString("CrossContextDispatchServlet")); + assertThat(content, containsString("QUERY_STRING=include=/verify")); + assertThat(content, containsString("REQUEST_URI=/context/dispatch/")); + String params = extractLine(contentLines, "PARAMS="); + assertNotNull(params); + params = params.substring(params.indexOf("=") + 1); + params = params.substring(1, params.length() - 1); //dump leading, trailing [ ] + assertThat(Arrays.asList(StringUtil.csvSplit(params)), containsInAnyOrder("a", "include", "ctx")); + } + @Test public void testSimpleCrossContextForward() throws Exception { @@ -749,10 +840,12 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { RequestDispatcher dispatcher; - + String ctx = request.getParameter("ctx"); + if (StringUtil.isBlank(ctx)) + ctx = "/foreign"; if (request.getParameter("forward") != null) { - ServletContext foreign = getServletContext().getContext("/foreign"); + ServletContext foreign = getServletContext().getContext(ctx); assertNotNull(foreign); dispatcher = foreign.getRequestDispatcher(request.getParameter("forward") + "/pinfo?a=b"); @@ -768,7 +861,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t } else if (request.getParameter("include") != null) { - ServletContext foreign = getServletContext().getContext("/foreign"); + ServletContext foreign = getServletContext().getContext(ctx); assertNotNull(foreign); dispatcher = foreign.getRequestDispatcher(request.getParameter("include") + "/pinfo?a=b"); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/resources/docroot/empty b/jetty-ee9/jetty-ee9-servlet/src/test/resources/docroot/empty new file mode 100644 index 000000000000..e69de29bb2d1 From 8414f79a9c476ecb78998c8ce88f0c5ae548f7e6 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 10 Jan 2025 16:31:14 +0100 Subject: [PATCH 08/12] #12690 cap MAX_HEADER_LIST_SIZE Signed-off-by: Ludovic Orban --- .../org/eclipse/jetty/http2/HTTP2Session.java | 7 +++- .../jetty/http2/hpack/HpackEncoder.java | 2 +- .../jetty/http2/tests/SettingsTest.java | 36 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 5f622fa09a47..07281e28ec50 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -464,9 +464,14 @@ private void configure(Map settings, boolean local) if (LOG.isDebugEnabled()) LOG.debug("Updating {} max header list size to {} for {}", local ? "decoder" : "encoder", value, this); if (local) + { parser.getHpackDecoder().setMaxHeaderListSize(value); + } else - generator.getHpackEncoder().setMaxHeaderListSize(value); + { + HpackEncoder hpackEncoder = generator.getHpackEncoder(); + hpackEncoder.setMaxHeaderListSize(Math.min(value, hpackEncoder.getMaxHeaderListSize())); + } } case SettingsFrame.ENABLE_CONNECT_PROTOCOL -> { diff --git a/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java index bcbac88974d4..d58182c76520 100644 --- a/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java +++ b/jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java @@ -156,7 +156,7 @@ public int getMaxHeaderListSize() public void setMaxHeaderListSize(int maxHeaderListSize) { - _maxHeaderListSize = maxHeaderListSize; + _maxHeaderListSize = maxHeaderListSize > 0 ? maxHeaderListSize : HpackContext.DEFAULT_MAX_HEADER_LIST_SIZE; } public HpackContext getHpackContext() diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SettingsTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SettingsTest.java index f7224ec82edc..578a64d18659 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SettingsTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SettingsTest.java @@ -446,6 +446,42 @@ public void onGoAway(Session session, GoAwayFrame frame) assertTrue(goAwayLatch.await(5, TimeUnit.SECONDS)); } + @Test + public void testMaxHeaderListSizeCappedByClient() throws Exception + { + int maxHeadersSize = 2 * 1024; + CountDownLatch goAwayLatch = new CountDownLatch(1); + start(new ServerSessionListener() + { + @Override + public Map onPreface(Session session) + { + return Map.of(SettingsFrame.MAX_HEADER_LIST_SIZE, maxHeadersSize); + } + + @Override + public void onGoAway(Session session, GoAwayFrame frame) + { + goAwayLatch.countDown(); + } + }); + http2Client.setMaxRequestHeadersSize(maxHeadersSize / 2); + + Session clientSession = newClientSession(new Session.Listener() {}); + HttpFields requestHeaders = HttpFields.build() + .put("X-Large", "x".repeat(maxHeadersSize - 256)); // 256 bytes to account for the other headers + MetaData.Request request = newRequest("GET", requestHeaders); + HeadersFrame frame = new HeadersFrame(request, null, true); + + Throwable failure = assertThrows(ExecutionException.class, + () -> clientSession.newStream(frame, new Stream.Listener() {}).get(5, TimeUnit.SECONDS)) + .getCause(); + // The HPACK context is compromised trying to encode the large header. + assertThat(failure, Matchers.instanceOf(HpackException.SessionException.class)); + + assertTrue(goAwayLatch.await(5, TimeUnit.SECONDS)); + } + @Test public void testMaxHeaderListSizeExceededByServer() throws Exception { From c8c2515936ef968dc8a3cecd9e79d1e69291e4bb Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 10 Jan 2025 16:57:48 +0100 Subject: [PATCH 09/12] #12690 fix test Signed-off-by: Ludovic Orban --- .../src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java index 6245602464ca..16c35a251c33 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java @@ -1315,6 +1315,7 @@ public boolean handle(Request request, Response response, Callback callback) }, httpConfig); connector.getBean(AbstractHTTP2ServerConnectionFactory.class).setMaxFrameSize(17 * 1024); http2Client.setMaxFrameSize(18 * 1024); + http2Client.setMaxRequestHeadersSize(2 * maxHeadersSize); // Wait for the SETTINGS frame to be exchanged. CountDownLatch settingsLatch = new CountDownLatch(1); From 097080c48bafcb0a6838a7d7b1a93e2b948382a2 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 9 Jan 2025 18:56:38 +0100 Subject: [PATCH 10/12] #12689 add no-bucket acquire statistics Signed-off-by: Ludovic Orban --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 32 ++++++++++++++++++- .../jetty/io/ArrayByteBufferPoolTest.java | 25 +++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index eff48e666838..8e25da119e49 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -23,6 +23,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; @@ -40,6 +41,7 @@ import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; +import org.eclipse.jetty.util.component.DumpableMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,7 +67,10 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable private final long _maxHeapMemory; private final long _maxDirectMemory; private final IntUnaryOperator _bucketIndexFor; + private final IntUnaryOperator _bucketCapacity; private final AtomicBoolean _evictor = new AtomicBoolean(false); + private final ConcurrentMap _noBucketDirectSizes = new ConcurrentHashMap<>(); + private final ConcurrentMap _noBucketIndirectSizes = new ConcurrentHashMap<>(); private boolean _statisticsEnabled; /** @@ -164,6 +169,7 @@ protected ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity, int _maxHeapMemory = maxMemory(maxHeapMemory); _maxDirectMemory = maxMemory(maxDirectMemory); _bucketIndexFor = bucketIndexFor; + _bucketCapacity = bucketCapacity; } private long maxMemory(long maxMemory) @@ -205,7 +211,10 @@ public RetainableByteBuffer acquire(int size, boolean direct) // No bucket, return non-pooled. if (bucket == null) + { + recordNoBucketAcquire(size, direct); return RetainableByteBuffer.wrap(BufferUtil.allocate(size, direct)); + } bucket.recordAcquire(); @@ -223,6 +232,22 @@ public RetainableByteBuffer acquire(int size, boolean direct) return buffer; } + private void recordNoBucketAcquire(int size, boolean direct) + { + if (isStatisticsEnabled()) + { + ConcurrentMap map = direct ? _noBucketDirectSizes : _noBucketIndirectSizes; + int idx = _bucketIndexFor.applyAsInt(size); + int key = _bucketCapacity.applyAsInt(idx); + map.compute(key, (k, v) -> + { + if (v == null) + return 1L; + return v + 1L; + }); + } + } + @Override public boolean removeAndRelease(RetainableByteBuffer buffer) { @@ -427,7 +452,9 @@ public long getAvailableHeapMemory() public void clear() { clearBuckets(_direct); + _noBucketDirectSizes.clear(); clearBuckets(_indirect); + _noBucketIndirectSizes.clear(); } private void clearBuckets(RetainedBucket[] buckets) @@ -446,7 +473,10 @@ public void dump(Appendable out, String indent) throws IOException indent, this, DumpableCollection.fromArray("direct", _direct), - DumpableCollection.fromArray("indirect", _indirect)); + new DumpableMap("direct non-pooled acquisitions", _noBucketDirectSizes), + DumpableCollection.fromArray("indirect", _indirect), + new DumpableMap("indirect non-pooled acquisitions", _noBucketIndirectSizes) + ); } @Override diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index bb92c805080d..8a1d370368ec 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.lessThan; @@ -38,6 +39,30 @@ public class ArrayByteBufferPoolTest { + @Test + public void testDump() + { + ArrayByteBufferPool pool = new ArrayByteBufferPool(0, 10, 100, Integer.MAX_VALUE, 200, 200); + pool.setStatisticsEnabled(true); + + List buffers = new ArrayList<>(); + + for (int i = 1; i < 151; i++) + buffers.add(pool.acquire(i, true)); + + buffers.forEach(RetainableByteBuffer::release); + + String dump = pool.dump(); + assertThat(dump, containsString("direct non-pooled acquisitions size=5\n")); + assertThat(dump, containsString("110: 10\n")); + assertThat(dump, containsString("120: 10\n")); + assertThat(dump, containsString("130: 10\n")); + assertThat(dump, containsString("140: 10\n")); + assertThat(dump, containsString("150: 10\n")); + pool.clear(); + assertThat(pool.dump(), containsString("direct non-pooled acquisitions size=0\n")); + } + @Test public void testMaxMemoryEviction() { From 0a23b82b8b6efbabfc7477dddb8309bfb593e48e Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 10 Jan 2025 10:21:41 +0100 Subject: [PATCH 11/12] #12689 handle review comments Signed-off-by: Ludovic Orban --- .../org/eclipse/jetty/io/ArrayByteBufferPool.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 8e25da119e49..eab35fcd45c8 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -69,8 +69,8 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable private final IntUnaryOperator _bucketIndexFor; private final IntUnaryOperator _bucketCapacity; private final AtomicBoolean _evictor = new AtomicBoolean(false); - private final ConcurrentMap _noBucketDirectSizes = new ConcurrentHashMap<>(); - private final ConcurrentMap _noBucketIndirectSizes = new ConcurrentHashMap<>(); + private final ConcurrentMap _noBucketDirectAcquires = new ConcurrentHashMap<>(); + private final ConcurrentMap _noBucketIndirectAcquires = new ConcurrentHashMap<>(); private boolean _statisticsEnabled; /** @@ -236,7 +236,7 @@ private void recordNoBucketAcquire(int size, boolean direct) { if (isStatisticsEnabled()) { - ConcurrentMap map = direct ? _noBucketDirectSizes : _noBucketIndirectSizes; + ConcurrentMap map = direct ? _noBucketDirectAcquires : _noBucketIndirectAcquires; int idx = _bucketIndexFor.applyAsInt(size); int key = _bucketCapacity.applyAsInt(idx); map.compute(key, (k, v) -> @@ -452,9 +452,9 @@ public long getAvailableHeapMemory() public void clear() { clearBuckets(_direct); - _noBucketDirectSizes.clear(); + _noBucketDirectAcquires.clear(); clearBuckets(_indirect); - _noBucketIndirectSizes.clear(); + _noBucketIndirectAcquires.clear(); } private void clearBuckets(RetainedBucket[] buckets) @@ -473,9 +473,9 @@ public void dump(Appendable out, String indent) throws IOException indent, this, DumpableCollection.fromArray("direct", _direct), - new DumpableMap("direct non-pooled acquisitions", _noBucketDirectSizes), + new DumpableMap("direct non-pooled acquisitions", _noBucketDirectAcquires), DumpableCollection.fromArray("indirect", _indirect), - new DumpableMap("indirect non-pooled acquisitions", _noBucketIndirectSizes) + new DumpableMap("indirect non-pooled acquisitions", _noBucketIndirectAcquires) ); } From 73e60583f07516f075f982c1e8280e07ea406056 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 14 Jan 2025 07:40:00 +1100 Subject: [PATCH 12/12] Partial fix for #12681 caching content (#12702) This is a partial fix for #12681. It does not fix the actual release race on a single content, but by restricting the shrinking to a single thread, make the race less likely to occur. There is a more comprehensive fix in #12678 and #12704 for 12.1, --- .../content/CachingHttpContentFactory.java | 62 +++++++++++-------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java index 15a5402e62ed..896de6798e2b 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/content/CachingHttpContentFactory.java @@ -68,6 +68,7 @@ public class CachingHttpContentFactory implements HttpContent.Factory private final HttpContent.Factory _authority; private final ConcurrentHashMap _cache = new ConcurrentHashMap<>(); private final AtomicLong _cachedSize = new AtomicLong(); + private final AtomicBoolean _shrinking = new AtomicBoolean(); private final ByteBufferPool _bufferPool; private int _maxCachedFileSize = DEFAULT_MAX_CACHED_FILE_SIZE; private int _maxCachedFiles = DEFAULT_MAX_CACHED_FILES; @@ -148,35 +149,46 @@ public void setUseDirectByteBuffers(boolean useDirectByteBuffers) private void shrinkCache() { - // While we need to shrink - int numCacheEntries = _cache.size(); - while (numCacheEntries > 0 && (numCacheEntries > _maxCachedFiles || _cachedSize.get() > _maxCacheSize)) + // Only 1 thread shrinking at once + if (_shrinking.compareAndSet(false, true)) { - // Scan the entire cache and generate an ordered list by last accessed time. - SortedSet sorted = new TreeSet<>((c1, c2) -> + try { - long delta = NanoTime.elapsed(c2.getLastAccessedNanos(), c1.getLastAccessedNanos()); - if (delta != 0) - return delta < 0 ? -1 : 1; - - delta = c1.getContentLengthValue() - c2.getContentLengthValue(); - if (delta != 0) - return delta < 0 ? -1 : 1; - - return c1.getKey().compareTo(c2.getKey()); - }); - sorted.addAll(_cache.values()); - - // TODO: Can we remove the buffers from the content before evicting. - // Invalidate least recently used first - for (CachingHttpContent content : sorted) + // While we need to shrink + int numCacheEntries = _cache.size(); + while (numCacheEntries > 0 && (numCacheEntries > _maxCachedFiles || _cachedSize.get() > _maxCacheSize)) + { + // Scan the entire cache and generate an ordered list by last accessed time. + SortedSet sorted = new TreeSet<>((c1, c2) -> + { + long delta = NanoTime.elapsed(c2.getLastAccessedNanos(), c1.getLastAccessedNanos()); + if (delta != 0) + return delta < 0 ? -1 : 1; + + delta = c1.getContentLengthValue() - c2.getContentLengthValue(); + if (delta != 0) + return delta < 0 ? -1 : 1; + + return c1.getKey().compareTo(c2.getKey()); + }); + sorted.addAll(_cache.values()); + + // TODO: Can we remove the buffers from the content before evicting. + // Invalidate least recently used first + for (CachingHttpContent content : sorted) + { + if (_cache.size() <= _maxCachedFiles && _cachedSize.get() <= _maxCacheSize) + break; + removeFromCache(content); + } + + numCacheEntries = _cache.size(); + } + } + finally { - if (_cache.size() <= _maxCachedFiles && _cachedSize.get() <= _maxCacheSize) - break; - removeFromCache(content); + _shrinking.set(false); } - - numCacheEntries = _cache.size(); } }