On Tue, 29 Oct 2024 17:25:37 GMT, Alan Bateman <al...@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
