I'm a little hesitant, given recent events, however I think I should ask
the question.
This patch is applied to my local copy of River, either I can revert my
local copy in preparation for building River 3.0 release artifacts, or I
should commit it?
What does this patch address?
PermissionComparator is used in a number of places, in
CombinerSecurityManager and policy provider implementations. It avoids
equals and hashcode methods and orders Permissions in Sets and avoids
duplicates to improve performance during permission checks. It helps to
avoid a number of DNS calls that SocketPermission might cause.
PrivateCredentialPermission, is a relatively complex permission.
Most permission implementations have a two argument constructor; name
and argument strings. PrivateCredentialPermission has this constructor,
when used, the name argument passed to the constructor is returned by
the getName() method.
However during permission checks, the getName() method only returns the
credential class. Crucial information is missing, namely principal
classes and principals, it results in incorrect results for
PrivateCredentialPermission when using PermissionComparator. When using
PermissionComparator in a Set, a number of PrivateCredentialPermissions
may be missing, as they are deemed equal by the PermissionComparator
when they are not. The only way to address this issue, is to access the
information contained in PrivateCredentialPermission using other methods.
Hence the patch attached.
I discovered this discrepancy while dynamically generating policy files
and testing those files for use a policy files.
Shall I submit this to trunk now, before generating River 3.0 release
artifacts this coming weekend? Or should I report the issue to JIRA
after release?
Regards,
Peter.
diff --git a/src/org/apache/river/api/security/PermissionComparator.java
b/src/org/apache/river/api/security/PermissionComparator.java
index 9a4eb91..a630bb7 100644
--- a/src/org/apache/river/api/security/PermissionComparator.java
+++ b/src/org/apache/river/api/security/PermissionComparator.java
@@ -23,6 +23,7 @@
import java.security.cert.Certificate;
import java.util.Arrays;
import java.util.Comparator;
+import javax.security.auth.PrivateCredentialPermission;
/**
* A Comparator for Permission that avoids using equals and hashCode() on
@@ -68,7 +69,8 @@
if (hash1 < hash2) return -1;
if (hash1 > hash2) return 1;
//hashcodes equal.
- if (o1 instanceof UnresolvedPermission && o2 instanceof
UnresolvedPermission){
+ if (o1 instanceof UnresolvedPermission
+ && o2 instanceof UnresolvedPermission){
// Special case
UnresolvedPermission u1 = (UnresolvedPermission) o1, u2 =
(UnresolvedPermission) o2;
String type1 = u1.getUnresolvedType(), type2 =
u2.getUnresolvedType();
@@ -116,7 +118,47 @@
if (c != 0) return c;
}
return -1;
- }
+ } else if (o1 instanceof PrivateCredentialPermission &&
+ o2 instanceof PrivateCredentialPermission){
+ // PrivateCredentialPermission.getName() may only include the
credential
+ // while the action always equals read.
+ PrivateCredentialPermission p1 = (PrivateCredentialPermission) o1;
+ PrivateCredentialPermission p2 = (PrivateCredentialPermission) o2;
+ String cred1 = p1.getCredentialClass(), cred2 =
p2.getCredentialClass();
+ if (cred1 == null){
+ if (cred2 ==null) return 0;
+ return -1; //o1 is less
+ }
+ if (cred2 == null) return 1; //o1 is greater
+ comparison = cred1.compareTo(cred2);
+ if ( comparison != 0 ) return comparison;
+ // credentials equal
+ String [][] prin1 = p1.getPrincipals();
+ String [][] prin2 = p2.getPrincipals();
+ int len1 = prin1.length, len2 = prin2.length;
+ if (len1 < len2) return -1;
+ if (len1 > len2) return 1;
+ // lengths equal
+ // Now we could get complex and order the Principal class names
+ // and principal names, for more accurate equals, however the only
+ // consequence of not doing so, is that equivalent permissions
+ // may be contained, in the same collection.
+ for (int i=0; i<len1; i++){
+ String [] pr1 = prin1[i];
+ String [] pr2 = prin2[i];
+ // length should always be 2, but just in case.
+ int l1 = pr1.length;
+ int l2 = pr2.length;
+ if (l1 < l2) return -1;
+ if (l1 > l2) return 2;
+ // length equal
+ for (int j=0; j<l1; j++){
+ comparison = pr1[j].compareTo(pr2[j]);
+ if (comparison != 0) return comparison;
+ }
+ }
+ return -1;
+ }
String name1 = o1.getName();
String name2 = o2.getName();
if ( name1 == null ){
