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 {
