On Mon, 15 Sep 2025 21:38:29 GMT, David Beaumont <[email protected]> wrote:
>> 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.
If this is called with "/" in a module name then it means we've got a bug
somewhere. So I think the IAE would help find that bug quicker.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2350977385
