On Fri, 12 Feb 2021 02:10:02 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> Claes Redestad has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Consolidate verifyClassname and verifyFixClassname > > This more limited cleanup looks good. This patch changes `JVM_FindLoadedClass` interface to only accept a binary name. It used to accept both a binary name and internal form. Most, if not all, JVM entry points take the name of internal name. So this change makes this JVM entry point inconsistent with others. Looking closer each API that involves `fixClassName` or `verifyXXXClassName`, the JVM entry points called expects the internal form except `JVM_FindLoadedClass` (see details below). I think a better change is to change the native `JVM_FindLoadedClass` to accept the internal form only and have `findLoadedClass0` method to detect if the name contains '/' or '['. ClassLoader API does not allow loading of an array type whereas `Class::forName` allows to find an array type. Perhaps `verifyFixClassName` should be renamed like `binaryNameToInternalForm`. I think we don't need `fixClassname`? ClassLoader::defineClass - `preDefineClass` checks the name and throws if it contains '/' or '[' - no name check in `JVM_DefineClassWithSource` and `JVM_LookupDefineClass` which expects the name is of internal form native Class::forName0 - converts the binary name to internal form (i.e. replace '.' with '/') - throw if the name contains '/' - no explicit name check in `JVM_FindClassFromCaller` ClassLoader::loadClass - calls native `findLoadedClass0` that calls `JVM_FindLoadedClass` which accepts binary form and converts '.' to '/' but the current implementation accepts both binary name and internal form - calls `native findBootstrapClass` which converts '.' to '/' and pass the internal form to `JVM_FindBootstrapClass`. It'd be helpful to document the internal APIs and JVM entry points clearly what it expects for example binary name vs internal form and where it does the validation e.g. Class::forName0 allows array type and native library methods do the name validation. ------------- PR: https://git.openjdk.java.net/jdk/pull/2378