This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 8d2f8fe2b0a57d2c8fc99f5608129cb03fb2ef32 Author: opencode <[email protected]> AuthorDate: Mon Oct 19 16:36:09 2026 +0200 Honour a FastRemovalDequeue maximum size of one The constructor raised a maximum size of one, or below, silently to two. The queue is used by the JSP runtime to limit the number of loaded JSPs through the maxLoadedJsps init parameter, which is documented to accept any positive value, so a configured limit of one kept two JSPs loaded and evicted nothing until a third one was requested. Every operation of the queue handles a maximum size of one correctly, so the clamping is not needed for correctness. The constructor now honours a maximum size of one and rejects a maximum size below one with a localized IllegalArgumentException. The only in-tree caller creates the queue only for limits greater than zero, so no configured value can reach the new exception. Add tests for the size of one, including eviction from both ends and the no-op removal through an evicted handle, and for the rejected sizes. Update the two test comments that recorded the minimum of the maxLoadedJsps parameter as two. Found by code review. --- .../jasper/resources/LocalStrings.properties | 1 + .../org/apache/jasper/util/FastRemovalDequeue.java | 9 ++- .../catalina/core/TestDefaultInstanceManager.java | 2 +- test/org/apache/el/TestELInJsp.java | 2 +- .../apache/jasper/util/TestFastRemovalDequeue.java | 67 ++++++++++++++++++++++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/java/org/apache/jasper/resources/LocalStrings.properties b/java/org/apache/jasper/resources/LocalStrings.properties index ca68c87c73..c298350b4f 100644 --- a/java/org/apache/jasper/resources/LocalStrings.properties +++ b/java/org/apache/jasper/resources/LocalStrings.properties @@ -77,6 +77,7 @@ jsp.error.dynamic.attributes.not.implemented=The [{0}] tag declares that it acce jsp.error.el.parse=[{0}] : [{1}] jsp.error.el.template.deferred=#{...} is not allowed in template text jsp.error.el_interpreter_class.instantiation=Failed to load or instantiate ELInterpreter class [{0}] +jsp.error.fastremovaldequeue.maxSize=The maximum size of the queue must be at least one but was [{0}] jsp.error.file.already.registered=Recursive include of file [{0}] jsp.error.file.cannot.read=Cannot read file: [{0}] jsp.error.file.close=Exception closing reader diff --git a/java/org/apache/jasper/util/FastRemovalDequeue.java b/java/org/apache/jasper/util/FastRemovalDequeue.java index 8fdce038b1..35dfe2b436 100644 --- a/java/org/apache/jasper/util/FastRemovalDequeue.java +++ b/java/org/apache/jasper/util/FastRemovalDequeue.java @@ -16,6 +16,8 @@ */ package org.apache.jasper.util; +import org.apache.jasper.compiler.Localizer; + /** * The FastRemovalDequeue is a Dequeue that supports constant time removal of entries. This is achieved by using a * doubly linked list and wrapping any object added to the collection with an Entry type, that is returned to the @@ -55,10 +57,13 @@ public class FastRemovalDequeue<T> { * Initialize empty queue. * * @param maxSize The maximum size to which the queue will be allowed to grow + * + * @throws IllegalArgumentException if the maximum size is less than one */ public FastRemovalDequeue(int maxSize) { - if (maxSize <= 1) { - maxSize = 2; + if (maxSize < 1) { + throw new IllegalArgumentException( + Localizer.getMessage("jsp.error.fastremovaldequeue.maxSize", Integer.valueOf(maxSize))); } this.maxSize = maxSize; first = null; diff --git a/test/org/apache/catalina/core/TestDefaultInstanceManager.java b/test/org/apache/catalina/core/TestDefaultInstanceManager.java index 5f622a673d..7bd13c49e2 100644 --- a/test/org/apache/catalina/core/TestDefaultInstanceManager.java +++ b/test/org/apache/catalina/core/TestDefaultInstanceManager.java @@ -77,7 +77,7 @@ public class TestDefaultInstanceManager extends TomcatBaseTest { ctxt.addServletContainerInitializer(new JasperInitializer(), null); // Configure the defaults and then tweak the JSP servlet settings - // Note: Min value for maxLoadedJsps is 2 + // Note: Min value for maxLoadedJsps is 1 Tomcat.initWebappDefaults(ctxt); Wrapper w = (Wrapper) ctxt.findChild("jsp"); w.addInitParameter("maxLoadedJsps", "2"); diff --git a/test/org/apache/el/TestELInJsp.java b/test/org/apache/el/TestELInJsp.java index 3055bf47b8..40ddad3362 100644 --- a/test/org/apache/el/TestELInJsp.java +++ b/test/org/apache/el/TestELInJsp.java @@ -310,7 +310,7 @@ public class TestELInJsp extends TomcatBaseTest { ctxt.addServletContainerInitializer(new JasperInitializer(), null); // Configure the defaults and then tweak the JSP servlet settings - // Note: Min value for maxLoadedJsps is 2 + // Note: Min value for maxLoadedJsps is 1 Tomcat.initWebappDefaults(ctxt); Wrapper w = (Wrapper) ctxt.findChild("jsp"); diff --git a/test/org/apache/jasper/util/TestFastRemovalDequeue.java b/test/org/apache/jasper/util/TestFastRemovalDequeue.java index cf676ae648..237b571fef 100644 --- a/test/org/apache/jasper/util/TestFastRemovalDequeue.java +++ b/test/org/apache/jasper/util/TestFastRemovalDequeue.java @@ -194,4 +194,71 @@ public class TestFastRemovalDequeue { Assert.assertNull(q.first); Assert.assertNull(q.last); } + + + /* + * A maximum size of one must be honoured. The constructor used to + * silently raise it to two, so a configured maxLoadedJsps of one kept + * two JSPs loaded and evicted nothing until a third one was requested. + */ + @Test + public void testSingleSizeIsHonoured() throws Exception { + FastRemovalDequeue<Object> q = new FastRemovalDequeue<>(1); + + Object o1 = new Object(); + Object o2 = new Object(); + Object o3 = new Object(); + + FastRemovalDequeue<Object>.Entry e1 = q.push(o1); + Assert.assertNull(e1.getReplaced()); + Assert.assertEquals(1, q.getSize()); + + FastRemovalDequeue<Object>.Entry e2 = q.push(o2); + // Adding to a full queue of one displaces the single entry + Assert.assertSame(o1, e2.getReplaced()); + Assert.assertEquals(1, q.getSize()); + Assert.assertSame(o2, q.first.getContent()); + Assert.assertSame(o2, q.last.getContent()); + + // Removing the handle of the evicted object is a NOP + q.remove(e1); + Assert.assertEquals(1, q.getSize()); + Assert.assertSame(o2, q.first.getContent()); + Assert.assertSame(o2, q.last.getContent()); + + FastRemovalDequeue<Object>.Entry e3 = q.unpop(o3); + // The symmetric case at the other end of the queue + Assert.assertSame(o2, e3.getReplaced()); + Assert.assertEquals(1, q.getSize()); + Assert.assertSame(o3, q.first.getContent()); + Assert.assertSame(o3, q.last.getContent()); + + // Removing the single entry empties the queue + q.remove(e3); + Assert.assertEquals(0, q.getSize()); + Assert.assertNull(q.first); + Assert.assertNull(q.last); + } + + + /* + * A maximum size below one cannot produce a usable queue and is + * rejected, rather than silently replaced by a different size. + */ + @Test + public void testInvalidMaxSize() throws Exception { + try { + new FastRemovalDequeue<Object>(0); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // The message is localized, pin the rendered value + Assert.assertTrue(e.getMessage(), e.getMessage().contains("[0]")); + } + try { + new FastRemovalDequeue<Object>(-1); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage(), e.getMessage().contains("[-1]")); + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
