On 06.11.2011, at 19:07, ceki wrote: > On 11/6/2011 1:32 AM, Joern Huxhorn wrote: > >> There is one issue with the code in http://goo.gl/zASBm, though: > > > It checks for getClassLoader permission in a static code block only, > > i.e. when the class is initially loaded by the classloader, and saves > > that information for later reference. > > The reference is never used anywhere getClassLoaderAsPrivileged method > is never invoked by logback. The privileged block is result of a > compromise: http://jira.qos.ch/browse/LBCLASSIC-263
Yes, I checked out that issue. > > > I don't think that this is a valid optimization for precisely the > > reason that this permission can change during runtime (according to > > http://download.oracle.com/javase/1.4.2/docs/api/java/security/AccessController.html > > even per thread), for example if a different SecurityManager is > > installed. > > One rarely installs a different SecurityManager but in general you are > right. > >> >> The method using the statically initialized boolean should probably look >> like this instead: >> >> public static ClassLoader getClassLoaderAsPrivileged(final Class clazz) { >> try { >> AccessController.checkPermission(new >> RuntimePermission("getClassLoader")); >> return AccessController.doPrivileged( >> new PrivilegedAction<ClassLoader>() { >> public ClassLoader run() { >> return clazz.getClassLoader(); >> } >> }); >> } catch (AccessControlException e) { >> return null; >> } >> } >> >> Not sure if this would solve the problem at hand... > > It'll probably make the SecurityException in LBCLASSIC-303 go away by > virtue of call reordering. However, it does not explain why invoking > AccessController.doPrivileged changes the behavior of > RMISecurityManger. I don't get it, either. I just think that the current code in Logback is wrong anyway and should therefore be fixed. First of all, it doesn't make sense to execute AccessController.checkPermission(new RuntimePermission("getClassLoader")) wrapped into a PrivilegedAction since it is always allowed to check for permission. Wouldn't make sense otherwise, too. Secondly, the current implementation does not take into account that different threads may have different permissions. This could by a real problem in certain environments where, e.g., a worker-thread does not have the required permissions while the main app has it. My code in https://github.com/ceki/logback/tree/LBCLASSIC-304 should handle those scenarios gracefully while not degrading performance if no SecurityManager is in place. If this makes the problem go away I'm 90% fine with it. The remaining 10% of me would really like to understand the reason for all of this. :p One thing that could be somehow responsible for the test failure is the rather complex way "java.security.policy" is evaluated. See http://download.oracle.com/javase/1.4.2/docs/guide/security/PolicyFiles.html and search for it. It assumes an URL and not a file. Further, it has to be specified with and additional = to replace existing rules instead of merging them. So the line in the test should probably (!!) look like this instead: System.setProperty("java.security.policy","=./src/test/resources/java.policy"); or, even more likely, URL policyUrl = SecurityManagerTest.class.getResource("/java.policy"); String policyUrlString = policyUrl.toString(); System.setProperty("java.security.policy","="+policyUrlString); Joern. _______________________________________________ Logback-user mailing list [email protected] http://mailman.qos.ch/mailman/listinfo/logback-user
