RFR: JDK-8245432: Lookup::defineHiddenClass should throw UnsupportedClassVersionError if the given bytes are of an unsupported major or minor version

[Mandy Chung]

It's fixed.  Thanks
Mandy

On 5/28/20 4:35 PM, Johannes Kuhn wrote:
> Hi,
>
> just noticed a small thing:
> The magic is 4 bytes, but readUnsignedShort reads two bytes.
>
> - Johannes
>
> On 28-May-20 19:25, Mandy Chung wrote:
>>
>>
>> On 5/28/20 6:55 AM, Alan Bateman wrote:
>>> On 28/05/2020 05:13, Mandy Chung wrote:
>>>> Updated webrev:
>>>> http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8245432/webrev.01/
>>>>
>>>> I modify this patch to check the class file version and throws CFE 
>>>> if unsupported before creating ClassReader.  This also fixes 
>>>> JDK-8245061 that it reads the value of `this_class` as a constant 
>>>> (as ASM will throw an exception if it's not a constant) and also 
>>>> ensures that `this_class` is a CONSTANT_Class_info by checking the 
>>>> descriptor string from Type.
>>> I don't want to complicate this further but the --enable-preview 
>>> handling in the VM doesn't seem to expose an internal property that 
>>> allows code in the libraries know if preview feature are enabled or 
>>> not. I was assuming that isSupportedClassFileVersion would need to 
>>> check that.
>>>
>>
>> The runtime will ensure if --enable-preview is set if a class file 
>> with minor is loaded.   I will prefer to keep 
>> VM::isSupportedClassFileVersion to validate the given major/minor 
>> version.  At runtime, it will fail when such class file is loaded if 
>> preview is not enabled.
>>
>> I'll add a comment to describe that.
>>
>>> Will readUnsignedShort throw AIOBE if the offset is beyond the length? 
>>
>> Good catch.  I add the check to throw CFE.
>> +            private static int readUnsignedShort(byte[] bytes, int 
>> offset) {
>> +                if (offset >= bytes.length) {
>> +                    throw new ClassFormatError("Invalid ClassFile 
>> structure");
>> +                }
>> +                return ((bytes[offset] & 0xFF) << 8) | (bytes[offset 
>> + 1] & 0xFF);
>> +            }
>>
>>> Also are you sure about "& 0xCAFEBABE", I assumed it would just 
>>> check the magic number is equal to that.
>>
>> It should check ==.  Fixed.
>>
>> thanks
>> Mandy
>
>