RFR: 8335989: Implement Module Import Declarations (Second Preview) [v4]

[Jan Lahoda]

On Tue, 29 Oct 2024 17:25:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits:
>> 
>>  - Updating PreviewFeature metadata
>>  - Cleanup.
>>  - Merge branch 'master' into JDK-8335989
>>  - Merge branch 'master' into JDK-8335989
>>  - Reflecting review feedback.
>>  - Cleanup.
>>  - Cleanup.
>>  - Fixing tests
>>  - Adding a separate scope for module imports.
>>  - Cleanup.
>>  - ... and 8 more: https://git.openjdk.org/jdk/compare/964d8d22...94b8bf6d
>
> src/java.base/share/classes/jdk/internal/module/ModuleInfo.java line 193:
> 
>> 191:         int minor_version = in.readUnsignedShort();
>> 192:         int major_version = in.readUnsignedShort();
>> 193:         boolean isPreview = minor_version == ClassFile.PREVIEW_MINOR_VERSION;
> 
> I think this will need to throw invalidModuleDescriptor if isPreview && major version != ClassFile.latestMajorVersion, otherwise the check in readModuleAttribute will be defeated by hand craft class files (the VM doesn't read module-info class files so it won't reject it, and this code is called from exposed APIs such as ModuleDescriptor.read).

I believe the check is already being done, right on the next line:
https://github.com/openjdk/jdk/blob/821c514a132e809a14648ddbb56f2ffee85fd35a/src/java.base/share/classes/jdk/internal/module/ModuleInfo.java#L192
which leads to:
https://github.com/openjdk/jdk/blob/821c514a132e809a14648ddbb56f2ffee85fd35a/src/java.base/share/classes/jdk/internal/misc/VM.java#L188
which then leads to:
https://github.com/openjdk/jdk/blob/821c514a132e809a14648ddbb56f2ffee85fd35a/src/java.base/share/classes/jdk/internal/misc/VM.java#L174
which has this check:
https://github.com/openjdk/jdk/blob/821c514a132e809a14648ddbb56f2ffee85fd35a/src/java.base/share/classes/jdk/internal/misc/VM.java#L179

I tried with a `module-info.class` with major version 65, and minor version 0xFFFF, and it correctly threw the exception for me.

Do I miss something?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21431#discussion_r1822553137