> On Mar 24, 2016, at 7:52 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > Hi Mandy, > > Here is the new webrev - I have added comments for the > string constants as well as for the permissions passed > through to the limited doPrivileged call: > > http://cr.openjdk.java.net/~dfuchs/webrev_8150840/webrev.02/index.html > > I hope that it makes it more clear that the > java.util.logging.* properties are only used when java.logging > is present, and the jdk.system.logger.* properties when > java.logging is not present. >
It looks okay. “jdk.logger.packages” is to specify which packages to be filtered and maybe better to rename it to “jdk.logger.filtered.packages”? About usage of limited doPrivileged, it’d be useful to have some guideline of using limited doPrivileged - I brought this up to Jeff when it was introduced and will ping him again. Like this example (in SimpleConsoleLogger.java) 476 String format = AccessController.doPrivileged( 477 new GetPropertyAction(key), null, 482 new PropertyPermission(DEFAULT_FORMAT_PROP_KEY, "read"), 487 new PropertyPermission(JUL_FORMAT_PROP_KEY, "read")); The doPrivileged block simply reads a system property and one single security-sensitive operation. It’s nothing wrong using limited doPrivileged in this case but I think it’s unnecessary. Limited doPrivileged would benefit to limiting a block of code that is hard to tell what privileges are elevated. getSimpleFormat should probably check the given key argument is either DEFAULT_FORMAT_PROP_KEY or JUL_FORMAT_PROP_KEY; otherwise throws IAE. Otherwise, this method would pass if key is unexpected key if security manager is absent but fails if security manager is present. Mandy
