mirror of
https://github.com/apache/struts.git
synced 2026-08-06 15:17:00 +00:00
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.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
+16
-4
@@ -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(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();
|
||||
}
|
||||
|
||||
|
||||
+7
-3
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user