WW-5630 - Performance Issue SecurityMemberAccess

core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java

@@ -18,6 +18,8 @@
  */
 package org.apache.struts2.util;
 
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
 import org.apache.struts2.config.ConfigurationException;
 import org.apache.struts2.ognl.OgnlUtil;
 
@@ -33,6 +35,15 @@
 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).
+    private static final int MAX_CLASS_CACHE_SIZE = 50;
+
+    private static final Cache<ClassLoader, Cache<String, Class<?>>> VALIDATED_CLASS_CACHE = Caffeine.newBuilder()
+            .weakKeys()
+            .weakValues()
+            .maximumSize(MAX_CLASS_CACHE_SIZE)
+            .build();
 
     private ConfigParseUtil() {
     }
@@ -73,14 +84,43 @@ public static Set<Class<?>> validateClasses(Set<String> classNames, ClassLoader
         Set<Class<?>> 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);
             }
         }
         return classes;
     }
 
+    private static Class<?> loadAndCacheClass(ClassLoader validatingClassLoader, String className) throws ClassNotFoundException {
+        Cache<String, Class<?>> classLoaderCache = VALIDATED_CLASS_CACHE.get(validatingClassLoader,
+                key -> Caffeine.newBuilder().weakValues().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<String> toPackageNamesSet(String newDelimitedPackageNames) throws ConfigurationException {
         Set<String> packageNames = commaDelimitedStringToSet(newDelimitedPackageNames)
                 .stream().map(s -> strip(s, ".")).collect(toSet());

core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java

@@ -0,0 +1,213 @@
+/*
+ * 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 org.apache.struts2.util;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import junit.framework.TestCase;
+import org.apache.struts2.config.ConfigurationException;
+
+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<String> 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<String> 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<String> 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<String> classNames = Collections.singleton(String.class.getName());
+
+        ConfigParseUtil.validateClasses(classNames, firstLoader);
+        ConfigParseUtil.validateClasses(classNames, secondLoader);
+
+        assertEquals(1, firstLoader.getStringClassLoads());
+        assertEquals(1, secondLoader.getStringClassLoads());
+    }
+
+    public void testValidateClassesCacheIsLimitedTo50Entries() {
+        Set<String> 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<Object, Object> cache = validatedClassCache();
+        cache.cleanUp();
+
+        assertTrue(cache.estimatedSize() <= 50);
+    }
+
+    public void testValidateClassesThrowsForNonExistingClassNameOnEachCall() {
+        String missingClassName = "org.apache.struts2.util.NonExistingClassForValidationTest";
+        Set<String> 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<String> 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);
+
+        assertEquals(1, loader.getStringClassLoads());
+    }
+
+    public void testValidateClassesNestedCacheIsReusedForSameLoader() {
+        CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "reuse-cache-loader");
+        Set<String> 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 testValidateClassesEnforcesMaximumCacheSize() {
+        Set<String> classNames = Collections.singleton(String.class.getName());
+        Cache<Object, Object> cache = validatedClassCache();
+
+        for (int i = 0; i < 60; i++) {
+            CountingClassLoader loader = new CountingClassLoader(getClass().getClassLoader(), "size-limit-loader-" + i);
+            ConfigParseUtil.validateClasses(classNames, loader);
+        }
+
+        cache.cleanUp();
+        assertTrue("Cache size should not exceed 50 after loading 60 loaders", cache.estimatedSize() <= 50);
+    }
+
+    @SuppressWarnings("unchecked")
+    private static Cache<Object, Object> validatedClassCache() {
+        try {
+            Field cacheField = ConfigParseUtil.class.getDeclaredField("VALIDATED_CLASS_CACHE");
+            cacheField.setAccessible(true);
+            return (Cache<Object, Object>) cacheField.get(null);
+        } catch (NoSuchFieldException | IllegalAccessException e) {
+            throw new AssertionError("Cannot access ConfigParseUtil cache field", e);
+        }
+    }
+
+    private static final class CountingClassLoader extends ClassLoader {
+        private final String loaderName;
+        private int stringClassLoads;
+
+        private CountingClassLoader(ClassLoader parent, String loaderName) {
+            super(parent);
+            this.loaderName = loaderName;
+        }
+
+        @Override
+        public Class<?> loadClass(String name) throws ClassNotFoundException {
+            if (String.class.getName().equals(name)) {
+                stringClassLoads++;
+            }
+            return super.loadClass(name);
+        }
+
+        private int getStringClassLoads() {
+            return stringClassLoads;
+        }
+
+        @Override
+        public String toString() {
+            return loaderName;
+        }
+    }
+}