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 b18e568fc..ffc7d5e89 100644 --- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java @@ -403,7 +403,7 @@ public class SecurityMemberAccess implements MemberAccess { * @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 first, Set second) { + static boolean isClassBelongsToPackages(Class clazz, Set first, Set 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 361a1c43b..413b0c669 100644 --- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java @@ -89,6 +89,7 @@ public class SecurityMemberAccessPackageMatchingTest { 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")); @@ -116,14 +117,25 @@ public class SecurityMemberAccessPackageMatchingTest { public void siblingPackageWithSharedCharacterPrefixDoesNotMatch() { Set excluded = Set.of("org.apache.struts2"); - assertThat(legacyPrefixMatch("org.apache.struts2x", excluded)) - .as("a sibling package sharing a character prefix must not match") + 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") + .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") + .as("a sub-package must match (legacy oracle)") .isTrue(); } diff --git a/docs/superpowers/specs/2026-08-03-WW-5674-isclassbelongstopackages-allocation-design.md b/docs/superpowers/specs/2026-08-03-WW-5674-isclassbelongstopackages-allocation-design.md index f156d0804..5666a44e1 100644 --- a/docs/superpowers/specs/2026-08-03-WW-5674-isclassbelongstopackages-allocation-design.md +++ b/docs/superpowers/specs/2026-08-03-WW-5674-isclassbelongstopackages-allocation-design.md @@ -125,7 +125,7 @@ Verified across eleven class shapes: | `String` | non-null | `"java.lang"` | `"java.lang"` | | default-package class | non-null | `""` | `""` | | nested (`Map.Entry`) | non-null | `"java.util"` | `"java.util"` | -| lambda (hidden class) | non-null | `""` | `""` | +| lambda (hidden class) | non-null | `"org.apache.struts2.ognl"` | `"org.apache.struts2.ognl"` | | JDK proxy | non-null | `"jdk.proxy1"` | `"jdk.proxy1"` | | `int`, `void` | null | `""` | `""` | | `int[]`, `String[]`, `String[][]` | null | `""` | `""` | @@ -183,8 +183,12 @@ Shortest-prefix-first ordering is preserved. Ordering does not affect the result exclusions such as `java.io`, which are the common case. The `isEmpty()` short-circuit skips the walk โ€” and therefore every substring -allocation โ€” when neither set is configured. `allowlistPackageNames` is empty by -default, so this is the common path for the allowlist check. +allocation โ€” when neither set is configured. That requires both sets to be +empty, so it does not fire on the allowlist path, where +`ALLOWLIST_REQUIRED_PACKAGES` is always non-empty (see ยง4), nor on the +exclusion path under the shipped configuration, where +`struts.excludedPackageNames` carries roughly thirty entries by default. It +protects deployments that configure both sets empty. ### 3. Both public entry points delegate to it