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 2013/02/26 11:42:30 UTC
svn commit: r1450119 - in /river/jtsk/skunk/qa_refactor/trunk/src:
com/sun/jini/start/ org/apache/river/api/security/
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;
}