FYI: TestSecurityManager enhancement

FYI: TestSecurityManager enhancement

[Gary Benson]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Hi all,

This commit makes the test security manager compare permissions using
p1.equals(p2) instead of p1.implies(p2).  This ensures that we are
testing that the permission we are checking is exactly the permission
we expected.  This is necessary for most of the networking tests.
InetAddresss.getCanonicalHostName(), for example is supposed to check
on the canonical hostname -- not the address -- yet testing using
implies would happily allow either.

Cheers,
Gary

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4634 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/mauve/mauve/ChangeLog,v
retrieving revision 1.1924
diff -u -r1.1924 ChangeLog
--- ChangeLog	21 Sep 2006 23:29:34 -0000	1.1924
+++ ChangeLog	22 Sep 2006 10:35:10 -0000
@@ -1,3 +1,11 @@
+2006-09-22  Gary Benson  <gbenson@redhat.com>
+
+	* gnu/testlet/TestSecurityManager.java:
+	Compare permissions using equals() rather than implies().
+	Provide a way for tests to specify that they need implies().
+	* gnu/testlet/java/io/File/security.java:
+	Use implies() comparisons for temp file tests.
+
 2006-09-21  Casey Marshall  <csm@gnu.org>
 
 	* gnu/testlet/java/nio/channels/Selector/testEmptySelect.java:
Index: gnu/testlet/TestSecurityManager.java
===================================================================
RCS file: /cvs/mauve/mauve/gnu/testlet/TestSecurityManager.java,v
retrieving revision 1.3
diff -u -r1.3 TestSecurityManager.java
--- gnu/testlet/TestSecurityManager.java	11 Jul 2006 09:55:00 -0000	1.3
+++ gnu/testlet/TestSecurityManager.java	22 Sep 2006 10:35:10 -0000
@@ -93,6 +93,21 @@
   private final SuccessException successException = new SuccessException();
 
   /**
+   * How should permissions be compared?
+   */
+  private int compare;
+
+  /**
+   * Compare permissions using <code>p1.equals(p2)</code>.
+   */
+  public static final int EQUALS = 1;
+  
+  /**
+   * Compare permissions using <code>p1.implies(p2)</code>.
+   */
+  public static final int IMPLIES = 2;
+   
+ /**
    * An empty list of checks, for convenience.
    */
   private final Permission[] noChecks = new Permission[0];
@@ -213,9 +228,42 @@
 
     checked = new boolean[mustCheck.length];
     enabled = true;
+    compare = EQUALS;
+  }
+
+  /**
+   * Under normal circumstances permissions are compared using
+   * <code>p1.equals(p2)</code> to ensure that the permission being
+   * checked is exactly the permission that is expected.  Sometimes it
+   * is not possible to know in advance the exact permission that will
+   * be checked -- the best you can do is some kind of wildcard -- and
+   * in such cases tests can specify that permissions should be
+   * compared using <code>p1.implies(p2)</code> using this method.
+   *
+   * @param style the desired comparison style (<code>EQUALS</code> or
+   *              <code>IMPLIES</code>).
+   */
+  public void setComparisonStyle(int style)
+  {
+    compare = style;
   }
 
   /**
+   * Compare two permissions.
+   */
+  private boolean permissionsMatch(Permission p1, Permission p2)
+  {
+    switch (compare) {
+    case EQUALS:
+      return p1.equals(p2);
+    case IMPLIES:
+      return p1.implies(p2);
+    default:
+      throw new IllegalArgumentException();
+    }      
+  }
+  
+  /**
    * Check that this permission is one that we should be checking.
    * 
    * @param perm the permission to be checked
@@ -223,7 +271,7 @@
    *         permissions have been checked and <code>isHalting</code>
    *         is true.
    * @throws SecurityException if none of the <code>mustCheck</code>
-   *         or <code>mayCheck</code> permissions implies
+   *         or <code>mayCheck</code> permissions matches
    *         <code>perm</code>.
    */
   public void checkPermission(Permission perm) throws SecurityException
@@ -236,13 +284,13 @@
 
     boolean matched = false;
     for (int i = 0; i < mustCheck.length; i++) {
-      if (mustCheck[i].implies(perm))
+      if (permissionsMatch(mustCheck[i], perm))
 	matched = checked[i] = true;
     }
 
     if (!matched) {
       for (int i = 0; i < mayCheck.length; i++) {
-	if (mayCheck[i].implies(perm))
+	if (permissionsMatch(mayCheck[i], perm))
 	  matched = true;
       }
     }
Index: gnu/testlet/java/io/File/security.java
===================================================================
RCS file: /cvs/mauve/mauve/gnu/testlet/java/io/File/security.java,v
retrieving revision 1.14
diff -u -r1.14 security.java
--- gnu/testlet/java/io/File/security.java	18 Jul 2006 11:49:18 -0000	1.14
+++ gnu/testlet/java/io/File/security.java	22 Sep 2006 10:35:10 -0000
@@ -210,6 +210,7 @@
       try {
 	sm.prepareChecks(new Permission[]{tmpallWritePerm},
 			 new Permission[]{tmpdirPropPerm});
+	sm.setComparisonStyle(TestSecurityManager.IMPLIES);
 	tf1 = File.createTempFile("pfx", "sfx");
 	sm.checkAllChecked();
       }
