river-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter_firmst...@apache.org
Subject svn commit: r1450119 - in /river/jtsk/skunk/qa_refactor/trunk/src: com/sun/jini/start/ org/apache/river/api/security/
Date Tue, 26 Feb 2013 10:42:30 GMT
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;
     }



Mime
View raw message