This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9ab642e885b58942009630f814ca32790e2f3a53 Author: opencode <[email protected]> AuthorDate: Mon Oct 12 20:01:23 2026 +0200 Invalidate cached collections when useClassLoaderResources differs CachedResource.validateResources() did not have the check that validateResource() has, which invalidates a cache entry when the lookup is made with a different value of useClassLoaderResources than the one used when the entry was created. A collection cached for a path under one flag value could therefore be served to callers using the other flag value. Since validateResources() does not re-validate entries of packed WARs at all once the TTL expires, the wrong collection could be served indefinitely. The plural path also lacked the handling Cache.getResource() has for the concurrent insertion race, where the losing thread is given a non cached entry when the entry added by the other thread was created with the other flag value. Cache.getResources() now behaves the same way. As with the singular path, cache entries are keyed on the path only and each entry is valid for a single value of useClassLoaderResources. No in-tree caller currently looks up the same path under both flag values; the gap was latent for users of the public plural WebResourceRoot API. Add a test to TestCachedResource that collections are not shared between class loader resource lookups made with differing useClassLoaderResources values. --- java/org/apache/catalina/webresources/Cache.java | 8 +++++ .../catalina/webresources/CachedResource.java | 8 +++++ .../catalina/webresources/TestCachedResource.java | 42 ++++++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/java/org/apache/catalina/webresources/Cache.java b/java/org/apache/catalina/webresources/Cache.java index 55ce6b649f..4f4ee8c3e9 100644 --- a/java/org/apache/catalina/webresources/Cache.java +++ b/java/org/apache/catalina/webresources/Cache.java @@ -228,6 +228,14 @@ public class Cache { } } else { // Another thread added the entry to the cache + if (cacheEntry.usesClassLoaderResources() != useClassLoaderResources) { + // Race condition adding cache entries with the same path + // but differing values for useClassLoaderResources. As per + // getResource(), let the other thread "win" and keep the + // entry in the cache. This thread will use a cache entry + // instance that isn't added to the cache. + cacheEntry = newCacheEntry; + } // Make sure it is validated cacheEntry.validateResources(useClassLoaderResources); } diff --git a/java/org/apache/catalina/webresources/CachedResource.java b/java/org/apache/catalina/webresources/CachedResource.java index 83b832692e..8f7f371792 100644 --- a/java/org/apache/catalina/webresources/CachedResource.java +++ b/java/org/apache/catalina/webresources/CachedResource.java @@ -181,6 +181,14 @@ public class CachedResource implements WebResource { * @return <code>true</code> if the cached resources are still valid */ protected boolean validateResources(boolean useClassLoaderResources) { + // As per validateResource(), it is possible that some resources will + // only be visible for a given value of useClassLoaderResources so + // invalidate the entry if the lookup is made with a different value + // than the one used when the cache entry was created. + if (usesClassLoaderResources != useClassLoaderResources) { + return false; + } + long now = System.currentTimeMillis(); if (webResources == null) { diff --git a/test/org/apache/catalina/webresources/TestCachedResource.java b/test/org/apache/catalina/webresources/TestCachedResource.java index 237dd8169f..a86848b6a6 100644 --- a/test/org/apache/catalina/webresources/TestCachedResource.java +++ b/test/org/apache/catalina/webresources/TestCachedResource.java @@ -26,6 +26,7 @@ import org.junit.Assert; import org.junit.Test; import org.apache.catalina.Context; +import org.apache.catalina.WebResource; import org.apache.catalina.WebResourceRoot; import org.apache.catalina.core.StandardHost; import org.apache.catalina.startup.Tomcat; @@ -98,6 +99,47 @@ public class TestCachedResource extends TomcatBaseTest { } + @Test + public void testCollectionsAreNotSharedBetweenClassLoaderResourceValues() + throws Exception { + + // Regression test: a collection cached for a path with + // useClassLoaderResources set to false must not be served to a caller + // using true (or vice versa). Cache entries are keyed on path only and + // each entry is valid for a single value of useClassLoaderResources. + // A class loader resource lookup for the class loader root ('' maps + // to the path '/WEB-INF/classes' which is also the key used by the + // static lookup) hits the same cache entry. The packed WAR provides a + // resource set that is only visible to class loader resource lookups + // (the JAR in WEB-INF/lib mounted at WEB-INF/classes). + Tomcat tomcat = getTomcatInstance(); + File docBase = new File("test/webresources/war-url-connection.war"); + Context ctx = tomcat.addWebapp("/test", docBase.getAbsolutePath()); + ((StandardHost) tomcat.getHost()).setUnpackWARs(false); + tomcat.start(); + + WebResourceRoot root = ctx.getResources(); + + // This populates the cache entry for the path with + // useClassLoaderResources set to false. + WebResource[] withoutClassLoaderResources = root.getResources("/WEB-INF/classes"); + // Repeated lookups within the cache TTL are served from the cache. + Assert.assertEquals(withoutClassLoaderResources.length, + root.getResources("/WEB-INF/classes").length); + + // This previously returned the cached collection created with + // useClassLoaderResources set to false, which does not include the + // JAR in WEB-INF/lib. + WebResource[] withClassLoaderResources = root.getClassLoaderResources(""); + Assert.assertTrue(withClassLoaderResources.length > withoutClassLoaderResources.length); + + // The collection cached for the class loader resource lookup must not + // be served to the static lookup either. + Assert.assertEquals(withoutClassLoaderResources.length, + root.getResources("/WEB-INF/classes").length); + } + + @Test public void testDirectoryListingsWar() throws Exception { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
