diff --git a/core/src/main/java/com/opensymphony/xwork2/util/ConfigParseUtil.java b/core/src/main/java/com/opensymphony/xwork2/util/ConfigParseUtil.java index 4617286a2d..a1977fc424 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/ConfigParseUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/ConfigParseUtil.java @@ -20,6 +20,8 @@ import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.ognl.OgnlUtil; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import java.util.Collection; import java.util.HashSet; @@ -33,6 +35,17 @@ import static org.apache.commons.lang3.StringUtils.strip; public class ConfigParseUtil { + // Size the cache to prevent excessive memory usage in environments with many classloaders and/or large numbers of classes being validated. + // While still providing a reasonable caching benefit for common cases (e.g. multiple Struts instances in the same container, or multiple calls to validate the same class across different containers). + // The cache is sized to allow for some level of caching across multiple classloaders, while still allowing for a reasonable number of classes to be cached per classloader. + private static final int MAX_CLASSLOADER_CACHE_SIZE = 25; + // The cache for validated classes is a two-level cache, with the first level keyed by ClassLoader and the second level keyed by class name. + private static final int MAX_CLASS_CACHE_PER_LOADER_SIZE = 50; + + private static final Cache>> VALIDATED_CLASS_CACHE = Caffeine.newBuilder() + .weakKeys() + .maximumSize(MAX_CLASSLOADER_CACHE_SIZE) + .build(); private ConfigParseUtil() { } @@ -73,7 +86,7 @@ public static Set> validateClasses(Set classNames, ClassLoader Set> classes = new HashSet<>(); for (String className : classNames) { try { - classes.add(validatingClassLoader.loadClass(className)); + classes.add(loadAndCacheClass(validatingClassLoader, className)); } catch (ClassNotFoundException e) { throw new ConfigurationException("Cannot load class for exclusion/exemption configuration: " + className, e); } @@ -81,6 +94,35 @@ public static Set> validateClasses(Set classNames, ClassLoader return classes; } + private static Class loadAndCacheClass(ClassLoader validatingClassLoader, String className) throws ClassNotFoundException { + Cache> classLoaderCache = VALIDATED_CLASS_CACHE.get(validatingClassLoader, + key -> Caffeine.newBuilder().weakValues().maximumSize(MAX_CLASS_CACHE_PER_LOADER_SIZE).build()); + + try { + return classLoaderCache.get(className, key -> { + try { + return validatingClassLoader.loadClass(key); + } catch (ClassNotFoundException e) { + throw new ClassLookupException(e); + } + }); + } catch (ClassLookupException e) { + // The ClassLookupException only serves to wrap the checked ClassNotFoundException thrown by ClassLoader.loadClass. + throw (ClassNotFoundException) e.getCause(); + } + } + + /** + * This is a wrapper class to allow the checked ClassNotFoundException thrown by ClassLoader.loadClass to be propagated + * We should always be able to unwrap this exception without risk of ClassCastException since the only code that can throw it is the mapping function passed to the cache + * and it only ever throws this wrapper with a ClassNotFoundException cause. + */ + private static final class ClassLookupException extends RuntimeException { + private ClassLookupException(ClassNotFoundException cause) { + super(cause); + } + } + public static Set toPackageNamesSet(String newDelimitedPackageNames) throws ConfigurationException { Set packageNames = commaDelimitedStringToSet(newDelimitedPackageNames) .stream().map(s -> strip(s, ".")).collect(toSet()); diff --git a/core/src/test/java/com/opensymphony/xwork2/util/ConfigParseUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/util/ConfigParseUtilTest.java new file mode 100644 index 0000000000..0b27bdf50c --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/util/ConfigParseUtilTest.java @@ -0,0 +1,348 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.opensymphony.xwork2.util; + +import com.github.benmanes.caffeine.cache.Cache; +import com.opensymphony.xwork2.config.ConfigurationException; +import junit.framework.TestCase; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.Set; + +public class ConfigParseUtilTest extends TestCase { + + @Override + protected void setUp() throws Exception { + super.setUp(); + validatedClassCache().invalidateAll(); + } + + @Override + protected void tearDown() throws Exception { + validatedClassCache().invalidateAll(); + super.tearDown(); + } + + public void testValidateClassesCachesByClassLoaderAndClassName() { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "loader-one"); + Set classNames = Collections.singleton(String.class.getName()); + + ConfigParseUtil.validateClasses(classNames, loader); + ConfigParseUtil.validateClasses(classNames, loader); + + assertEquals(1, loader.getStringClassLoads()); + } + + public void testValidateClassesCachesAcrossMultipleRepeatedCallsWithSameClassLoader() { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "loader-one"); + Set classNames = Collections.singleton(String.class.getName()); + + for (int i = 0; i < 10; i++) { + ConfigParseUtil.validateClasses(classNames, loader); + } + + assertEquals(1, loader.getStringClassLoads()); + } + + public void testValidateClassesSeparatesEntriesAcrossDifferentClassLoaders() { + CountingClassLoader firstLoader = new CountingClassLoader(getClass().getClassLoader(), "loader-one"); + CountingClassLoader secondLoader = new CountingClassLoader(getClass().getClassLoader(), "loader-two"); + Set classNames = Collections.singleton(String.class.getName()); + + ConfigParseUtil.validateClasses(classNames, firstLoader); + ConfigParseUtil.validateClasses(classNames, secondLoader); + + assertEquals(1, firstLoader.getStringClassLoads()); + assertEquals(1, secondLoader.getStringClassLoads()); + } + + public void testValidateClassesSeparatesEntriesAcrossDifferentClassLoadersWithSameToString() { + CountingClassLoader firstLoader = new CountingClassLoader(getClass().getClassLoader(), "same-loader-name"); + CountingClassLoader secondLoader = new CountingClassLoader(getClass().getClassLoader(), "same-loader-name"); + Set classNames = Collections.singleton(String.class.getName()); + + ConfigParseUtil.validateClasses(classNames, firstLoader); + ConfigParseUtil.validateClasses(classNames, secondLoader); + + assertEquals(1, firstLoader.getStringClassLoads()); + assertEquals(1, secondLoader.getStringClassLoads()); + } + + public void testValidateClassesEnforcesOuterCacheMaximumSize() { + Set classNames = Collections.singleton(String.class.getName()); + + for (int i = 0; i < 60; i++) { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "loader-" + i); + ConfigParseUtil.validateClasses(classNames, loader); + } + + Cache cache = validatedClassCache(); + cache.cleanUp(); + + assertTrue("Outer cache size should not exceed configured maximum", cache.estimatedSize() <= outerCacheLimit()); + } + + public void testValidateClassesThrowsForNonExistingClassNameOnEachCall() { + String missingClassName = "com.opensymphony.xwork2.util.NonExistingClassForValidationTest"; + Set classNames = Collections.singleton(missingClassName); + int[] missingClassLoads = new int[1]; + ClassLoader loader = new ClassLoader(getClass().getClassLoader()) { + @Override + public Class loadClass(String name) throws ClassNotFoundException { + if (missingClassName.equals(name)) { + missingClassLoads[0]++; + throw new ClassNotFoundException(name); + } + return super.loadClass(name); + } + + @Override + public String toString() { + return "missing-class-loader"; + } + }; + + for (int i = 0; i < 2; i++) { + try { + ConfigParseUtil.validateClasses(classNames, loader); + fail("Expected ConfigurationException for class: " + missingClassName); + } catch (ConfigurationException e) { + assertTrue(e.getMessage().contains(missingClassName)); + assertNotNull(e.getCause()); + assertEquals(ClassNotFoundException.class, e.getCause().getClass()); + } + } + + assertEquals(2, missingClassLoads[0]); + } + + public void testValidateClassesLoadsMultipleDifferentClassesPerLoaderOnce() { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "multi-class-loader"); + Set classNames = new java.util.HashSet<>(); + classNames.add(String.class.getName()); + classNames.add(Integer.class.getName()); + classNames.add(Boolean.class.getName()); + + ConfigParseUtil.validateClasses(classNames, loader); + ConfigParseUtil.validateClasses(classNames, loader); + + // Verify each class was loaded only once per loader through the cache + assertEquals(1, loader.getLoadCount(String.class.getName())); + assertEquals(1, loader.getLoadCount(Integer.class.getName())); + assertEquals(1, loader.getLoadCount(Boolean.class.getName())); + } + + public void testValidateClassesNestedCacheIsReusedForSameLoader() { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "reuse-cache-loader"); + Set classNames = Collections.singleton(String.class.getName()); + + ConfigParseUtil.validateClasses(classNames, loader); + int firstCallLoadCount = loader.getStringClassLoads(); + + ConfigParseUtil.validateClasses(classNames, loader); + int secondCallLoadCount = loader.getStringClassLoads(); + + assertEquals(1, firstCallLoadCount); + assertEquals(1, secondCallLoadCount); + } + + public void testInnerCacheEnforcesMaximumSizePerClassLoader() { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "many-classes-loader"); + Set classNames = new java.util.HashSet<>(); + + // Use real built-in classes that are guaranteed to exist (70+ classes) + String[] realClasses = { + String.class.getName(), Integer.class.getName(), Long.class.getName(), + Double.class.getName(), Float.class.getName(), Boolean.class.getName(), + Character.class.getName(), Byte.class.getName(), Short.class.getName(), + Object.class.getName(), Class.class.getName(), Thread.class.getName(), + Exception.class.getName(), RuntimeException.class.getName(), java.io.File.class.getName(), + java.util.ArrayList.class.getName(), java.util.HashMap.class.getName(), + java.util.HashSet.class.getName(), java.util.LinkedList.class.getName(), + java.util.TreeMap.class.getName(), java.util.regex.Pattern.class.getName(), + java.io.InputStream.class.getName(), java.io.OutputStream.class.getName(), + java.io.Reader.class.getName(), java.io.Writer.class.getName(), + java.net.URL.class.getName(), java.net.URI.class.getName(), + java.nio.file.Path.class.getName(), java.nio.file.Paths.class.getName(), + java.nio.file.Files.class.getName(), java.nio.file.StandardOpenOption.class.getName(), + java.lang.reflect.Method.class.getName(), java.lang.reflect.Field.class.getName(), + java.lang.reflect.Constructor.class.getName(), java.lang.annotation.Annotation.class.getName(), + java.util.concurrent.Future.class.getName(), java.util.concurrent.ExecutorService.class.getName(), + java.time.LocalDate.class.getName(), java.time.LocalTime.class.getName(), + java.time.LocalDateTime.class.getName(), java.time.ZonedDateTime.class.getName(), + java.time.Instant.class.getName(), java.time.Duration.class.getName(), + java.time.Period.class.getName(), java.util.stream.Stream.class.getName(), + java.util.stream.Collectors.class.getName(), java.util.Optional.class.getName(), + java.util.function.Function.class.getName(), java.util.function.Predicate.class.getName(), + java.util.function.Consumer.class.getName(), java.util.function.Supplier.class.getName(), + java.util.function.BiFunction.class.getName(), java.util.function.BiConsumer.class.getName(), + java.util.function.BiPredicate.class.getName(), java.lang.StringBuilder.class.getName(), + java.lang.StringBuffer.class.getName(), java.util.Collections.class.getName(), + java.util.Arrays.class.getName(), java.util.Objects.class.getName(), + java.util.UUID.class.getName(), java.util.Locale.class.getName(), + java.util.TimeZone.class.getName(), java.util.Calendar.class.getName(), + java.util.Date.class.getName(), java.util.GregorianCalendar.class.getName(), + java.util.Random.class.getName(), java.util.Scanner.class.getName(), + java.util.Formatter.class.getName(), java.util.Properties.class.getName(), + java.util.concurrent.ConcurrentHashMap.class.getName(), java.util.concurrent.atomic.AtomicInteger.class.getName(), + java.util.concurrent.atomic.AtomicLong.class.getName(), java.util.concurrent.locks.Lock.class.getName(), + java.util.concurrent.locks.ReentrantLock.class.getName(), java.lang.Comparable.class.getName(), + java.lang.Cloneable.class.getName(), java.io.Serializable.class.getName(), + java.nio.ByteBuffer.class.getName(), java.nio.CharBuffer.class.getName(), + java.nio.charset.Charset.class.getName(), java.security.MessageDigest.class.getName() + }; + + // Add all real classes to the set (should be 72) + for (String className : realClasses) { + classNames.add(className); + } + + assertTrue("Test setup requires more class names than inner cache capacity", classNames.size() > innerCacheLimit()); + + ConfigParseUtil.validateClasses(classNames, loader); + + Cache innerCache = innerCacheFor(loader); + innerCache.cleanUp(); + assertTrue("Inner cache size should not exceed configured maximum per loader", innerCache.estimatedSize() <= innerCacheLimit()); + } + + public void testInnerCacheIndependentPerClassLoader() { + CountingClassLoader loaderA = new CountingClassLoader(getClass().getClassLoader(), "loader-a"); + CountingClassLoader loaderB = new CountingClassLoader(getClass().getClassLoader(), "loader-b"); + + Set classNamesA = new java.util.HashSet<>(); + classNamesA.add(String.class.getName()); + classNamesA.add(Integer.class.getName()); + + Set classNamesB = new java.util.HashSet<>(); + classNamesB.add(Boolean.class.getName()); + classNamesB.add(Double.class.getName()); + + ConfigParseUtil.validateClasses(classNamesA, loaderA); + ConfigParseUtil.validateClasses(classNamesB, loaderB); + + // Validate load counts show independent caching - each loader loaded its own classes once + assertEquals(1, loaderA.getLoadCount(String.class.getName())); + assertEquals(1, loaderA.getLoadCount(Integer.class.getName())); + assertEquals(0, loaderB.getLoadCount(String.class.getName())); + assertEquals(1, loaderB.getLoadCount(Boolean.class.getName())); + assertEquals(1, loaderB.getLoadCount(Double.class.getName())); + } + + public void testInnerCacheReusesCachedClassLookupForSameLoader() { + CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "reload-test-loader"); + Set classNames = Collections.singleton(String.class.getName()); + + // Load once + ConfigParseUtil.validateClasses(classNames, loader); + assertEquals(1, loader.getStringClassLoads()); + + // Reload - should use cache + ConfigParseUtil.validateClasses(classNames, loader); + assertEquals(1, loader.getStringClassLoads()); + } + + public void testMultipleClassLoadersWithDistinctInnerCaches() { + CountingClassLoader loader1 = new CountingClassLoader(getClass().getClassLoader(), "distinct-1"); + CountingClassLoader loader2 = new CountingClassLoader(getClass().getClassLoader(), "distinct-2"); + CountingClassLoader loader3 = new CountingClassLoader(getClass().getClassLoader(), "distinct-3"); + + Set classNames = Collections.singleton(String.class.getName()); + + ConfigParseUtil.validateClasses(classNames, loader1); + ConfigParseUtil.validateClasses(classNames, loader2); + ConfigParseUtil.validateClasses(classNames, loader3); + + // Each loader should have loaded String once independently + assertEquals(1, loader1.getLoadCount(String.class.getName())); + assertEquals(1, loader2.getLoadCount(String.class.getName())); + assertEquals(1, loader3.getLoadCount(String.class.getName())); + + // Revalidating with loader1 should still use cache + ConfigParseUtil.validateClasses(classNames, loader1); + assertEquals(1, loader1.getLoadCount(String.class.getName())); + } + + @SuppressWarnings("unchecked") + private static Cache validatedClassCache() { + try { + Field cacheField = ConfigParseUtil.class.getDeclaredField("VALIDATED_CLASS_CACHE"); + cacheField.setAccessible(true); + return (Cache) cacheField.get(null); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new AssertionError("Cannot access ConfigParseUtil cache field", e); + } + } + + @SuppressWarnings("unchecked") + private static Cache innerCacheFor(ClassLoader loader) { + Cache outer = validatedClassCache(); + Object inner = outer.getIfPresent(loader); + assertNotNull("Expected an inner cache entry for loader", inner); + return (Cache) inner; + } + + private static int outerCacheLimit() { + return intConstant("MAX_CLASSLOADER_CACHE_SIZE"); + } + + private static int innerCacheLimit() { + return intConstant("MAX_CLASS_CACHE_PER_LOADER_SIZE"); + } + + private static int intConstant(String fieldName) { + try { + Field field = ConfigParseUtil.class.getDeclaredField(fieldName); + field.setAccessible(true); + return field.getInt(null); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new AssertionError("Cannot access ConfigParseUtil constant: " + fieldName, e); + } + } + + private static final class CountingClassLoader extends ClassLoader { + private final String loaderName; + private final java.util.Map loadCounts = new java.util.HashMap<>(); + + private CountingClassLoader(ClassLoader parent, String loaderName) { + super(parent); + this.loaderName = loaderName; + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + loadCounts.merge(name, 1, Integer::sum); + return super.loadClass(name); + } + + private int getStringClassLoads() { + return loadCounts.getOrDefault(String.class.getName(), 0); + } + + private int getLoadCount(String className) { + return loadCounts.getOrDefault(className, 0); + } + + @Override + public String toString() { + return loaderName; + } + } +}