Re: [Patch][JDK10] Use Class.getPackageName() where possible

[mandy chung]

On 11/3/17 6:52 PM, Bernd Eckenfels wrote:
The private static helper in ObjectStreamClass became a oneliner and can be 
removed and the callsites can transform from getPackageName(c) to 
c.getPackageName().

Good catch.   we should clean that up.

Christoph - can you send a updated patch to remove 
ObjectStreamClass::getPackageName and replace the calls with 
Class::getPackageName and also remove VerifyAccess::getPackageName?

thanks
Mandy

Gruss
Bernd
--
http://bernd.eckenfels.net
_____________________________
From: mandy chung <mandy.ch...@oracle.com<mailto:mandy.ch...@oracle.com>>
Sent: Samstag, November 4, 2017 1:12 AM
Subject: Re: [Patch][JDK10] Use Class.getPackageName() where possible
To: Christoph Dreis <christoph.dr...@freenet.de<mailto:christoph.dr...@freenet.de>>, 
'Core-Libs-Dev' <core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>>




On 11/3/17 1:06 AM, Christoph Dreis wrote:
Thanks - I updated both as you suggested to use Class::getPackageName. Please 
find the updated patch below.
I have created https://bugs.openjdk.java.net/browse/JDK-8190733 for this
patch.

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)?
VerifyAccess::getPackageName is unused in jdk8u-dev neither. So I think it can 
be removed. No deprecation is needed since this is JDK internal API and I 
hardly think anyone is depending on it.

Mandy

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));