root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 4 Packages Projects Releases Wiki Activity

Prevent cache pollution by storing only the factories 

Update `SpringFactoriesLoader` so that the cache stores only the
factories and not the complete loader.

Prior to this commit, if a cache entry was added with the thread
context classloader, the loader instance would be added and the
classloader stored. If the thread context classloader subsequently
changes, and a call is made to `forDefaultResourceLocation` with
`null` for the classloader, the cached entry would be used which
contains the older classloader.

Closes gh-34732
This commit is contained in:
Phillip Webb 2025-04-09 14:36:34 -07:00 committed by Sébastien Deleuze
parent 900dd0c3eb
commit 89b8b1a468
2 changed files with 28 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java
View File

@ -101,7 +101,7 @@ public class SpringFactoriesLoader {
private static final Log logger = LogFactory.getLog(SpringFactoriesLoader.class);
static final Map<ClassLoader, Map<String, SpringFactoriesLoader>> cache = new ConcurrentReferenceHashMap<>();
static final Map<ClassLoader, Map<String, Factories>> cache = new ConcurrentReferenceHashMap<>();
@Nullable
@ -321,10 +321,11 @@ public class SpringFactoriesLoader {
Assert.hasText(resourceLocation, "'resourceLocation' must not be empty");
ClassLoader resourceClassLoader = (classLoader != null ? classLoader :
SpringFactoriesLoader.class.getClassLoader());
Map<String, SpringFactoriesLoader> loaders = cache.computeIfAbsent(
Map<String, Factories> factoriesCache = cache.computeIfAbsent(
resourceClassLoader, key -> new ConcurrentReferenceHashMap<>());
return loaders.computeIfAbsent(resourceLocation, key ->
new SpringFactoriesLoader(classLoader, loadFactoriesResource(resourceClassLoader, resourceLocation)));
Factories factories = factoriesCache.computeIfAbsent(resourceLocation, key ->
new Factories(loadFactoriesResource(resourceClassLoader, resourceLocation)));
return new SpringFactoriesLoader(classLoader, factories.byType());
}
protected static Map<String, List<String>> loadFactoriesResource(ClassLoader classLoader, String resourceLocation) {
@ -667,4 +668,8 @@ public class SpringFactoriesLoader {
}
}
private record Factories(Map<String, List<String>> byType) {
}
}

20
spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java
View File

@ -25,6 +25,7 @@ import java.util.ArrayList;
import java.util.List;
import org.apache.commons.logging.Log;
import org.assertj.core.extractor.Extractors;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Nested;
@ -166,9 +167,26 @@ class SpringFactoriesLoaderTests {
void sameCachedResultIsUsedForDefaultClassLoaderAndNullClassLoader() {
SpringFactoriesLoader forNull = SpringFactoriesLoader.forDefaultResourceLocation(null);
SpringFactoriesLoader forDefault = SpringFactoriesLoader.forDefaultResourceLocation(ClassUtils.getDefaultClassLoader());
assertThat(forNull).isSameAs(forDefault);
assertThat(forNull).extracting("factories").isSameAs(Extractors.byName("factories").apply(forDefault));
}
@Test
void staleClassLoaderIsUsedWithCachedResult() {
ClassLoader defaultClassLoader = ClassUtils.getDefaultClassLoader();
ClassLoader cl1 = new ClassLoader() {
};
SpringFactoriesLoader factories1 = SpringFactoriesLoader.forDefaultResourceLocation(defaultClassLoader);
assertThat(factories1).extracting("classLoader").isEqualTo(defaultClassLoader);
ClassLoader previousClassLoader = Thread.currentThread().getContextClassLoader();
try {
Thread.currentThread().setContextClassLoader(cl1);
SpringFactoriesLoader factories2 = SpringFactoriesLoader.forDefaultResourceLocation(null);
assertThat(factories2).extracting("classLoader").isNull();
}
finally {
Thread.currentThread().setContextClassLoader(previousClassLoader);
}
}
@Nested
class FailureHandlerTests {