Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
In TypesTest, change the check for correct functioning in the presenc…
…e of a SecurityManager. Previously we had a SecurityManager that refused Method.setAccessible except in a few well-known places, the intent being that one of the places it would refuse would be the use in Types.TypeVariableInvocationHandler that we wanted to test. But that proved to be fragile. Instead, we explicitly refuse that place, and we check that the refusal happened. This builds more knowledge of the implementation into the test, but should be more robust.

-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=94699561
  • Loading branch information
eamonnmcmanus authored and cpovirk committed Jun 1, 2015
1 parent cc9a685 commit cc65b4f
Showing 1 changed file with 31 additions and 13 deletions.
44 changes: 31 additions & 13 deletions guava-tests/test/com/google/common/reflect/TypesTest.java
Expand Up @@ -19,8 +19,6 @@
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;


import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.testing.EqualsTester; import com.google.common.testing.EqualsTester;
import com.google.common.testing.NullPointerTester; import com.google.common.testing.NullPointerTester;
Expand All @@ -36,6 +34,7 @@
import java.lang.reflect.Type; import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable; import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType; import java.lang.reflect.WildcardType;
import java.security.AccessControlException;
import java.security.Permission; import java.security.Permission;
import java.util.Arrays; import java.util.Arrays;
import java.util.HashMap; import java.util.HashMap;
Expand All @@ -51,28 +50,42 @@
public class TypesTest extends TestCase { public class TypesTest extends TestCase {
private SecurityManager oldSecurityManager; private SecurityManager oldSecurityManager;


// This must be static because the point where it is needed may be earlier than the execution
// of the specific test case that uses it. Currently we need it to operate in the static
// initializer of TypeVariableInvocationHandler.
private static final NoSetAccessibleSecurityManager securityManager =
new NoSetAccessibleSecurityManager();

/** /**
* A SecurityManager that disallows {@link java.lang.reflect.Method#setAccessible(boolean)} * A SecurityManager that disallows {@link java.lang.reflect.Method#setAccessible(boolean)}
* except when it comes from one of a set of known classes. The purpose is to detect whether we * when it comes from a specific place. The idea is to ensure that
* do a setAccessible call that might fail in a context where a SecurityManager forbids it. * {@code Types.newTypeVariableImpl}, which calls {@code setAccessible}, can still work if that
* call is refused. We also record whether the refusal happened, so that we can test that we are
* indeed refusing access where we expect.
*/ */
private static class NoSetAccessibleSecurityManager extends SecurityManager { private static class NoSetAccessibleSecurityManager extends SecurityManager {
private static final Permission DISALLOWED_PERMISSION = private static final Permission DISALLOWED_PERMISSION =
new ReflectPermission("suppressAccessChecks"); new ReflectPermission("suppressAccessChecks");
private static final ImmutableSet<String> ALLOWED_CALLERS = volatile boolean setAccessibleWasRefused;
ImmutableSet.of("NullPointerTester", "EqualsTester", "ObjectStreamClass");


@Override @Override
public void checkPermission(Permission p) { public void checkPermission(Permission p) {
if (p.equals(DISALLOWED_PERMISSION) && !allowedCaller()) { if (p.equals(DISALLOWED_PERMISSION)
super.checkPermission(p); && callStackContainsMethod("TypeVariableInvocationHandler", "<clinit>")) {
try {
super.checkPermission(p);
fail("Did not get expected AccessControlException");
} catch (AccessControlException expected) {
setAccessibleWasRefused = true;
throw expected;
}
} }
} }


private static boolean allowedCaller() { private static boolean callStackContainsMethod(String classNameFragment, String methodName) {
String stack = Throwables.getStackTraceAsString(new Throwable()); for (StackTraceElement stackTraceElement : new Throwable().getStackTrace()) {
for (String allowed : ALLOWED_CALLERS) { if (stackTraceElement.getClassName().contains(classNameFragment)
if (stack.contains(allowed)) { && stackTraceElement.getMethodName().equals(methodName)) {
return true; return true;
} }
} }
Expand All @@ -83,7 +96,7 @@ private static boolean allowedCaller() {
@Override @Override
protected void setUp() { protected void setUp() {
oldSecurityManager = System.getSecurityManager(); oldSecurityManager = System.getSecurityManager();
System.setSecurityManager(new NoSetAccessibleSecurityManager()); System.setSecurityManager(securityManager);
} }


@Override @Override
Expand Down Expand Up @@ -369,6 +382,11 @@ public void testNewTypeVariable() throws Exception {
.addEqualityGroup(objectBoundJvmType, objectBound) .addEqualityGroup(objectBoundJvmType, objectBound)
.addEqualityGroup(upperBoundJvmType, upperBound) .addEqualityGroup(upperBoundJvmType, upperBound)
.testEquals(); .testEquals();

// At some point before now we should have hit the permission check that prevents us from
// calling setAccessible. We check that that is indeed true because we want to make sure
// that creating a TypeVariable instance works even when that is not allowed.
assertTrue(securityManager.setAccessibleWasRefused);
} }


public void testNewTypeVariable_primitiveTypeBound() { public void testNewTypeVariable_primitiveTypeBound() {
Expand Down

0 comments on commit cc65b4f

Please sign in to comment.