Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -310,22 +310,31 @@ protected boolean checkExclusionList(Object target, Member member) {
}

/**
* Blocks access to classes in the default (unnamed) package.
* <p>
* The emptiness of {@link #toPackageName(Class)} is the same test as the
* {@code getPackage() == null || getPackage().getName().isEmpty()} form this replaced, for every
* class shape: {@code getPackage()} is null for arrays, primitives and {@code void}, and names the
* unnamed package with the empty string, all of which {@code toPackageName} reports as empty. It
* avoids the {@code getPackage()} lookup through the defining classloader's package map, which
* ran twice per class here. See WW-5677.
*
* @return {@code true} if member access is allowed
*/
protected boolean checkDefaultPackageAccess(Object target, Member member) {
if (!disallowDefaultPackageAccess) {
return true;
}
Class<?> memberClass = member.getDeclaringClass();
if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
if (toPackageName(memberClass).isEmpty()) {
LOG.warn("Class [{}] from the default package is excluded!", memberClass);
return false;
}
if (target == null || target.getClass() == memberClass) {
return true;
}
Class<?> targetClass = target.getClass();
if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
if (toPackageName(targetClass).isEmpty()) {
LOG.warn("Class [{}] from the default package is excluded!", targetClass);
return false;
}
Expand Down Expand Up @@ -407,7 +416,9 @@ public static String toPackageName(Class<?> clazz) {
}

protected boolean isExcludedPackageNamePatterns(Class<?> clazz) {
return excludedPackageNamePatterns.stream().anyMatch(pattern -> pattern.matcher(toPackageName(clazz)).matches());
// Resolved once rather than inside the lambda, which re-resolved it per configured pattern.
String packageName = toPackageName(clazz);
return excludedPackageNamePatterns.stream().anyMatch(pattern -> pattern.matcher(packageName).matches());
}

protected boolean isExcludedPackageNames(Class<?> clazz) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ private static boolean legacyPrefixMatch(String packageName, Set<String> matchin
.anyMatch(matchingPackages::contains);
}

/**
* The default-package condition replaced by WW-5677 in {@code checkDefaultPackageAccess},
* retained verbatim as the reference oracle. Deliberately calls {@link Class#getPackage()}
* directly rather than delegating to production code, so that it cannot drift with it.
*/
private static boolean legacyDefaultPackageCondition(Class<?> clazz) {
return clazz.getPackage() == null || clazz.getPackage().getName().isEmpty();
}

/**
* The {@code toPackageName} implementation replaced by WW-5674, retained as the reference oracle.
*/
Expand Down Expand Up @@ -168,6 +177,21 @@ public void toPackageNameMatchesLegacyAcrossClassShapes() throws Exception {
}
}

/**
* WW-5677 replaced {@code getPackage() == null || getPackage().getName().isEmpty()} in
* {@code checkDefaultPackageAccess} with {@code toPackageName(clazz).isEmpty()}. That gate
* decides whether a class counts as living in the default package, so the equivalence is
* asserted against the frozen oracle over every class shape rather than argued.
*/
@Test
public void defaultPackageConditionMatchesLegacyAcrossClassShapes() throws Exception {
for (Class<?> clazz : classShapes()) {
assertThat(toPackageName(clazz).isEmpty())
.as("default-package condition for %s", clazz.getName())
.isEqualTo(legacyDefaultPackageCondition(clazz));
}
}

@Test
public void arraysAndPrimitivesResolveToTheEmptyPackage() {
assertThat(toPackageName(int.class)).isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,33 @@
assertFalse("default package isn't excluded!", actual);
}

/**
* WW-5677 routed {@code checkDefaultPackageAccess} through {@code toPackageName}. A class in a
* named package must still pass the gate when the setting is on.
*/
@Test
public void testDefaultPackageAccessPermitsNamedPackageClass() throws Exception {
sma.useDisallowDefaultPackageAccess(Boolean.TRUE.toString());

Check warning on line 413 in core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this call to a deprecated method, it has been marked for removal.

See more on https://sonarcloud.io/project/issues?id=apache_struts&issues=AaAvhihQqMyWNFuzPSTn&open=AaAvhihQqMyWNFuzPSTn&pullRequest=1859

Member member = FooBar.class.getMethod("getStringField");

assertTrue("a named-package class is blocked!", sma.checkDefaultPackageAccess(new FooBar(), member));
}

/**
* WW-5677 equivalence at the shape most likely to break it: an array target reports the empty
* package under both the old {@code getPackage() == null} form and the new one, so it must stay
* blocked. The member here declares in {@code java.lang}, so only the target branch can block.
*/
@Test
public void testDefaultPackageAccessBlocksArrayTarget() throws Exception {
sma.useDisallowDefaultPackageAccess(Boolean.TRUE.toString());

Check warning on line 427 in core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this call to a deprecated method, it has been marked for removal.

See more on https://sonarcloud.io/project/issues?id=apache_struts&issues=AaAvhihQqMyWNFuzPSTo&open=AaAvhihQqMyWNFuzPSTo&pullRequest=1859

Member member = Object.class.getMethod("toString");

assertFalse("an array target isn't blocked!", sma.checkDefaultPackageAccess(new String[0], member));
}

@Test
public void testDefaultPackageExclusion2() throws Exception {
// given
Expand Down
Loading