WW-5674 Cut the per-call allocations in SecurityMemberAccess package matching

core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java

@@ -32,11 +32,9 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Member;
 import java.lang.reflect.Modifier;
-import java.util.List;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.IntStream;
 
 import static java.text.MessageFormat.format;
 import static java.util.Collections.emptySet;
@@ -256,8 +254,7 @@ protected boolean isClassAllowlisted(Class<?> clazz) {
                 || 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);
     }
 
     /**
@@ -372,10 +369,16 @@ protected boolean isPackageExcluded(Class<?> clazz) {
     }
 
     public static String toPackageName(Class<?> clazz) {
-        if (clazz.getPackage() == null) {
+        // Class.getPackage() resolves through the defining classloader's package map on every
+        // call, whereas getPackageName() is computed once and cached on the Class. getPackage()
+        // returns null for exactly arrays, primitives and void, so the guard reproduces the
+        // previous result for every input. Note that void.class.isPrimitive() is true.
+        // Arrays deliberately keep the empty package here: getPackageName() would resolve them
+        // to the element type's package, which would loosen the allowlist. See WW-5674.
+        if (clazz.isArray() || clazz.isPrimitive()) {
             return "";
         }
-        return clazz.getPackage().getName();
+        return clazz.getPackageName();
     }
 
     protected boolean isExcludedPackageNamePatterns(Class<?> clazz) {
@@ -387,10 +390,54 @@ protected boolean isExcludedPackageNames(Class<?> clazz) {
     }
 
     public static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> matchingPackages) {
-        List<String> packageParts = List.of(toPackageName(clazz).split("\\."));
-        return IntStream.range(0, packageParts.size())
-                .mapToObj(i -> String.join(".", packageParts.subList(0, i + 1)))
-                .anyMatch(matchingPackages::contains);
+        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
+     */
+    static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> first, Set<String> second) {
+        return isPackageBelongsToPackages(toPackageName(clazz), first, second);
+    }
+
+    /**
+     * Tests whether the given package name, or any of its parent packages, is present in either
+     * set. Walks the name in place rather than building the full prefix list, since this runs on
+     * the OGNL member-access path. Shortest prefix first, so broad entries such as {@code java.io}
+     * short-circuit earliest.
+     *
+     * <p>
+     * The package name must not end in {@code '.'}. Such a name is probed one prefix more than by
+     * the implementation this replaced, which matches more broadly — tightening exclusion but
+     * <em>loosening</em> the allowlist. {@link Class#getPackageName()} cannot produce a trailing
+     * dot, so every current caller is safe; route any other string through here only after
+     * confirming the same.
+     *
+     * @param packageName the package name to test, empty for the default package, never ending in {@code '.'}
+     * @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 package or any parent package is in either set
+     */
+    static boolean isPackageBelongsToPackages(String packageName, Set<String> first, Set<String> second) {
+        if (first.isEmpty() && second.isEmpty()) {
+            return false;
+        }
+        int idx = packageName.indexOf('.');
+        while (idx != -1) {
+            String prefix = packageName.substring(0, idx);
+            if (first.contains(prefix) || second.contains(prefix)) {
+                return true;
+            }
+            idx = packageName.indexOf('.', idx + 1);
+        }
+        return first.contains(packageName) || second.contains(packageName);
     }
 
     protected boolean isClassExcluded(Class<?> clazz) {

core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java

@@ -0,0 +1,224 @@
+/*
+ * 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.ognl;
+
+import org.apache.struts2.util.ConfigParseUtil;
+import org.junit.Test;
+
+import java.lang.reflect.Proxy;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.IntStream;
+
+import static java.util.Collections.emptySet;
+import static org.apache.struts2.ognl.SecurityMemberAccess.isClassBelongsToPackages;
+import static org.apache.struts2.ognl.SecurityMemberAccess.toPackageName;
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Characterisation and equivalence tests for the static package-matching helpers in
+ * {@link SecurityMemberAccess}, covering WW-5674.
+ * <p>
+ * These helpers gate OGNL member access, so the rewrite in WW-5674 must be exactly
+ * behaviour-preserving. That is proven here by running the replaced implementation
+ * side by side with the new one over a matrix of inputs.
+ */
+public class SecurityMemberAccessPackageMatchingTest {
+
+    /**
+     * The implementation replaced by WW-5674, retained verbatim apart from taking the package
+     * name directly instead of a {@link Class}. Used as the reference oracle for the rewrite.
+     */
+    private static boolean legacyPrefixMatch(String packageName, Set<String> matchingPackages) {
+        List<String> packageParts = List.of(packageName.split("\\."));
+        return IntStream.range(0, packageParts.size())
+                .mapToObj(i -> String.join(".", packageParts.subList(0, i + 1)))
+                .anyMatch(matchingPackages::contains);
+    }
+
+    /**
+     * The {@code toPackageName} implementation replaced by WW-5674, retained as the reference oracle.
+     */
+    private static String legacyToPackageName(Class<?> clazz) {
+        if (clazz.getPackage() == null) {
+            return "";
+        }
+        return clazz.getPackage().getName();
+    }
+
+    /**
+     * Package-name shapes. Deliberately excludes trailing-dot inputs such as {@code "a.b."}:
+     * {@code split} drops trailing empty segments where an index walk would not, and
+     * {@code Class.getPackage().getName()} cannot produce a trailing dot, so the shape is
+     * unreachable through every caller. See the spec's "Verified current semantics" section.
+     */
+    private static final List<String> PACKAGE_NAMES = List.of(
+            "",
+            "java",
+            "a.b.c",
+            "a..b",
+            ".a",
+            "org.apache.struts2",
+            "org.apache.struts2.ognl",
+            "org.apache.struts2x",
+            "java.io",
+            "java.io.tmp",
+            "javax.servlet.http");
+
+    private static final List<Set<String>> CANDIDATE_SETS = List.of(
+            emptySet(),
+            Set.of(""),
+            Set.of("java"),
+            Set.of("java.io"),
+            Set.of("org.apache.struts2"),
+            Set.of("a"),
+            Set.of("a."),
+            Set.of("a.b"),
+            Set.of("zzz.not.matching"),
+            Set.of("java.io", "org.apache.struts2", "javax"));
+
+    private static List<Class<?>> classShapes() throws Exception {
+        return List.of(
+                String.class,
+                Map.Entry.class,
+                SecurityMemberAccess.class,
+                Class.forName("PackagelessAction"),
+                int.class,
+                void.class,
+                int[].class,
+                String[].class,
+                String[][].class,
+                ((Runnable) () -> {
+                }).getClass(),
+                Proxy.newProxyInstance(
+                        SecurityMemberAccessPackageMatchingTest.class.getClassLoader(),
+                        new Class<?>[]{Runnable.class},
+                        (proxy, method, args) -> null).getClass());
+    }
+
+    @Test
+    public void siblingPackageWithSharedCharacterPrefixDoesNotMatch() {
+        Set<String> excluded = Set.of("org.apache.struts2");
+
+        assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2x", excluded, emptySet()))
+                .as("a sibling package sharing a character prefix must not match (production)")
+                .isFalse();
+        assertThat(legacyPrefixMatch("org.apache.struts2x", excluded))
+                .as("a sibling package sharing a character prefix must not match (legacy oracle)")
+                .isFalse();
+
+        assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2", excluded, emptySet()))
+                .as("an exact match must match (production)")
+                .isTrue();
+        assertThat(legacyPrefixMatch("org.apache.struts2", excluded))
+                .as("an exact match must match (legacy oracle)")
+                .isTrue();
+
+        assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2.ognl", excluded, emptySet()))
+                .as("a sub-package must match (production)")
+                .isTrue();
+        assertThat(legacyPrefixMatch("org.apache.struts2.ognl", excluded))
+                .as("a sub-package must match (legacy oracle)")
+                .isTrue();
+    }
+
+    @Test
+    public void dotOnlyConfigurationYieldsEmptyStringPackageName() {
+        assertThat(ConfigParseUtil.toPackageNamesSet("."))
+                .as("struts.excludedPackageNames=\".\" strips to the empty string")
+                .containsExactly("");
+    }
+
+    @Test
+    public void defaultPackageMatchesOnlyWhenEmptyStringConfigured() throws Exception {
+        Class<?> packageless = Class.forName("PackagelessAction");
+
+        assertThat(toPackageName(packageless)).isEmpty();
+        assertThat(isClassBelongsToPackages(packageless, Set.of("")))
+                .as("a default-package class is matched by the empty-string entry")
+                .isTrue();
+        assertThat(isClassBelongsToPackages(packageless, Set.of("java")))
+                .as("a default-package class is not matched by an unrelated entry")
+                .isFalse();
+    }
+
+    @Test
+    public void toPackageNameMatchesLegacyAcrossClassShapes() throws Exception {
+        for (Class<?> clazz : classShapes()) {
+            assertThat(toPackageName(clazz))
+                    .as("toPackageName(%s)", clazz.getName())
+                    .isEqualTo(legacyToPackageName(clazz));
+        }
+    }
+
+    @Test
+    public void arraysAndPrimitivesResolveToTheEmptyPackage() {
+        assertThat(toPackageName(int.class)).isEmpty();
+        assertThat(toPackageName(void.class)).isEmpty();
+        assertThat(toPackageName(int[].class)).isEmpty();
+        assertThat(toPackageName(String[].class)).isEmpty();
+        assertThat(toPackageName(String[][].class)).isEmpty();
+    }
+
+    @Test
+    public void classEntryPointMatchesLegacyAcrossCandidateSets() throws Exception {
+        for (Class<?> clazz : classShapes()) {
+            for (Set<String> candidates : CANDIDATE_SETS) {
+                assertThat(isClassBelongsToPackages(clazz, candidates))
+                        .as("clazz=[%s] candidates=%s", clazz.getName(), candidates)
+                        .isEqualTo(legacyPrefixMatch(legacyToPackageName(clazz), candidates));
+            }
+        }
+    }
+
+    @Test
+    public void indexWalkMatchesLegacyAcrossPackageNameShapes() {
+        for (String packageName : PACKAGE_NAMES) {
+            for (Set<String> candidates : CANDIDATE_SETS) {
+                assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, candidates, emptySet()))
+                        .as("packageName=[%s] candidates=%s", packageName, candidates)
+                        .isEqualTo(legacyPrefixMatch(packageName, candidates));
+            }
+        }
+    }
+
+    @Test
+    public void bothSetsEmptyShortCircuitsToFalse() {
+        for (String packageName : PACKAGE_NAMES) {
+            assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, emptySet(), emptySet()))
+                    .as("packageName=[%s] with no configured packages", packageName)
+                    .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));
+                }
+            }
+        }
+    }
+}