On Wed, 17 Jul 2024 19:47:44 GMT, Chen Liang <li...@openjdk.org> wrote:
> `Class` has 2 VM-injected fields that can be made explicit: `Object[] > signers` and `ProtectionDomain protectionDomain`. We make the signers field > explicit. (The ProtectionDomain can be revisited when SecurityManager is > removed, as SecurityManager is accessing it via JNI as well.) > > Migrate the JNI code to Java. The getter previously had a redundant primitive > type check, which is dropped in the migrated Java code. The `Object[] > getSigners` is no longer `native`, thus requiring a CSR record. Reviewers > please help review the associated CSR. Signers dates back to JDK 1.1, touching it now will shine light on long standing technical debt and other issues. I think the main thing that is jumping out is that ClassLoader.setSigners (protected final method so may be called in subclasses) isn't fully specified and it is also missing a number of important checks. The method doesn't specify that the method ignores array classes or class objects for primitives. It doesn't say anything about the elements that aren't a Certificate are ignored. It doesn't specify null behavior either and doesn't say anything that the signers can change at any time. What's worse is that in the hands of a cowboy builder, it can be used to set or clear the signers for any Class, or keep a reference to the signers array and muck with them mid-flight. So lots of issues there. Technical debt aside, I think the transformation looks okay, just a bit confusing to have signers declared under a comment "Set by VM", it's not clear the comment applies only to the classData before it. At some point I think we should put a question mark on the JVMTI JVMTI_HEAP_REFERENCE_SIGNERS heap ref and the HPROF heap dump CLASS record where there is a slot for the signers array. I don't see any need for these in 2024 and could be potentially be null'ed in the future (would require a JVM TI spec change of course). On David's comment about exposing the field to code using Class.getDecalredField or Class.getDeclaredFields. Nosy code can use these methods to get a reference to the Field but it's not accessible by default. For now, code can using sun.misc.Unsafe but that is temporary and will go away. I don't have a strong opinion and no objection to adding it to the filter (which I think is what David is wondering about). ------------- PR Comment: https://git.openjdk.org/jdk/pull/20223#issuecomment-2236065493
