On Mon, 30 Nov 2020 15:59:07 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
>> This pull request replaces https://github.com/openjdk/jdk/pull/1227. >> >> From the original PR: >> >>> Please review the code for the second iteration of sealed classes. In this >>> iteration we are: >>> >>> * Enhancing narrowing reference conversion to allow for stricter >>> checking of cast conversions with respect to sealed type hierarchies >>> >>> * Also local classes are not considered when determining implicitly >>> declared permitted direct subclasses of a sealed class or sealed interface >>> >>> * renaming Class::permittedSubclasses to Class::getPermittedSubclasses, >>> still in the same method, the return type has been changed to Class<?>[] >>> instead of the previous ClassDesc[] >>> >>> * adding code to make sure that annotations can't be sealed >>> >>> * improving some tests >>> >>> >>> TIA >>> >>> Related specs: >>> [Sealed Classes >>> JSL](http://cr.openjdk.java.net/~gbierman/jep397/jep397-20201104/specs/sealed-classes-jls.html) >>> [Sealed Classes >>> JVMS](http://cr.openjdk.java.net/~gbierman/jep397/jep397-20201104/specs/sealed-classes-jvms.html) >>> [Additional: Contextual >>> Keywords](http://cr.openjdk.java.net/~gbierman/jep397/jep397-20201104/specs/contextual-keywords-jls.html) >> >> This PR strives to reflect the review comments from 1227: >> * adjustments to javadoc of j.l.Class methods >> * package access checks in Class.getPermittedSubclasses() >> * fixed to the narrowing conversion/castability as pointed out by Maurizio > > Jan Lahoda has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 12 commits: > > - Improving getPermittedSubclasses javadoc. > - Merge branch 'master' into JDK-8246778 > - Moving checkPackageAccess from getPermittedSubclasses to a separate method. > - Improving getPermittedSubclasses() javadoc. > - Enhancing the Class.getPermittedSubclasses() test to verify behavior both > for sealed classes in named and unnamed modules. > - Removing unnecessary file. > - Tweaking javadoc. > - Reflecting review comments w.r.t. narrowing conversion. > - Improving checks in getPermittedSubclasses() > - Merging master into JDK-8246778 > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/6e006223...4d484179 src/java.base/share/classes/java/lang/Class.java line 3042: > 3040: for (Class<?> c : classes) { > 3041: // skip the package access check on a proxy class in > default proxy package > 3042: if (!Proxy.isProxyClass(c) || > ReflectUtil.isNonPublicProxyClass(c)) { If a sealed class is in a named module, the permitted subclasses/subinterfaces are in the same module as the sealed class. If a sealed class is in an unnamed module, it will be in the same runtime package as the sealed class. A proxy class is dynamically generated and not intended for statically named in `permits` clause of a sealed class`. It can be in a different module or different package. So a permitted subclass or interface should never be a proxy class. So the package access check for permitted subclasses/subinterfaces can be simplified. I would suggest this check be inlined in `getPermittedSubclasses` as follows: SecurityManager sm = System.getSecurityManager(); if (subclasses.length > 0 && sm != null) { ClassLoader ccl = ClassLoader.getClassLoader(Reflection.getCallerClass()); ClassLoader cl = getClassLoader0(); if (ReflectUtil.needsPackageAccessCheck(ccl, cl)) { Set<String> packages = new HashSet<>(); for (Class<?> c : subclasses) { if (Proxy.isProxyClass(c)) throw new InternalError("a permitted subclass should not be a proxy class: " + c); String pkg = c.getPackageName(); if (!pkg.isEmpty()) packages.add(pkg); } for (String pkg : packages) { sm.checkPackageAccess(pkg); } } } ------------- PR: https://git.openjdk.java.net/jdk/pull/1483
