RFR: 8241294: Examine input checking in ClassLoader::package_from_class_name

[Claes Redestad]

On 2020-03-27 21:47, Lois Foltan wrote:
> On 3/27/2020 9:55 AM, Claes Redestad wrote:
>> Hi,
>>
>> in ClassLoader::package_from_class_name, testing if the class name input
>> is NULL is redundant, and testing for and skipping ['s is wrong, since
>> fully qualified array class names doesn't start with [.
>>
>> We can also get rid of the bad_class_name parameter, and move
>> the complexity of that checking to the only call site that distinguishes
>> between an empty package NULL and a bad class name NULL
>>
>> Webrev: http://cr.openjdk.java.net/~redestad/8241294/open.00/
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8241294
>>
>> Testing: tier1-4 (ongoing)
>>
>> Thanks!
>>
>> /Claes
> 
> Hi Claes,
> Looks good.  I'm glad to be rid of that "bad_class_name" parameter. 
> Thank you for doing this. 

Thanks!

> Minor comment:
> 
> - classfile/classLoader.cpp
> line #180 & #188 - plase consider adding an improved comment over "Can't 
> be" in the asserts.  Maybe
> line #180 "unexpected null class name"
> line #188 "class name's first character contains an array opening bracket"

Fixed.

> 
> Also, I was able to write a .jasm file with a class whose name is "[FOO" 
> for example.  With your patch, I did try executing the resulting .class 
> file and am receiving a CNFE, "Could not find or load main class [FOO" 
> exception.  If you would like the .jasm file I can mail it to you to 
> experiment more with.

Added a sanity test based on runtime/ClassFile/FormatCheckingTest 
(thanks for the pointer!):

http://cr.openjdk.java.net/~redestad/8241294/open.01/

The test outputs the same ClassFormatError as the old test case
(LBadHelloWorld; vs [BadHelloWorld2). Running BadHelloWorld2 fails with
a CNFE if I run without verification, both with and without my patch,
so I think we have verified that such classes are rejected before ever
hitting the package_for_class_name.

Thanks,

/Claes

> 
> Thanks,
> Lois