That's really strange. I've just re-checked with j9 on SUSe9 and Win2003 Server - 100% passed with my changes. Are you sure this commit was really guilty?? Stepan did the rollback actually - seems he also investigated the build alert. Guys, could you please provide some more details on this?
-- Alexey 2006/12/12, Alexey Petrenko <[EMAIL PROTECTED]>:
Alexey, looks like your commit is the cause of unit test failure in nio module. I'll revert it. SY, Alexey 2006/12/12, [EMAIL PROTECTED] <[EMAIL PROTECTED]>: > Author: varlax > Date: Mon Dec 11 23:43:28 2006 > New Revision: 486052 > > URL: http://svn.apache.org/viewvc?view=rev&rev=486052 > Log: > Fixed HARMONY-2301 [luni] flawed SecurityManager.checkPackageAccess() impl > Regression tests added. > Tested on DRLVM > > Modified: > harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/SecurityManager.java > harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java > harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/MutableSecurityManager.java > harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/SecurityManagerTest.java > > Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/SecurityManager.java > URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/SecurityManager.java?view=diff&rev=486052&r1=486051&r2=486052 > ============================================================================== > --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/SecurityManager.java (original) > +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/SecurityManager.java Mon Dec 11 23:43:28 2006 > @@ -46,7 +46,8 @@ > private static final PropertyPermission READ_WRITE_ALL_PROPERTIES_PERMISSION = new PropertyPermission( > "*", "read,write"); > > - private static String[] securePackageList; > + private static final String PKG_ACC_KEY = "package.access"; > + private static final String PKG_DEF_KEY = "package.definition"; > > /** > * Flag to indicate whether a security check is in progress. > @@ -299,26 +300,8 @@ > if (packageName == null) { > throw new NullPointerException(); > } > - synchronized (SecurityManager.class) { > - if (securePackageList == null) { > - String securePackages = getSecurityProperty("package.access"); > - if (securePackages != null) { > - StringTokenizer tokenizer = new StringTokenizer(securePackages, ", "); > - int i = 0; > - securePackageList = new String[tokenizer.countTokens()]; > - while (tokenizer.hasMoreTokens()) { > - securePackageList[i++] = tokenizer.nextToken(); > - } > - } else { > - securePackageList = new String[0]; > - } > - } > - for (int i = 0; i < securePackageList.length; i++) { > - if (packageName.startsWith(securePackageList[i])) { > - checkPermission(new RuntimePermission("accessClassInPackage." + packageName)); > - return; > - } > - } > + if (checkPackageProperty(PKG_ACC_KEY, packageName)) { > + checkPermission(new RuntimePermission("accessClassInPackage." + packageName)); > } > } > > @@ -330,26 +313,44 @@ > * the name of the package to add a class to. > */ > public void checkPackageDefinition(String packageName) { > - if (packageName == null) { > - throw new NullPointerException(); > + if (packageName == null) { > + throw new NullPointerException(); > + } > + if (checkPackageProperty(PKG_DEF_KEY, packageName)) { > + checkPermission(new RuntimePermission("defineClassInPackage." + packageName)); > + } > + } > + > + /** > + * Returns true if the package name is restricted by > + * the specified security property. > + */ > + private static boolean checkPackageProperty( > + final String property, final String pkg) > + { > + String list = AccessController.doPrivileged(PriviAction > + .getSecurityProperty(property)); > + if (list != null) { > + int plen = pkg.length(); > + StringTokenizer tokenizer = new StringTokenizer(list, ", "); > + while (tokenizer.hasMoreTokens()) { > + String token = tokenizer.nextToken(); > + int tlen = token.length(); > + if (plen > tlen) { > + if (pkg.startsWith(token) > + && (token.charAt(tlen - 1) == '.' || pkg > + .charAt(tlen) == '.')) { > + return true; > + } > + } else if (plen + 1 >= tlen && token.startsWith(pkg)) { > + if (plen == tlen || token.charAt(tlen - 1) == '.') { > + return true; > + } > + } > + } > } > - String securePackages = getSecurityProperty("package.definition"); > - if (securePackages != null) { > - StringTokenizer tokenizer = new StringTokenizer(securePackages, > - ", "); > - while (tokenizer.hasMoreTokens()) { > - if (packageName.startsWith(tokenizer.nextToken())) { > - checkPermission(new RuntimePermission( > - "defineClassInPackage." + packageName)); > - return; > - } > - } > - } > - } > - > - private String getSecurityProperty(final String property) { > - PrivilegedAction<String> pa = PriviAction.getSecurityProperty(property); > - return AccessController.doPrivileged(pa); > + > + return false; > } > > /** > > Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java > URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java?view=diff&rev=486052&r1=486051&r2=486052 > ============================================================================== > --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java (original) > +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/util/PriviAction.java Mon Dec 11 23:43:28 2006 > @@ -53,8 +53,8 @@ > * > * @see Security#getProperty > */ > - public static <T> PrivilegedAction<T> getSecurityProperty(String property) { > - return new PriviAction<T>(GET_SECURITY_PROPERTY, property); > + public static PrivilegedAction<String> getSecurityProperty(String property) { > + return new PriviAction<String>(GET_SECURITY_PROPERTY, property); > } > > private PriviAction(int action, Object arg) { > > Modified: harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/MutableSecurityManager.java > URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/MutableSecurityManager.java?view=diff&rev=486052&r1=486051&r2=486052 > ============================================================================== > --- harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/MutableSecurityManager.java (original) > +++ harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/MutableSecurityManager.java Mon Dec 11 23:43:28 2006 > @@ -17,53 +17,54 @@ > package org.apache.harmony.luni.tests.java.lang; > > import java.security.Permission; > -import java.util.HashSet; > -import java.util.Set; > +import java.security.PermissionCollection; > +import java.security.Permissions; > > class MutableSecurityManager extends SecurityManager { > > static final RuntimePermission SET_SECURITY_MANAGER = new RuntimePermission("setSecurityManager"); > > - private final Set<Permission> permissions; > + private PermissionCollection enabled; > > - private String deny; > + private PermissionCollection denied; > > public MutableSecurityManager() { > super(); > - this.permissions = new HashSet<Permission>(); > + this.enabled = new Permissions(); > } > > public MutableSecurityManager(Permission... permissions) { > this(); > for (int i = 0; i < permissions.length; i++) { > - this.permissions.add(permissions[i]); > + this.enabled.add(permissions[i]); > } > } > > void addPermission(Permission permission) { > - permissions.add(permission); > - } > - > - void removePermission(Permission permission) { > - permissions.remove(permission); > + enabled.add(permission); > } > > void clearPermissions() { > - permissions.clear(); > + enabled = new Permissions(); > } > > - void denyPermission(String name) { > - deny = name; > + void denyPermission(Permission p) { > + if (denied == null) { > + denied = new Permissions(); > + } > + denied.add(p); > } > > @Override > public void checkPermission(Permission permission) { > - if (permissions.contains(permission)) { > + if (enabled.implies(permission)) { > return; > } > - if (permission.getName().equals(deny)){ > - throw new SecurityException(); > + > + if (denied != null && denied.implies(permission)){ > + throw new SecurityException("Denied " + permission); > } > + > super.checkPermission(permission); > } > } > > Modified: harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/SecurityManagerTest.java > URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/SecurityManagerTest.java?view=diff&rev=486052&r1=486051&r2=486052 > ============================================================================== > --- harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/SecurityManagerTest.java (original) > +++ harmony/enhanced/classlib/trunk/modules/luni/src/test/java/org/apache/harmony/luni/tests/java/lang/SecurityManagerTest.java Mon Dec 11 23:43:28 2006 > @@ -17,7 +17,7 @@ > package org.apache.harmony.luni.tests.java.lang; > > import java.io.File; > - > +import java.security.Security; > import tests.support.Support_Exec; > > import junit.framework.TestCase; > @@ -25,12 +25,102 @@ > public class SecurityManagerTest extends TestCase { > > /** > + * @tests java.lang.SecurityManager#checkPackageAccess(String) > + */ > + public void test_checkPackageAccessLjava_lang_String() { > + final String old = Security.getProperty("package.access"); > + Security.setProperty("package.access", "a.,bbb, c.d."); > + > + MutableSecurityManager sm = new MutableSecurityManager(); > + sm.denyPermission(new RuntimePermission("accessClassInPackage.*")); > + > + try { > + sm.checkPackageAccess("z.z.z"); > + sm.checkPackageAccess("aa"); > + sm.checkPackageAccess("bb"); > + sm.checkPackageAccess("c"); > + > + try { > + sm.checkPackageAccess("a"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + try { > + sm.checkPackageAccess("bbb"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + try { > + sm.checkPackageAccess("c.d.e"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + Security.setProperty("package.access", "QWERTY"); > + sm.checkPackageAccess("a"); > + sm.checkPackageAccess("qwerty"); > + try { > + sm.checkPackageAccess("QWERTY"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + } finally { > + Security.setProperty("package.access", > + old == null ? "" : old); > + } > + } > + > + /** > + * @tests java.lang.SecurityManager#checkPackageDefinition(String) > + */ > + public void test_checkPackageDefinitionLjava_lang_String() { > + final String old = Security.getProperty("package.definition"); > + Security.setProperty("package.definition", "a.,bbb, c.d."); > + > + MutableSecurityManager sm = new MutableSecurityManager(); > + sm.denyPermission(new RuntimePermission("defineClassInPackage.*")); > + > + try { > + sm.checkPackageDefinition("z.z.z"); > + sm.checkPackageDefinition("aa"); > + sm.checkPackageDefinition("bb"); > + sm.checkPackageDefinition("c"); > + > + try { > + sm.checkPackageDefinition("a"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + try { > + sm.checkPackageDefinition("bbb"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + try { > + sm.checkPackageDefinition("c.d.e"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + Security.setProperty("package.definition", "QWERTY"); > + sm.checkPackageDefinition("a"); > + sm.checkPackageDefinition("qwerty"); > + try { > + sm.checkPackageDefinition("QWERTY"); > + fail("This should throw a SecurityException."); > + } catch (SecurityException ok) {} > + > + } finally { > + Security.setProperty("package.definition", > + old == null ? "" : old); > + } > + } > + > + /** > * @tests java.lang.SecurityManager#checkMemberAccess(java.lang.Class, int) > */ > public void test_checkMemberAccessLjava_lang_ClassI() { > MutableSecurityManager sm = new MutableSecurityManager(); > sm.addPermission(MutableSecurityManager.SET_SECURITY_MANAGER); > - sm.denyPermission("accessDeclaredMembers"); > + sm.denyPermission(new RuntimePermission("accessDeclaredMembers")); > System.setSecurityManager(sm); > try { > try { > > >
