This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5674-isclassbelongstopackages-allocation in repository https://gitbox.apache.org/repos/asf/struts.git
commit 34e5cf8b2a5c3add24211b013761a892fef238f4 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Aug 3 13:12:27 2026 +0200 WW-5674 perf(ognl): match both allowlist package sets in one walk isClassAllowlisted walked the class's package name twice, once for ALLOWLIST_REQUIRED_PACKAGES and once for the configured allowlist. A two-set overload probes both sets at each prefix, halving the work on a path that runs for every OGNL member access. Asserted equivalent to OR-ing the two single-set calls across a matrix of class shapes and candidate sets. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../org/apache/struts2/ognl/SecurityMemberAccess.java | 19 ++++++++++++++++--- .../ognl/SecurityMemberAccessPackageMatchingTest.java | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java index e41004a2b..b18e568fc 100644 --- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java @@ -254,8 +254,7 @@ public class SecurityMemberAccess implements MemberAccess { || ALLOWLIST_REQUIRED_CLASSES.contains(clazz) || (providerAllowlist != null && providerAllowlist.getProviderAllowlist().contains(clazz)) || (threadAllowlist != null && threadAllowlist.getAllowlist().contains(clazz)) - || isClassBelongsToPackages(clazz, ALLOWLIST_REQUIRED_PACKAGES) - || isClassBelongsToPackages(clazz, allowlistPackageNames); + || isClassBelongsToPackages(clazz, ALLOWLIST_REQUIRED_PACKAGES, allowlistPackageNames); } /** @@ -391,7 +390,21 @@ public class SecurityMemberAccess implements MemberAccess { } public static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> matchingPackages) { - return isPackageBelongsToPackages(toPackageName(clazz), matchingPackages, emptySet()); + return isClassBelongsToPackages(clazz, matchingPackages, emptySet()); + } + + /** + * Tests the class's package against two sets in a single walk. Equivalent to calling + * {@link #isClassBelongsToPackages(Class, Set)} once per set and OR-ing the results, but + * walks the package name only once. + * + * @param clazz the class whose package is tested + * @param first the first set of package names to match against + * @param second the second set of package names to match against + * @return {@code true} if the class's package or any parent package is in either set + */ + public static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> first, Set<String> second) { + return isPackageBelongsToPackages(toPackageName(clazz), first, second); } /** diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java index 04e2da96c..361a1c43b 100644 --- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java @@ -195,4 +195,18 @@ public class SecurityMemberAccessPackageMatchingTest { .isFalse(); } } + + @Test + public void twoSetOverloadEqualsDisjunctionOfSingleSetCalls() throws Exception { + for (Class<?> clazz : classShapes()) { + for (Set<String> first : CANDIDATE_SETS) { + for (Set<String> second : CANDIDATE_SETS) { + assertThat(isClassBelongsToPackages(clazz, first, second)) + .as("clazz=[%s] first=%s second=%s", clazz.getName(), first, second) + .isEqualTo(isClassBelongsToPackages(clazz, first) + || isClassBelongsToPackages(clazz, second)); + } + } + } + } }