@@ -222,6 +223,7 @@
       harness.checkPoint("createTempFile(3-args)");
       try {
 	sm.prepareChecks(new Permission[]{tmpdirallWritePerm});
+	sm.setComparisonStyle(TestSecurityManager.IMPLIES);
 	tf2 = File.createTempFile("pfx", "sfx", tmpdir);
 	sm.checkAllChecked();
       }

FYI: TestSecurityManager enhancement

[Gary Benson]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

Hi again,

This commit makes the test security manager inhibit checks except
within testcases.  It relies on the fact that all testcases must begin
with a call to prepareChecks() and end either when checkAllChecked()
is called or when checkPermission() throws an exception.  Because it
is known when testcases start and end, we can inhibit checking outside
of testcases.  This stops the security manager stomping on GCJ's line
number process, as well allowing a bunch of hacks to be removed.

Cheers,
Gary

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3390 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/mauve/mauve/ChangeLog,v
retrieving revision 1.1783
diff -u -r1.1783 ChangeLog
--- ChangeLog	11 Jul 2006 09:40:52 -0000	1.1783
+++ ChangeLog	11 Jul 2006 09:54:21 -0000
@@ -1,3 +1,13 @@
+2006-07-11  Gary Benson  <gbenson@redhat.com>
+
+	* gnu/testlet/TestSecurityManager.java: Add new "enabled" flag.
+	(install): Disable self before setting security manager.
+	(uninstall): Likewise.
+	(prepareChecks): Enable self after preparing checks.
+	(checkPermission): Don't check unless enabled, and disable self
+	before throwing exceptions.
+	(checkAllChecked): Disable self before checking.
+
 2006-07-11  Gary Benson  <gbenson@redhat.com>
 
 	* configure.in: Reinstated SRCDIR.
Index: gnu/testlet/TestSecurityManager.java
===================================================================
RCS file: /cvs/mauve/mauve/gnu/testlet/TestSecurityManager.java,v
retrieving revision 1.2
diff -u -r1.2 TestSecurityManager.java
--- gnu/testlet/TestSecurityManager.java	27 Jun 2006 12:23:38 -0000	1.2
+++ gnu/testlet/TestSecurityManager.java	11 Jul 2006 09:54:21 -0000
@@ -23,8 +23,6 @@
 package gnu.testlet;
 
 import java.security.Permission;
-import java.security.SecurityPermission;
-import java.util.PropertyPermission;
 
 /**
  * A security manager for testing that security checks are performed.
@@ -66,6 +64,11 @@
   private Permission[] mayCheck;
 
   /**
+   * Whether we are enabled or not.
+   */
+  private boolean enabled;
+  
+  /**
    * Must-check permissions are flagged as they are checked.
    */
   private boolean[] checked;
@@ -115,12 +118,7 @@
       throw new IllegalStateException("already installed");
     oldManager = oldsm;
 
-    // On some JVMs, setting the security manager for the first time
-    // triggers some initialization that reads system properties.
-    prepareChecks(noChecks, new Permission[] {
-	new SecurityPermission("getProperty.*"),
-	new PropertyPermission("*", "read")});
-
+    enabled = false;
     System.setSecurityManager(this);
   }
 
@@ -134,9 +132,7 @@
     if (oldsm != this)
       throw new IllegalStateException("not installed");
 
-    prepareChecks(noChecks, new Permission[] {
-	new RuntimePermission("setSecurityManager")});
-
+    enabled = false;
     System.setSecurityManager(oldManager);
   }
 
@@ -213,8 +209,10 @@
   {
     this.mayCheck = mayCheck;
     this.mustCheck = mustCheck;
-    this.checked = new boolean[mustCheck.length];
     this.isHalting = isHalting;
+
+    checked = new boolean[mustCheck.length];
+    enabled = true;
   }
 
   /**
@@ -230,6 +228,9 @@
    */
   public void checkPermission(Permission perm) throws SecurityException
   {
+    if (!enabled)
+      return;
+
     if (harness != null)
       harness.debug("checkPermission(" + perm + ")");
 
@@ -247,6 +248,8 @@
     }
 
     if (!matched) {
+      enabled = false;
+      
       harness.debug("unexpected check: " + perm);
 
       if (mustCheck.length != 0) {
@@ -272,8 +275,10 @@
 	if (!checked[i])
 	  allChecked = false;
       }
-      if (allChecked)
+      if (allChecked) {
+	enabled = false;
 	throw successException;
+      }
     }
   }
 
@@ -283,6 +288,8 @@
    */
   public void checkAllChecked()
   {
+    enabled = false;
+
     boolean allChecked = true;
     for (int i = 0; i < checked.length; i++) {
       if (!checked[i]) {