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 866722bbb6b93d93c2c70ed3f39ead1268238fbd Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Aug 3 13:28:48 2026 +0200 WW-5674 test(ognl): assert package-boundary case against the live gate The sibling-package test asserted only against the test-local copy of the replaced implementation, so it would have stayed green even if the production walk were gutted. It now asserts on both the live helper and the frozen oracle. Also narrows the three-argument isClassBelongsToPackages overload to package-private: it has a single caller and its test is in the same package, and public static on a public class is frozen API until the next major release. Adds a candidate set that makes the consecutive-dot prefix the deciding probe, and corrects two inaccuracies in the design document. --- .../org/apache/struts2/ognl/SecurityMemberAccess.java | 2 +- .../ognl/SecurityMemberAccessPackageMatchingTest.java | 18 +++++++++++++++--- ...-5674-isclassbelongstopackages-allocation-design.md | 10 +++++++--- 3 files changed, 23 insertions(+), 7 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 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<String> first, Set<String> second) { + 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 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<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") + .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
