Hi Mandy,
> It's good to see JDK code be updated to use Class::getPackageName. One thing
> to pay attention is that Class.getPackageName() returns "java.lang" for
> primitive type and void. Your patch fixing ObjectStreamClass::getPackageName
> and Proxy::checkNewProxyPermission look fine.
> There are other places that can be converted. Do you mind updating
> java.io.ObjectInputFilter::matchesPackage and
> ClassLoader::checkPackageAccess? I may miss there are other places in
> java.base.
Thanks - I updated both as you suggested to use Class::getPackageName. Please
find the updated patch below.
There is also a public static VerifyAccess::getPackageName which seems to be
not working with arrays at all and as far as I can see is at least not used
inside the JDK itself.
Should this be deprecated maybe in favor of Class.getPackageName() or also
adjusted to use it (which would mean different return values than before as you
already noted)?
Cheers,
Christoph
====== PATCH =======
diff -r 438e0c9f2f17 src/java.base/share/classes/java/io/ObjectInputFilter.java
--- a/src/java.base/share/classes/java/io/ObjectInputFilter.java Mon Oct
30 17:49:33 2017 -0700
+++ b/src/java.base/share/classes/java/io/ObjectInputFilter.java Fri Nov
03 08:38:15 2017 +0100
@@ -656,8 +656,8 @@
* otherwise {@code false}
*/
private static boolean matchesPackage(Class<?> c, String pkg) {
- String n = c.getName();
- return n.startsWith(pkg) && n.lastIndexOf('.') == pkg.length()
- 1;
+ String n = c.getPackageName();
+ return n.length() == pkg.length() - 1 && n.startsWith(pkg);
}
/**
diff -r 438e0c9f2f17 src/java.base/share/classes/java/io/ObjectStreamClass.java
--- a/src/java.base/share/classes/java/io/ObjectStreamClass.java Mon Oct
30 17:49:33 2017 -0700
+++ b/src/java.base/share/classes/java/io/ObjectStreamClass.java Fri Nov
03 08:38:15 2017 +0100
@@ -1587,11 +1587,7 @@
* Returns package name of given class.
*/
private static String getPackageName(Class<?> cl) {
- String s = cl.getName();
- int i = s.lastIndexOf('[');
- i = (i < 0) ? 0 : i + 2;
- int j = s.lastIndexOf('.');
- return (i < j) ? s.substring(i, j) : "";
+ return cl.getPackageName();
}
/**
diff -r 438e0c9f2f17 src/java.base/share/classes/java/lang/ClassLoader.java
--- a/src/java.base/share/classes/java/lang/ClassLoader.java Mon Oct 30
17:49:33 2017 -0700
+++ b/src/java.base/share/classes/java/lang/ClassLoader.java Fri Nov 03
08:38:15 2017 +0100
@@ -672,12 +672,11 @@
return;
}
- final String name = cls.getName();
- final int i = name.lastIndexOf('.');
- if (i != -1) {
+ final String packageName = cls.getPackageName();
+ if (!packageName.isEmpty()) {
AccessController.doPrivileged(new PrivilegedAction<>() {
public Void run() {
- sm.checkPackageAccess(name.substring(0, i));
+ sm.checkPackageAccess(packageName);
return null;
}
}, new AccessControlContext(new ProtectionDomain[] {pd}));
diff -r 438e0c9f2f17 src/java.base/share/classes/java/lang/reflect/Proxy.java
--- a/src/java.base/share/classes/java/lang/reflect/Proxy.java Mon Oct 30
17:49:33 2017 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java Fri Nov 03
08:38:15 2017 +0100
@@ -1034,11 +1034,8 @@
// do permission check if the caller is in a different runtime
package
// of the proxy class
- int n = proxyClass.getName().lastIndexOf('.');
- String pkg = (n == -1) ? "" :
proxyClass.getName().substring(0, n);
-
- n = caller.getName().lastIndexOf('.');
- String callerPkg = (n == -1) ? "" :
caller.getName().substring(0, n);
+ String pkg = proxyClass.getPackageName();
+ String callerPkg = caller.getPackageName();
if (pcl != ccl || !pkg.equals(callerPkg)) {
sm.checkPermission(new
ReflectPermission("newProxyInPackage." + pkg));
