You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by pe...@apache.org on 2011/12/26 12:59:42 UTC
svn commit: r1224720 - in /river/jtsk/skunk/peterConcurrentPolicy:
qa/src/com/sun/jini/qa/harness/ src/com/sun/jini/start/
src/net/jini/security/ src/net/jini/security/policy/
src/org/apache/river/api/security/
Author: peter_firmstone
Date: Mon Dec 26 11:59:41 2011
New Revision: 1224720
URL: http://svn.apache.org/viewvc?rev=1224720&view=rev
Log:
River-323 Refactoring as part of a process to fix the following falling tests:
com/sun/jini/test/spec/lookupservice/test_set00/NotifyOnAttrAdd.td
com/sun/jini/test/spec/lookupservice/test_set00/NotifyOnAttrDel.td
com/sun/jini/test/spec/lookupservice/test_set00/NotifyOnAttrMod.td
com/sun/jini/test/spec/lookupservice/test_set02/NotifyOnAttrSet.td
com/sun/jini/test/impl/start/aggregatepolicyprovider/GetContextTest.td
Improvements to PrincipalGrant equals and hashCode implementations to avoid inadvertent dns lookup caused by SocketPermission.
Modified:
river/jtsk/skunk/peterConcurrentPolicy/qa/src/com/sun/jini/qa/harness/MergedPolicyProvider.java
river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java
river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/ConcurrentPermissions.java
river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/DynamicPermissionCollection.java
river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/GrantPermission.java
river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/PermissionComparator.java
river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java
river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/CertificateGrant.java
river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java
river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java
Modified: river/jtsk/skunk/peterConcurrentPolicy/qa/src/com/sun/jini/qa/harness/MergedPolicyProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/qa/src/com/sun/jini/qa/harness/MergedPolicyProvider.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/qa/src/com/sun/jini/qa/harness/MergedPolicyProvider.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/qa/src/com/sun/jini/qa/harness/MergedPolicyProvider.java Mon Dec 26 11:59:41 2011
@@ -24,10 +24,14 @@ import java.security.Policy;
import java.security.ProtectionDomain;
import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
import java.util.StringTokenizer;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import net.jini.security.policy.PolicyInitializationException;
import net.jini.security.policy.PolicyFileProvider;
@@ -42,10 +46,12 @@ import net.jini.security.policy.PolicyFi
*/
public class MergedPolicyProvider extends Policy {
+ /** class state */
+ private static final Lock lock = new ReentrantLock();; // protects first
+ private static boolean first = false; // Why is first static?
+
/** the collection of underlying policies */
- private ArrayList policies = new ArrayList();
-
- private static boolean first = false;
+ private final Collection<Policy> policies ;
/**
* Creates a new <code>MergedPolicyProvider</code> instance that wraps a
@@ -74,6 +80,7 @@ public class MergedPolicyProvider extend
}
// no-arg semantics for 'default policy' necessary for correct behavior
// of PolicyFileProvider.refresh
+ Collection<Policy> policies = new ArrayList<Policy>();
try {
if (p1 != null) {
policies.add(new PolicyFileProvider());
@@ -93,6 +100,7 @@ public class MergedPolicyProvider extend
throw new PolicyInitializationException(
"unable to construct base policy", e);
}
+ this.policies = Collections.unmodifiableCollection(policies);
}
/**
@@ -133,53 +141,58 @@ public class MergedPolicyProvider extend
*/
public PermissionCollection getPermissions(ProtectionDomain domain) {
Iterator it = policies.iterator();
- ArrayList list = new ArrayList();
- if (it.hasNext()) {
- PermissionCollection pc =
- ((Policy) it.next()).getPermissions(domain);
- if (first) {
- first = false;
- Enumeration en = pc.elements();
- list.add("BASE PERMISSIONS for domain " + domain);
- while (en.hasMoreElements()) {
- Permission perm = (Permission) en.nextElement();
- list.add(perm.toString());
- }
- first = true;
- }
- while (it.hasNext()) {
- PermissionCollection pc2 =
- ((Policy) it.next()).getPermissions(domain);
- Enumeration en = pc2.elements();
- while (en.hasMoreElements()) {
- Permission perm = (Permission) en.nextElement();
- if (!pc.implies(perm)) {
- if (first) {
- first = false;
- list.add("checking " + perm + " and adding");
- first = true;
- }
- pc.add(perm);
- } else {
- if (first) {
- first = false;
- list.add("checking " + perm + " and not adding");
- first = true;
- }
- }
- }
- }
- if (first) {
- first = false;
- for (int i = 0; i < list.size(); i++) {
- System.out.println((String) list.get(i));
- }
- first = true;
- }
- return pc;
- } else {
- throw new IllegalStateException("No policies in provider");
- }
+ ArrayList list = new ArrayList(64);
+ lock.lock();
+ try {
+ if (it.hasNext()) {
+ PermissionCollection pc =
+ ((Policy) it.next()).getPermissions(domain);
+ if (first) {
+ first = false;
+ Enumeration en = pc.elements();
+ list.add("BASE PERMISSIONS for domain " + domain);
+ while (en.hasMoreElements()) {
+ Permission perm = (Permission) en.nextElement();
+ list.add(perm.toString());
+ }
+ first = true;
+ }
+ while (it.hasNext()) {
+ PermissionCollection pc2 =
+ ((Policy) it.next()).getPermissions(domain);
+ Enumeration en = pc2.elements();
+ while (en.hasMoreElements()) {
+ Permission perm = (Permission) en.nextElement();
+ if (!pc.implies(perm)) {
+ if (first) {
+ first = false;
+ list.add("checking " + perm + " and adding");
+ first = true;
+ }
+ pc.add(perm);
+ } else {
+ if (first) {
+ first = false;
+ list.add("checking " + perm + " and not adding");
+ first = true;
+ }
+ }
+ }
+ }
+ if (first) {
+ first = false;
+ for (int i = 0; i < list.size(); i++) {
+ System.out.println((String) list.get(i));
+ }
+ first = true;
+ }
+ return pc;
+ } else {
+ throw new IllegalStateException("No policies in provider");
+ }
+ }finally{
+ lock.unlock();
+ }
}
/**
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java Mon Dec 26 11:59:41 2011
@@ -494,7 +494,7 @@ public class AggregatePolicyProvider
*/
private static class AggregateSecurityContext implements SecurityContext {
- private final ClassLoader ccl = getContextClassLoader();
+ private final ClassLoader ccl;
private final SecurityContext sc;
AggregateSecurityContext(SecurityContext sc) {
@@ -502,6 +502,7 @@ public class AggregatePolicyProvider
throw new NullPointerException();
}
this.sc = sc;
+ ccl = getContextClassLoader();
}
public PrivilegedAction wrap(PrivilegedAction a) {
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/ConcurrentPermissions.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/ConcurrentPermissions.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/ConcurrentPermissions.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/ConcurrentPermissions.java Mon Dec 26 11:59:41 2011
@@ -109,7 +109,7 @@ implements Serializable {
// this get saves unnecessary object creation.
Class clas = permission.getClass();
PermissionCollection pc = permsMap.get(clas);
- if (pc == null){
+ if (pc == null){
pc = new DynamicPermissionCollection(null, clas);
PermissionCollection existed =
permsMap.putIfAbsent(clas, pc);
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/DynamicPermissionCollection.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/DynamicPermissionCollection.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/DynamicPermissionCollection.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/DynamicPermissionCollection.java Mon Dec 26 11:59:41 2011
@@ -31,6 +31,8 @@ import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Enumeration;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import org.apache.river.impl.util.CollectionsConcurrent;
/**
@@ -47,17 +49,18 @@ final class DynamicPermissionCollection
private final Class cl;
private final Comparator<Permission> comp;
+ private final Lock lock;
+ private PermissionCollection cache; // protected by lock.
+ private volatile int cacheCount;
+ private volatile boolean noPC;
+
DynamicPermissionCollection(Comparator<Permission> c, Class cl){
perms = CollectionsConcurrent.multiReadCollection(new ArrayList<Permission>());
this.cl = cl;
comp = c;
- }
-
- private DynamicPermissionCollection(Class cl, Comparator<Permission> c){
- this.cl = cl;
- comp = c;
- Collection<Permission> col = new ArrayList<Permission>();
- perms = CollectionsConcurrent.multiReadCollection(col);
+ lock = new ReentrantLock();
+ cacheCount = 0;
+ noPC = false;
}
@Override
@@ -71,21 +74,46 @@ final class DynamicPermissionCollection
@Override
public boolean implies(Permission permission) {
if ( ! cl.isInstance(permission)) return false;
- Thread thread = Thread.currentThread();
- if (thread.isInterrupted()) return false;
+ if ( !noPC && lock.tryLock()){
+ try {
+ if (cache != null && cache.implies(permission)) return true;
+ } finally {
+ lock.unlock();
+ }
+ }
Permission [] p = perms.toArray(new Permission[0]); //perms.size() may change
if (comp != null){
Arrays.sort(p, comp);
}
- PermissionCollection pc = permission.newPermissionCollection();
- int l = p.length;
+ if (noPC) return rawImplies(permission, p);
+ int len = p.length;
+ PermissionCollection pc = null;
+ boolean result = false;
+ pc = permission.newPermissionCollection();
if (pc != null) {
- for ( int i = 0; i < l ; i++ ){
+ for ( int i = 0; i < len ; i++ ){
pc.add(p[i]);
}
- return pc.implies(permission);
+ result = pc.implies(permission);
+ if ( cacheCount < len){
+ lock.lock();
+ try {
+ cache = pc;
+ cacheCount = len;
+ } finally {
+ lock.unlock();
+ }
+ }
+ return result;
+ } else {
+ noPC = true;
+ return rawImplies(permission, p);
}
- for ( int i = 0; i < l ; i++ ){
+ }
+
+ private boolean rawImplies(Permission permission, Permission[] p){
+ int len = p.length;
+ for ( int i = 0; i < len ; i++ ){
if (p[i].implies(permission)) return true;
}
return false;
@@ -138,7 +166,7 @@ final class DynamicPermissionCollection
private Object readResolve() {
PermissionCollection pc =
- new DynamicPermissionCollection(clazz, comp);
+ new DynamicPermissionCollection(comp, clazz);
int length = permissions.length;
for ( int i = 0 ; i < length ; i++){
pc.add(permissions[i]);
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/GrantPermission.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/GrantPermission.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/GrantPermission.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/GrantPermission.java Mon Dec 26 11:59:41 2011
@@ -36,11 +36,13 @@ import java.security.Permissions;
import java.security.UnresolvedPermission;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.TreeSet;
import net.jini.security.policy.DynamicPolicy;
/**
@@ -763,7 +765,8 @@ public final class GrantPermission exten
new ObjectStreamField("perms", List.class, true)
};
- private List perms = new ArrayList(40);
+ private Collection<Permission> perms =
+ new TreeSet<Permission>(new PermissionComparator());
private Implier implier = new Implier();
public synchronized void add(Permission p) {
@@ -774,13 +777,13 @@ public final class GrantPermission exten
throw new SecurityException(
"can't add to read-only PermissionCollection");
}
-// if (!perms.contains(p)){
+ if (!perms.contains(p)){
perms.add(p);
implier.add((GrantPermission) p);
-// }
+ }
}
- public synchronized Enumeration elements() {
+ public synchronized Enumeration<Permission> elements() {
return Collections.enumeration(perms);
}
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/PermissionComparator.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/PermissionComparator.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/PermissionComparator.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/PermissionComparator.java Mon Dec 26 11:59:41 2011
@@ -12,6 +12,7 @@ import java.util.Comparator;
public class PermissionComparator implements Comparator<Permission> {
public int compare(Permission o1, Permission o2) {
+ if (o1 == o2) return 0;
Class c1 = o1.getClass();
String name1 = o1.getName();
String actions1 = o1.getActions();
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java Mon Dec 26 11:59:41 2011
@@ -42,6 +42,7 @@ import java.security.PrivilegedActionExc
import java.security.PrivilegedExceptionAction;
import java.security.ProtectionDomain;
import java.security.SecurityPermission;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Enumeration;
@@ -189,7 +190,7 @@ public class ConcurrentPolicyFile extend
private final Lock wl;
- private final Set<PermissionGrant> grants ; // protected by rwl
+ private final Collection<PermissionGrant> grants ; // protected by rwl
// Calculated Permissions cache, organized as
// Map{Object->Collection<Permission>}.
@@ -223,7 +224,7 @@ public class ConcurrentPolicyFile extend
ReadWriteLock rwl = new ReentrantReadWriteLock();
rl = rwl.readLock();
wl = rwl.writeLock();
- grants = new HashSet<PermissionGrant>(120);
+ grants = new ArrayList<PermissionGrant>(120);
ConcurrentMap<Referrer<Object>, Referrer<Collection<Permission>>> cacheInternal
= new ConcurrentHashMap<Referrer<Object>, Referrer<Collection<Permission>>>(120);
cache = RC.concurrentMap(cacheInternal, Ref.WEAK_IDENTITY, Ref.SOFT);
@@ -356,17 +357,22 @@ public class ConcurrentPolicyFile extend
@Override
public boolean implies(ProtectionDomain domain, Permission permission) {
if (permission == null) throw new NullPointerException("permission not allowed to be null");
- PermissionCollection pc =
- domain != null ? impliesCache.get(domain): null;
- if (pc == null){
- pc = getPermissions(domain);
- if (domain != null){
- PermissionCollection existed = impliesCache.putIfAbsent(domain, pc);
- // Don't bother replacing pc with existed, we're not mutating
- // so it doesn't matter.
+ rl.lock();
+ try {
+ PermissionCollection pc =
+ domain != null ? impliesCache.get(domain): null;
+ if (pc == null){
+ pc = getPermissions(domain);
+ if (domain != null){
+ PermissionCollection existed = impliesCache.putIfAbsent(domain, pc);
+ // Don't bother replacing pc with existed, we're not mutating
+ // so it doesn't matter.
+ }
}
+ return pc.implies(permission);
+ }finally{
+ rl.unlock();
}
- return pc.implies(permission);
}
/**
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/CertificateGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/CertificateGrant.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/CertificateGrant.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/CertificateGrant.java Mon Dec 26 11:59:41 2011
@@ -77,13 +77,13 @@ class CertificateGrant extends Principal
@Override
public String toString(){
StringBuilder sb = new StringBuilder(400);
- sb.append(super.toString())
- .append("Certificate's: \n");
+ sb.append("\nCertificate's: \n");
Iterator<Certificate> it = certs.iterator();
while (it.hasNext()){
sb.append(it.next().toString())
.append("\n");
}
+ sb.append(super.toString());
return sb.toString();
}
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java Mon Dec 26 11:59:41 2011
@@ -29,6 +29,7 @@ import java.security.Principal;
import java.security.ProtectionDomain;
import java.security.acl.Group;
import java.security.cert.Certificate;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -36,6 +37,8 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
+import java.util.TreeSet;
+import net.jini.security.PermissionComparator;
import org.apache.river.impl.security.policy.util.PolicyUtils;
/**
@@ -57,18 +60,30 @@ class PrincipalGrant implements Permissi
palCol.addAll(Arrays.asList(pals));
this.pals = Collections.unmodifiableSet(palCol);
}else {
- this.pals = Collections.EMPTY_SET;
+ this.pals = Collections.emptySet();
}
if (perm == null || perm.length == 0) {
- this.perms = Collections.EMPTY_SET;
+ this.perms = Collections.emptySet();
}else{
- Set<Permission> perms = new HashSet<Permission>(perm.length);
+ // PermissionComparator is used to avoid broken hashCode and equals
+ Set<Permission> perms = new TreeSet<Permission>(new PermissionComparator());
perms.addAll(Arrays.asList(perm));
this.perms = Collections.unmodifiableSet(perms);
}
int hash = 5;
hash = 97 * hash + (this.pals != null ? this.pals.hashCode() : 0);
- hash = 97 * hash + (this.perms != null ? this.perms.hashCode() : 0);
+ Iterator<Permission> i = perms.iterator();
+ while (i.hasNext()){
+ Permission p = i.next();
+ if (p != null){
+ Class c = p.getClass();
+ String name = p.getName();
+ String actions = p.getActions();
+ hash = 97 * hash + (c != null ? c.hashCode() : 0);
+ hash = 97 * hash + (name != null ? name.hashCode() : 0);
+ hash = 97 * hash + (actions != null ? actions.hashCode() : 0);
+ }
+ }
hashCode = hash;
this.inverse = inverse;
}
@@ -81,7 +96,9 @@ class PrincipalGrant implements Permissi
if (o instanceof PrincipalGrant ){
PrincipalGrant p = (PrincipalGrant) o;
if (pals.equals(p.pals)
- && perms.equals(p.perms)) return true;
+ // To avoid broken equals:
+ && perms.containsAll(p.perms)
+ && p.perms.containsAll(perms)) return true;
}
return false;
}
@@ -94,19 +111,18 @@ class PrincipalGrant implements Permissi
@Override
public String toString(){
StringBuilder sb = new StringBuilder(300);
- sb.append(this.getClass().getCanonicalName())
- .append("Permissions: \n");
- Iterator<Permission> permIt = perms.iterator();
- while (permIt.hasNext()){
- sb.append(permIt.next().toString())
- .append("\n");
- }
sb.append("Principals: \n");
Iterator<Principal> palIt = pals.iterator();
while (palIt.hasNext()){
sb.append(palIt.next().toString())
.append("\n");
}
+ sb.append("\nPermissions: \n");
+ Iterator<Permission> permIt = perms.iterator();
+ while (permIt.hasNext()){
+ sb.append(permIt.next().toString())
+ .append("\n");
+ }
return sb.toString();
}
Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java?rev=1224720&r1=1224719&r2=1224720&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java Mon Dec 26 11:59:41 2011
@@ -80,9 +80,11 @@ class URIGrant extends CertificateGrant
@Override
public String toString(){
StringBuilder sb = new StringBuilder(500);
- return sb.append(super.toString())
- .append("URI: \n")
+ return sb.append("\n")
+ .append("URI: ")
.append(location.toString())
+ .append(super.toString())
+ .append("\n")
.toString();
}