From 8f1aed6ccf2878d1791ce64fb81627affa1d9275 Mon Sep 17 00:00:00 2001 From: Brian Andle Date: Thu, 28 May 2026 20:32:49 -0700 Subject: [PATCH 1/3] WW-5630 - Performance Issue SecurityMemberAccess * Add size bound cache, 50, for Class lookup * Add unit test Code generated by Copilot --- .../apache/struts2/util/ConfigParseUtil.java | 65 +++++++++- .../struts2/util/ConfigParseUtilTest.java | 115 ++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java diff --git a/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java b/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java index d6cdafabf3..8d166aa215 100644 --- a/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java +++ b/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java @@ -18,11 +18,14 @@ */ 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; import java.util.Collection; import java.util.HashSet; +import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; @@ -33,6 +36,13 @@ 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> VALIDATED_CLASS_CACHE = Caffeine.newBuilder() + .maximumSize(MAX_CLASS_CACHE_SIZE) + .build(); private ConfigParseUtil() { } @@ -73,7 +83,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 +91,59 @@ public static Set> validateClasses(Set classNames, ClassLoader return classes; } + private static Class loadAndCacheClass(ClassLoader validatingClassLoader, String className) throws ClassNotFoundException { + ClassLookupKey lookupKey = new ClassLookupKey(classLoaderName(validatingClassLoader), className); + + try { + return VALIDATED_CLASS_CACHE.get(lookupKey, key -> { + try { + return validatingClassLoader.loadClass(key.className); + } catch (ClassNotFoundException e) { + throw new ClassLookupException(e); + } + }); + } catch (ClassLookupException e) { + throw (ClassNotFoundException) e.getCause(); + } + } + + private static String classLoaderName(ClassLoader classLoader) { + return String.valueOf(classLoader); + } + + private static final class ClassLookupKey { + private final String classLoaderName; + private final String className; + + private ClassLookupKey(String classLoaderName, String className) { + this.classLoaderName = classLoaderName; + this.className = className; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ClassLookupKey that = (ClassLookupKey) o; + return Objects.equals(classLoaderName, that.classLoaderName) && Objects.equals(className, that.className); + } + + @Override + public int hashCode() { + return Objects.hash(classLoaderName, className); + } + } + + 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/org/apache/struts2/util/ConfigParseUtilTest.java b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java new file mode 100644 index 0000000000..2fa4843a06 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java @@ -0,0 +1,115 @@ +/* + * 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 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 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 testValidateClassesCacheIsLimitedTo50Entries() { + 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(cache.estimatedSize() <= 50); + } + + @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); + } + } + + 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; + } + } +} From 74bdca9b6c41963cf59c962d091162548340efe2 Mon Sep 17 00:00:00 2001 From: Brian Andle Date: Thu, 28 May 2026 20:38:23 -0700 Subject: [PATCH 2/3] WW-5630 - Add additional UT --- .../org/apache/struts2/util/ConfigParseUtilTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java index 2fa4843a06..cbc31f8439 100644 --- a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java +++ b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java @@ -49,6 +49,17 @@ public void testValidateClassesCachesByClassLoaderAndClassName() { 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"); From 0b303f1374428fbb1f1d336517c0526bf9be595c Mon Sep 17 00:00:00 2001 From: Brian Andle Date: Thu, 28 May 2026 20:46:17 -0700 Subject: [PATCH 3/3] WW-5630 - Add UT for non-existent class --- .../struts2/util/ConfigParseUtilTest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java index cbc31f8439..64063c4ae8 100644 --- a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java +++ b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java @@ -20,6 +20,7 @@ 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; @@ -86,6 +87,40 @@ public void testValidateClassesCacheIsLimitedTo50Entries() { assertTrue(cache.estimatedSize() <= 50); } + public void testValidateClassesThrowsForNonExistingClassNameOnEachCall() { + String missingClassName = "org.apache.struts2.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]); + } + @SuppressWarnings("unchecked") private static Cache validatedClassCache() { try {