Author: peter_firmstone Date: Tue Feb 26 10:42:30 2013 New Revision: 1450119
URL: http://svn.apache.org/r1450119 Log: Modifications to ensure safe publication of Permission after parsing. Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/start/AggregatePolicyProvider.java river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrant.java river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PrincipalGrant.java Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/start/AggregatePolicyProvider.java URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/start/AggregatePolicyProvider.java?rev=1450119&r1=1450118&r2=1450119&view=diff ============================================================================== --- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/start/AggregatePolicyProvider.java (original) +++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/start/AggregatePolicyProvider.java Tue Feb 26 10:42:30 2013 @@ -100,6 +100,8 @@ public class AggregatePolicyProvider } }); + // As tempting as it might be, do not allow multiple reads to WeakHashMap, + // it is not multi read thread safe. If multi read is required, use RC instead. private final Map<ClassLoader,Policy> subPolicies = new WeakHashMap<ClassLoader,Policy>();// protected by lock // The cache is used to avoid repeat security checks, the subPolicies map // cannot be used to cache child ClassLoaders because their policy could @@ -414,7 +416,7 @@ public class AggregatePolicyProvider boolean trust = trustGetContextClassLoader(t); ClassLoader ccl = trust ? getContextClassLoader() : null; if ( ccl == null ) return mainPolicy; - readLock.lock(); + readLock.lock(); // read lock is better than getting write locked at lookupSubPolicy try { Policy policy = subPolicyChildClassLoaderCache.get(ccl); // just a cache. if ( policy != null ) return policy; Modified: river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrant.java URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrant.java?rev=1450119&r1=1450118&r2=1450119&view=diff ============================================================================== --- river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrant.java (original) +++ river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrant.java Tue Feb 26 10:42:30 2013 @@ -26,10 +26,13 @@ import java.security.Principal; import java.security.ProtectionDomain; import java.util.Collection; import java.security.Policy; +import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.Set; -import java.util.TreeSet; +import java.util.concurrent.ConcurrentSkipListSet; import net.jini.security.GrantPermission; +import org.apache.river.api.security.PermissionGrantBuilderImp.NullPermissionGrant; /** * PermissionGrant implementations are expected to be immutable, non blocking, @@ -79,16 +82,27 @@ public abstract class PermissionGrant { * * Child classes are prevented from modifying the contained immutable Permissions. */ + private static final PermissionGrant nullGrant = new NullPermissionGrant(); private static final Guard PD_GUARD = new RuntimePermission("getProtectionDomain"); private static final Guard CL_GUARD = new RuntimePermission("getClassLoader"); private final Set<Permission> perms; private final boolean privileged; private final PermissionGrant decorated; + private final int hash; - public PermissionGrant(){ + /** + * Public constructor to enable serialization support? No a serialization + * builder is required. + */ + PermissionGrant(){ perms = Collections.emptySet(); privileged = false; decorated = null; + int hashcode = 7; + hashcode = 97 * hashcode + (this.perms != null ? this.perms.hashCode() : 0); + hashcode = 97 * hashcode + (this.privileged ? 1 : 0); + hashcode = 97 * hashcode + (this.decorated != null ? this.decorated.hashCode() : 0); + this.hash = hashcode; } PermissionGrant( Permission[] perm ){ @@ -98,7 +112,7 @@ public abstract class PermissionGrant { privileged = false; }else{ // PermissionComparator is used to avoid broken hashCode and equals - Set<Permission> perms = new TreeSet<Permission>(new PermissionComparator()); + Set<Permission> perms = new ConcurrentSkipListSet<Permission>(new PermissionComparator()); boolean privileged = false; int l = perm.length; for (int i = 0; i < l; i++){ @@ -108,6 +122,11 @@ public abstract class PermissionGrant { this.perms = Collections.unmodifiableSet(perms); this.privileged = privileged; } + int hashcode = 7; + hashcode = 97 * hashcode + (this.perms != null ? this.perms.hashCode() : 0); + hashcode = 97 * hashcode + (this.privileged ? 1 : 0); + hashcode = 97 * hashcode + (this.decorated != null ? this.decorated.hashCode() : 0); + this.hash = hashcode; } protected PermissionGrant(PermissionGrant decorated){ @@ -115,12 +134,28 @@ public abstract class PermissionGrant { CL_GUARD.checkGuard(null); this.decorated = decorated; perms = Collections.emptySet(); - privileged = false; + privileged = decorated.isPrivileged(); + hash = decorated.hashCode(); + } + + @Override + public int hashCode() { + return hash; + } + + public boolean equals(Object o){ + if ( !( o instanceof PermissionGrant)) return false; + PermissionGrant that = (PermissionGrant) o; + if (this.privileged != that.privileged) return false; + if (decorated != null){ + return decorated.equals(that.decorated); + } + return perms.equals(that.perms); } protected final PermissionGrant decorated(){ // REMIND: Consider null object pattern. - return decorated; + return decorated != null ? decorated : nullGrant; } /** @@ -173,7 +208,7 @@ public abstract class PermissionGrant { public final Collection<Permission> getPermissions(){ if (decorated() != null) return decorated().getPermissions(); return perms; - } + } /** * Returns true if this PermissionGrant defines no Permissions, or if Modified: river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java?rev=1450119&r1=1450118&r2=1450119&view=diff ============================================================================== --- river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java (original) +++ river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java Tue Feb 26 10:42:30 2013 @@ -228,7 +228,7 @@ class PermissionGrantBuilderImp extends // This is a singleton so we don't need to implement equals or hashCode. - private static class NullPermissionGrant extends PermissionGrant implements Serializable { + static class NullPermissionGrant extends PermissionGrant implements Serializable { private static final long serialVersionUID = 1L; public boolean implies(ProtectionDomain pd) { Modified: river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PrincipalGrant.java URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PrincipalGrant.java?rev=1450119&r1=1450118&r2=1450119&view=diff ============================================================================== --- river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PrincipalGrant.java (original) +++ river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PrincipalGrant.java Tue Feb 26 10:42:30 2013 @@ -91,10 +91,10 @@ class PrincipalGrant extends PermissionG if (o == null) return false; if (o == this) return true; if (o.hashCode() != this.hashCode()) return false; + if (!super.equals(o)) return false; if (o instanceof PrincipalGrant ){ PrincipalGrant p = (PrincipalGrant) o; - if (pals.equals(p.pals) - && getPermissions().equals(p.getPermissions())) return true; + if (pals.equals(p.pals)) return true; } return false; }
