[Patch][JDK10] Use Class.getPackageName() where possible
Christoph Dreis
christoph.dreis at freenet.de
Fri Nov 3 08:06:38 UTC 2017
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));
More information about the core-libs-dev
mailing list