follow-up to 8241294: Examine input checking in ClassLoader::package_from_class_name

David Holmes david.holmes at oracle.com
Tue Jun 9 05:00:51 UTC 2020


On 9/06/2020 2:59 am, Claes Redestad wrote:
> Hi,
> 
> the method in question has historically been tangled up with use sites
> that pass either fully qualified class names ('java/lang/Thread') or
> class name signatures (e.g., '[Ljava/lang/Thread').

I certainly find the code in question very confusing when looking at it 
now. It seems to be expecting the FQN form, yet is within logic that 
already found the array [ and so should always expect that to be 
followed by 'L'. The code was added in 2016 under JDK-8153858:

http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-June/019767.html

I reviewed it at the time, but now I am at a loss to explain it. :(

David
-----

> I have no clarifications to give other than I think the way forward is
> to ensure signatures are always parsed into their constituent parts as
> they are read from the constant pool and not allowed to roam around in
> the VM code acting as fully qualified class names (which they aren't).
> 
> /Claes
> 
> On 2020-06-08 17:20, Andrey Petushkov wrote:
>> Dear Claes, All,
>>
>> one question related to the subject. Even though it's considered
>> legitimate for the method to accept '['s at the start of the input
>> IMHO the check for subsequent 'L' is wrong and the related comment is
>> misleading.
>>
>> JLS does not prohibit class and package names to start from capital L,
>> so it's unclear why arrays of those, and only those (considering these
>> appear here in internal form) being banned?
>>
>> Moreover, given that this function is widely used it's even more
>> important that if there is some internal special case which passes
>> such forms as argument it's better to be clearly documented, instead
>> of saying false things like "Fully qualified class names should not
>> contain a 'L'." (1).
>>
>> However this this code seem to pass quite number of editorials by
>> different people and still has the same check. So there might be a
>> reason. Given that may I ask for kind clarification of the matter?
>>
>> Thank you,
>> Andrey
>>
>> [1] 
>> https://hg.openjdk.java.net/jdk/jdk/file/5efafa45f3b8/src/hotspot/share/classfile/classLoader.cpp#l201 
>>
>>


More information about the hotspot-runtime-dev mailing list