Logger initialization regression fix and a little story on security contexts during class initialization

[Mark Wielaard <mark at klomp dot org>]

Hi,

We had an interesting regression with the logging code in GNU Classpath.
It wasn't immediately apparent because it was only caused with certain
compilers. The class initialization order was subtly different between
byte code compiled with gcj -C and jikes. So it didn't show up with gcj
-C which I used when committing this code.

The mauve test pointed out that an unexpected SecurityManager check was
made in some cases. Those cases were when the Logger class was
initialized for the first time. This is an interesting case since
depending on how/where a class is first initialized its static
constructor will be run in different security context. So the fix is to
initialize the global Logger in a PrivilegedAction to make sure that
even if the first class initialization is done through a untrusted
context the static fields can be initialized. What makes this really
awkward is that if a class is initialized from a untrusted context for
the first time and throws a SecurityException from its static
constructor that class can then NEVER be initialized again, even if it
is loaded through a privileged context.

2006-05-11  Mark Wielaard  <mark@klomp.org>

    * java/util/logging/Logger.java (global): Initialize inside static
    PrivilegedAction.

Unfortunately this patch only solves the real world scenario. It doesn't
solve the mauve failure since the security manager used in the test
doesn't actually use the security context to see whether the calls are
made with the right privileges. It is actually hard to figure out how to
test for this situation correctly. Because depends on when exactly the
class is first initialized. So for now I just make sure that the logging
framework is completely initialized before installing the test security
manager.

2006-05-11  Mark Wielaard  <mark@klomp.org>

    * gnu/testlet/java/util/logging/Handler/TestSecurityManager.java
    (install): Initialize LogManager first.
    * gnu/testlet/java/util/logging/Logger/TestSecurityManager.java
    (install): Likewise.

What we really need is a test that is run with minimal privileges and
install a real security manager that respect the whole security context
(through AccessController) which then tries to load every class to see
whether or not they can at least be initialized properly from such a
minimal security context. The problem with that is that while setting up
such a test part of the core classes are already initialized of course.
So it is hard to get anything that really tests it all.

Both patches have been committed to classpath and mauve. The classpath
patch will also go onto the release branch.

Cheers,

Mark

Index: java/util/logging/Logger.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/logging/Logger.java,v
retrieving revision 1.13
diff -u -r1.13 Logger.java
--- java/util/logging/Logger.java	3 Apr 2006 08:59:53 -0000	1.13
+++ java/util/logging/Logger.java	11 May 2006 12:10:58 -0000
@@ -41,6 +41,8 @@
 import java.util.List;
 import java.util.MissingResourceException;
 import java.util.ResourceBundle;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 
 /**
  * A Logger is used for logging information about events. Usually, there
@@ -76,7 +78,20 @@
    * products are supposed to create and use their own Loggers, so
    * they can be controlled individually.
    */
-  public static final Logger global = getLogger("global");
+  public static final Logger global;
+
+  static
+    {
+      // Our class might be initialized from an unprivileged context
+      global = (Logger) AccessController.doPrivileged
+	(new PrivilegedAction()
+	  {
+	    public Object run()
+	    {
+	      return getLogger("global");
+	    }
+	  });
+    }
 
 
   /**

Index: gnu/testlet/java/util/logging/Handler/TestSecurityManager.java
===================================================================
RCS file: /cvs/mauve/mauve/gnu/testlet/java/util/logging/Handler/TestSecurityManager.java,v
retrieving revision 1.1
diff -u -r1.1 TestSecurityManager.java
--- gnu/testlet/java/util/logging/Handler/TestSecurityManager.java	10 Mar 2004 23:20:42 -0000	1.1
+++ gnu/testlet/java/util/logging/Handler/TestSecurityManager.java	11 May 2006 12:25:48 -0000
@@ -24,7 +24,7 @@
 import java.security.Permission;
 import java.security.AccessControlException;
 import java.util.logging.LoggingPermission;
-
+import java.util.logging.LogManager;
 
 /**
  * A SecurityManager that can be told whether or not to grant
@@ -57,6 +57,9 @@
 
   public void install()
   {
+    // Make sure the LogManager is fully installed first.
+    LogManager lm = LogManager.getLogManager();
+
     SecurityManager oldsm = System.getSecurityManager();
     
     if (oldsm == this)
Index: gnu/testlet/java/util/logging/Logger/TestSecurityManager.java
===================================================================
RCS file: /cvs/mauve/mauve/gnu/testlet/java/util/logging/Logger/TestSecurityManager.java,v
retrieving revision 1.2
diff -u -r1.2 TestSecurityManager.java
--- gnu/testlet/java/util/logging/Logger/TestSecurityManager.java	10 May 2004 14:50:51 -0000	1.2
+++ gnu/testlet/java/util/logging/Logger/TestSecurityManager.java	11 May 2006 12:25:48 -0000
@@ -24,7 +24,7 @@
 import java.security.Permission;
 import java.security.AccessControlException;
 import java.util.logging.LoggingPermission;
-
+import java.util.logging.LogManager;
 
 /**
  * A SecurityManager that can be told whether or not to grant
@@ -57,6 +57,9 @@
 
   public void install()
   {
+    // Make sure the LogManager is fully installed first.
+    LogManager lm = LogManager.getLogManager();
+
     SecurityManager oldsm = System.getSecurityManager();
     
     if (oldsm == this)