On Fri, 20 Jun 2025 11:13:12 GMT, David Beaumont <[email protected]> wrote:
>> Simplifying JavaRuntimeURLConnection to avoid accidentally returning
>> non-resource data to users.
>>
>> This change has the following distinct parts:
>> 1. Refactor code to use Node instead of directly accessing low level
>> ImageLocation type.
>> 2. Remove unnecessary use of "Resource" interface and related URL generation
>> code (completely unreachable).
>> 3. Adding comments explaining why there's a non-obvious distinction in how
>> module and resource names are treated with respect to URL percent encoding.
>> 4. Small constructor logic simplification (module name cannot be null
>> anymore)
>> 5. Small simplification around 'READER' use, since it is impossible for that
>> to ever be null (other users of ImageReaderFactory already assume it could
>> never be null, and code path analysis agrees).
>> 6. Adding tests for the non-resource cases.
>> 7. Adding extra test data to check the behaviour with respect to things like
>> percent escaping (previously untested).
>> 8. Adding TODO comments for things I could do in this PR or later (reviewer
>> opinions welcome).
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Feedback changes and renaming field to match nomenclature of JEP 220.
src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java
line 87:
> 85: if (resourceNode == null) {
> 86: if (path == null) {
> 87: throw new IOException("cannot connect to jrt:/" + module);
Maybe we can make this `module.isEmpty() || (path == null)` as it needs a
non-empty module and resource to connect.
test/jdk/sun/net/www/protocol/jrt/Basic.java line 64:
> 62: {"jrt:/java.base/", false},
> 63: // Cannot escape anything in the module name.
> 64: {"jrt:/java%2Ebase/java/lang/Object.class", false},
Can you add "jrt:/" to the list?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25871#discussion_r2159983831
PR Review Comment: https://git.openjdk.org/jdk/pull/25871#discussion_r2159984206
