On Fri, 12 Sep 2025 15:42:24 GMT, Alan Bateman <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Tweak test.
>
> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 360:
>
>> 358: // underlying jimage, but can only reliably return resource
>> nodes.
>> 359: if (moduleName.indexOf('/') >= 0) {
>> 360: return null;
>
> Can you explain this further? We can't have a "/' in the module name at the
> language or API level. We could at the class level but there is likely a lot
> of work required in jimage, jlink and elsewhere if we ever allowed "/" in a
> module name. So right now, it would be a IAE if a call if invoking this with
> a "/" in the module name.
Do you mean explain it in this review thread, or in the code comment?
I know module names shouldn't have a '/' in, but were someone to call it with
such, then the constructed path could become valid (e.g. module
name="java.base/java", resource path="lang/Object.class". Since I wasn't sure
which of the callers can be trusted to pass only valid module names, I didn't
want to make it an exception in case I caused something to blow up somewhere
else.
I'm happy to make it an IAE, I'm just trying to avoid code paths where this API
gets given invalid input, but is fooled into giving back real data.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2350185259
